-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
StringBuffer in JavaScript is much slower than String #1216
Comments
with frog r3425, here's what I see: buildWithConcat: 47 Added Area-Frog label. |
Added Triaged label. |
"The StringBuffer class should work at least as fast as String." might be impossible. The emitted JS uses JS string concatenation. In V8 this is optimized by having multiple string representations, including a tree or 'concat' node pointing to the left and right substrings, giving constant time concatenation of large strings (at the cost of more complex access to the string contents). I am not surprised that buildWithConcat beats buildWithStringBuffer when compiled to JS. The StringBuffer is simply incurring overhead by keeping the string fragments in list before using a concat loop! So "The StringBuffer class should work at least as fast as String." might be impossible on dartc/frog code running on V8. The best you can hope for is that the two are closer, and I think that is a frog corelib implementation issue. If all JS engines have similar concat tricks, direct concatenation might be the best implementation. DartVM has flat strings, which makes buildWithConcat O(N^2). One might say that the VM has a serious performance bug in that string concatenation is not as fast as StringBuffer, and the exceedingly common 's += ... in a loop' antipattern is not fixed by language runtime technology. |
This comment was originally written by jimhug@google.com John had an idea about this that he suggested to me recently. We could try rewriting the StringBuffer type for the frog corelib to just use string concatenation internally instead of using the default dart implementation. Based on this data, we should try that to see if we can make the two versions of the code closer in performance. Considering all of the optimization work that has gone into string concatentation in the various JS enginges out there, it is highly unlikely that sringbuffer will ever be faster than += for a dart->js compiler. We may be able to make the performance at least close. cc @jmesserly. |
I'd try both string concatenation and collecting the strings in an array and then use String.prototype.join to combine them. Both have their strengths and weaknesses, mostly depending on the further use of the string. |
This comment was originally written by gundla...@gmail.com Re @5's mention of .join(): why use StringBuffer.toString() at all, instead of using List<String>.join('')? I would hope the implementation would be similar under the hood, and would let Dart eliminate the redundant (verbosely named) class. FWIW, Python doesn't have a StringBuffer class; you use |
This comment was originally written by @seaneagan |
Removed Area-Frog label. |
This is likely still the case for dart2js. Removed FromAreaFrog label. |
Added this to the Later milestone. |
Removed this from the Later milestone. |
Added this to the M2 milestone. |
Removed Priority-Medium label. |
Removed this from the M2 milestone. |
This is the third most starred bug in dart2js. |
We should be able to do better. I just tried this JavaScript code: function StringBuffer() { function buildWithStringBuffer() { for (var i = 99999; i > 0; i--) { return sb.toString(); function buildWithConcat() { for (var i = 99999; i > 0; i--) { return s; function measure(expected, f) { buildWithStringBuffer took 83ms Now concat is "only" 2.4411764705882355 times faster which is an improvement over what was reported (3.8660170523751525) and frog (3.3404255319148937). |
Using this version of StringBuffer, they are roughly equivalent: function StringBuffer() { |
Even this version is fine: function StringBuffer() { Best numbers with this version: buildWithStringBuffer took 38ms |
This issue was originally filed by sethladd@gmail.com
For the following code:
// use new hotness: StringBuffer
buildWithStringBuffer() {
var sb = new StringBuffer();
for (var i = 9999; i > 0; i--) {
sb.add(i.toString());
sb.add(" beers on the wall\n");
}
var finalString = sb.toString();
}
// use old school + for string concatenation
buildWithConcat() {
var s = '';
for (var i = 9999; i > 0; i--) {
s += i.toString();
s += " beers on the wall\n";
}
var finalString = s;
}
// wrap a function fn with the Stopwatch class to time execution
xmeasure(text,fn) {
var sw = new Stopwatch.start();
fn();
print(text + sw.elapsedInMs());
}
// the following timings are from the Dart VM
// smaller is faster is better
main() {
xmeasure('buildWithConcat: ',buildWithConcat); // 3574 !!
xmeasure('buildWithStringBuffer: ',buildWithStringBuffer); // 20
}
In Dart VM, we see StringBuffer greatly out performing + concat.
When compiled to JavaScript, we see the opposite:
buildWithConcat: 821
buildWithStringBuffer: 3174
The StringBuffer class should work at least as fast as String.
The text was updated successfully, but these errors were encountered: