New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String concat speed #40

Closed
rjacoby opened this Issue Jan 11, 2013 · 17 comments

Comments

Projects
None yet
@rjacoby

rjacoby commented Jan 11, 2013

In the Strings section the standard is:

"Strings longer than 80 characters should be written across multiple lines using string concatenation."

I understand the clarity of code - but the performance is 80-100% worse. jsPerf

The need for such long strings is debatable anyway, but isn't this kind of a steep performance penalty for code style?

@hshoff

This comment has been minimized.

Show comment
Hide comment
@hshoff

hshoff Jan 11, 2013

Member

Hi Rafi, thanks for the checking out the style guide and including a jsPerf.

We think that if you are fine-tuning your app and you find that the bottleneck is string concatenation, then by all means move to a single line and leave a comment saying "this was slowing down the app, moved to a single line" kind of thing.

Ideally, we would avoid this mess all together and keep really long strings out of our apps, but sometimes it comes up.

This is one of those cases why we call it a "mostly reasonable approach to javascript", not everything was decided by a jsPerf, we just made a style call and went with clarity over performance.

My gut feeling is there will be many other things to optimize for performance before you get down to long string concatenation, but I haven't come across this problem before.

I'll leave this issue open for now and hopefully some other folks will chime in with an opinion and we can correct it if necessary.

🍻

Member

hshoff commented Jan 11, 2013

Hi Rafi, thanks for the checking out the style guide and including a jsPerf.

We think that if you are fine-tuning your app and you find that the bottleneck is string concatenation, then by all means move to a single line and leave a comment saying "this was slowing down the app, moved to a single line" kind of thing.

Ideally, we would avoid this mess all together and keep really long strings out of our apps, but sometimes it comes up.

This is one of those cases why we call it a "mostly reasonable approach to javascript", not everything was decided by a jsPerf, we just made a style call and went with clarity over performance.

My gut feeling is there will be many other things to optimize for performance before you get down to long string concatenation, but I haven't come across this problem before.

I'll leave this issue open for now and hopefully some other folks will chime in with an opinion and we can correct it if necessary.

🍻

@reissbaker

This comment has been minimized.

Show comment
Hide comment
@reissbaker

reissbaker Jan 11, 2013

Contributor

It's a good point that it's slower, but I agree with @hshoff. If you have a rare case where it's a bottleneck, definitely do what it takes to get rid of it (and include a comment!), but in general we aim for readability over raw speed.

Contributor

reissbaker commented Jan 11, 2013

It's a good point that it's slower, but I agree with @hshoff. If you have a rare case where it's a bottleneck, definitely do what it takes to get rid of it (and include a comment!), but in general we aim for readability over raw speed.

@rjacoby

This comment has been minimized.

Show comment
Hide comment
@rjacoby

rjacoby Jan 11, 2013

Totally agree with @hshoff and @reissbaker on this as an less-common bottleneck. Maybe just note that it shouldn't be overused?

Honestly it just popped out at me b/c of general concat performance in other languages (Java, Ruby).

rjacoby commented Jan 11, 2013

Totally agree with @hshoff and @reissbaker on this as an less-common bottleneck. Maybe just note that it shouldn't be overused?

Honestly it just popped out at me b/c of general concat performance in other languages (Java, Ruby).

@hshoff

This comment has been minimized.

Show comment
Hide comment
@hshoff

hshoff Jan 11, 2013

Member

@rjacoby good call. I'll make a note and add the jsPerf.

Member

hshoff commented Jan 11, 2013

@rjacoby good call. I'll make a note and add the jsPerf.

@hshoff hshoff closed this in 4815ab0 Jan 11, 2013

@michaelmior

This comment has been minimized.

Show comment
Hide comment
@michaelmior

michaelmior May 24, 2013

You really shouldn't have to worry about this in your code and this should be handled by whatever tool you use to minify your JS for production. For example, uglifyjs will turn

var x = "a" + "b";

into

var x="ab";

michaelmior commented May 24, 2013

You really shouldn't have to worry about this in your code and this should be handled by whatever tool you use to minify your JS for production. For example, uglifyjs will turn

var x = "a" + "b";

into

var x="ab";
@bridge-rabish

This comment has been minimized.

Show comment
Hide comment
@bridge-rabish

bridge-rabish Nov 11, 2013

I used this method for long string

['<div>',
    '<p>this is test</p>',
'</div>'].join('')

bridge-rabish commented Nov 11, 2013

I used this method for long string

['<div>',
    '<p>this is test</p>',
'</div>'].join('')
@gustavopaes

This comment has been minimized.

Show comment
Hide comment
@gustavopaes

gustavopaes commented Dec 11, 2013

Use join() it's a bad idea.
http://jsperf.com/ya-string-concat/10

@Nemoden

This comment has been minimized.

Show comment
Hide comment
@Nemoden

Nemoden Apr 8, 2015

So, why exactly strings concatenation is preferred by the Guide if it's 95% slower than joining it with the break symbol ()? It doesn't add much readability and extremely slow.

js_perf_long_strings

Nemoden commented Apr 8, 2015

So, why exactly strings concatenation is preferred by the Guide if it's 95% slower than joining it with the break symbol ()? It doesn't add much readability and extremely slow.

js_perf_long_strings

@nkbt

This comment has been minimized.

Show comment
Hide comment
@nkbt

nkbt Apr 8, 2015

@Nemoden maybe because of readability. When string concatenation becomes a bottleneck in the app, then it can be easily converted to the fastest way available.

nkbt commented Apr 8, 2015

@Nemoden maybe because of readability. When string concatenation becomes a bottleneck in the app, then it can be easily converted to the fastest way available.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost commented Apr 8, 2015

Ok

@yonghu86

This comment has been minimized.

Show comment
Hide comment
@yonghu86

yonghu86 Apr 10, 2015

well,it's not length.

yonghu86 commented Apr 10, 2015

well,it's not length.

@michaelmior

This comment has been minimized.

Show comment
Hide comment
@michaelmior

michaelmior Apr 10, 2015

I think concatenation is much more readable for the sole fact that you can keep the correct level of indentation. As I noted in my previous comment, I think concatenation of static strings should be removed by build tools so performance should not really ever be an issue.

michaelmior commented Apr 10, 2015

I think concatenation is much more readable for the sole fact that you can keep the correct level of indentation. As I noted in my previous comment, I think concatenation of static strings should be removed by build tools so performance should not really ever be an issue.

@ibc

This comment has been minimized.

Show comment
Hide comment
@ibc

ibc Jul 3, 2015

I think concatenation is much more readable for the sole fact that you can keep the correct level of indentation.

Or you can use a proper editor that properly indents extra lines if they belong to the same line.

ibc commented Jul 3, 2015

I think concatenation is much more readable for the sole fact that you can keep the correct level of indentation.

Or you can use a proper editor that properly indents extra lines if they belong to the same line.

@ivanjonas

This comment has been minimized.

Show comment
Hide comment
@ivanjonas

ivanjonas Sep 19, 2015

It seems that the latest versions of Chrome have made performance a moot point. Here's a screenshot of revision 6 results.
image

ivanjonas commented Sep 19, 2015

It seems that the latest versions of Chrome have made performance a moot point. Here's a screenshot of revision 6 results.
image

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Sep 19, 2015

Member

Nowaways it's preferred to use an ES6 template string literal - which transpiles to a long string (with or without breaks, i forget). This entire issue seems like a moot point now :-)

Member

ljharb commented Sep 19, 2015

Nowaways it's preferred to use an ES6 template string literal - which transpiles to a long string (with or without breaks, i forget). This entire issue seems like a moot point now :-)

lencioni added a commit that referenced this issue Aug 4, 2016

Reverse rule on string concatenation for long lines
Broken and concatenated long strings are painful to work with and
produce less readable and searchable code. I think we should reverse
this rule.

Unfortunately, the max-len rule currently does not allow for this, but
there is currently a proposal to add an option to ESLint's max-len rule
that would allow for strings to be ignored.

  eslint/eslint#5805

There have also been discussions around performance of string
concatenation (#40), but I
don't think that is very relevant here so I removed the links to them.

lencioni added a commit that referenced this issue Aug 4, 2016

Reverse rule on string concatenation for long lines
Broken and concatenated long strings are painful to work with and
produce less readable and searchable code. I think we should reverse
this rule.

Unfortunately, the max-len rule currently does not allow for this, but
there is currently a proposal to add an option to ESLint's max-len rule
that would allow for strings to be ignored.

  eslint/eslint#5805

There have also been discussions around performance of string
concatenation (#40), but I
don't think that is very relevant here so I removed the links to them.

lencioni added a commit that referenced this issue Aug 4, 2016

Reverse rule on string concatenation for long lines (#995)
Broken and concatenated long strings are painful to work with and
produce less readable and searchable code. I think we should reverse
this rule.

Unfortunately, the max-len rule currently does not allow for this, but
there is currently a proposal to add an option to ESLint's max-len rule
that would allow for strings to be ignored.

  eslint/eslint#5805

There have also been discussions around performance of string
concatenation (#40), but I
don't think that is very relevant here so I removed the links to them.
@GrayedFox

This comment has been minimized.

Show comment
Hide comment
@GrayedFox

GrayedFox Jan 16, 2017

Not quite a moot point.... basically, referencing variables inside of a template literal is still much slower than string concatenation, however without referencing a variable (and simply using a template literal for line breaks) seems to have no performance impact... https://jsperf.com/es6-string-literals-vs-string-concatenation/4

GrayedFox commented Jan 16, 2017

Not quite a moot point.... basically, referencing variables inside of a template literal is still much slower than string concatenation, however without referencing a variable (and simply using a template literal for line breaks) seems to have no performance impact... https://jsperf.com/es6-string-literals-vs-string-concatenation/4

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Jan 16, 2017

Member

But also, "much slower" won't make any difference unless you're building millions of strings per second. It's identically fast for most use cases.

Member

ljharb commented Jan 16, 2017

But also, "much slower" won't make any difference unless you're building millions of strings per second. It's identically fast for most use cases.

hibearpanda added a commit to 15Prospects/javascript that referenced this issue Jan 22, 2017

Reverse rule on string concatenation for long lines (#995)
Broken and concatenated long strings are painful to work with and
produce less readable and searchable code. I think we should reverse
this rule.

Unfortunately, the max-len rule currently does not allow for this, but
there is currently a proposal to add an option to ESLint's max-len rule
that would allow for strings to be ignored.

  eslint/eslint#5805

There have also been discussions around performance of string
concatenation (airbnb#40), but I
don't think that is very relevant here so I removed the links to them.

jaylaw81 added a commit to appirio-digital/ads-best-practices that referenced this issue Sep 19, 2017

Reverse rule on string concatenation for long lines (#995)
Broken and concatenated long strings are painful to work with and
produce less readable and searchable code. I think we should reverse
this rule.

Unfortunately, the max-len rule currently does not allow for this, but
there is currently a proposal to add an option to ESLint's max-len rule
that would allow for strings to be ignored.

  eslint/eslint#5805

There have also been discussions around performance of string
concatenation (airbnb#40), but I
don't think that is very relevant here so I removed the links to them.

sensiblegame added a commit to sensiblegame/React-BNB that referenced this issue Oct 23, 2017

Reverse rule on string concatenation for long lines (#995)
Broken and concatenated long strings are painful to work with and
produce less readable and searchable code. I think we should reverse
this rule.

Unfortunately, the max-len rule currently does not allow for this, but
there is currently a proposal to add an option to ESLint's max-len rule
that would allow for strings to be ignored.

  eslint/eslint#5805

There have also been discussions around performance of string
concatenation (airbnb/javascript#40), but I
don't think that is very relevant here so I removed the links to them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment