Closed Bug 609075 Opened 9 years ago Closed 9 years ago

speed up JSString::flatten a bit

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: luke, Assigned: luke)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(3 files)

Attached file SS harness results
The attached patches tweak JSString::flatten/js_ConcatStrings.  I'm measuring a 1-2% (2-4ms) win on SS and .5% (10ms) win on V8 (SS harness -m).  A micro-benchmark under valgrind shows a 38% decrease in instruction count in flatten.
Attached patch JS_CeilingLog2Splinter Review
Attachment #487642 - Flags: review?(alangpierce)
Attachment #487641 - Attachment mime type: application/octet-stream → text/plain
Attached patch optimize flattenSplinter Review
Attachment #487650 - Flags: review?(alangpierce)
I had a third patch that took out the "grow the left string in-place if there is room" case, but I remembered what it was for, so no change to js_ConcatStrings.  (It's for cases like

for (...) {
  x = x + "asdf";
  print(x);  // flatten x
}

where x is flatten on each iteration to avoid O(n^2) copying.  I think this case can be removed in bug 608776 by having flatten() do it.)
Summary: speed up JSString::flatten/js_ConcatStrings a bit → speed up JSString::flatten a bit
Attachment #487642 - Flags: review?(alangpierce) → review?(gal)
Comment on attachment 487650 [details] [diff] [review]
optimize flatten

Andreas, I think you reviewed the initial ropes patch.
Attachment #487650 - Flags: review?(alangpierce) → review?(gal)
Attachment #487642 - Flags: review?(gal) → review+
Comment on attachment 487650 [details] [diff] [review]
optimize flatten

Nice profiling job Luke. ++lw
Attachment #487650 - Flags: review?(gal) → review+
http://hg.mozilla.org/mozilla-central/rev/0ba07bd00178
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.