-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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
Comments
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. 🍻 |
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. |
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 good call. I'll make a note and add the jsPerf. |
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
into
|
I used this method for long string ['<div>', '<p>this is test</p>', '</div>'].join('') |
Use join() it's a bad idea. |
@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. |
Ok |
well,it's not length. |
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. |
Or you can use a proper editor that properly indents extra lines if they belong to the same line. |
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 :-) |
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.
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.
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.
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 |
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. |
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.
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.
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.
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.
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.
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.
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.
In the Strings section the standard is:
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?
The text was updated successfully, but these errors were encountered: