Closed Bug 1488697 Opened 2 years ago Closed 2 years ago

StringBuilder::ToString should use BulkWrite


(Core :: DOM: Core & HTML, enhancement, P3)




Tracking Status
firefox64 --- fixed


(Reporter: hsivonen, Assigned: hsivonen)


(Blocks 1 open bug)


(Keywords: perf)


(1 file)

StringBuider ensures that the appends it does can't fail. It also knows that what it what it appends can't alias the buffer of the string. Yet, on each append (including on a per-code unit basis!), the string implementation re-checks for these conditions. Also, the zero terminator gets written and rewritten all the time.

Additionally, StringBuider forgoes SIMD acceleration when appending text nodes that contain markup-significant characters.

This can be made more efficient using BulkWrite.
smaug, can you recommend a benchmark that tests the innerHTML getter?
Flags: needinfo?(bugs)
Look at the blame of StringBuilder. It was added because of performance.
Flags: needinfo?(bugs)
Priority: -- → P3
Blocks: 1489036
(In reply to Olli Pettay [:smaug] from comment #5)
> has a link to at
> least some testcase.

Thanks. With this patch, I see a 13% execution time reduction for body.innerHTML on that benchmark. Seems like a worthwhile improvement. (Desktop-class Haswell i7, from 182.8 ms to 158.2 ms.)
* Avoid the string implementation's capacity checks, since we know they
 * Avoid the string implementation's aliasing checks, since we know there's no
 * Avoid writing the zero terminator more than once or out of sequence.
 * Use u"" literals when appending literals to a UTF-16 string.
 * Write runs of non-escaped code units instead of writing code unit by code
   unit in order to benefit from SIMD (either via memcpy or

This results in a 13% execution time reduction on desktop Haswell i7 when
getting the innerHTML of the body of the Selectors spec. (The WebKit
optimization target from .)

MozReview-Commit-ID: LAg3gkGJnpQ
Comment on attachment 9007148 [details]
Bug 1488697 - Make StringBuilder::ToString avoid repeated capacity and aliasing checks as well as terminator writes.

Olli Pettay [:smaug] has approved the revision.
Attachment #9007148 - Flags: review+
Pushed by
Make StringBuilder::ToString avoid repeated capacity and aliasing checks as well as terminator writes. r=smaug
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Consider adding the `perf` keyword here?
Keywords: perf
On Haswell desktop i7 on Windows, for the body.innerHTML case, this patch took Firefox from slightly slower than Chrome to slightly faster than Chrome. On a Core2 Duo Mac, Safari is still faster in the body.innerHTML case.

For the div cases, both WebKit and Blink are suspiciously slow compared to Gecko making me wonder if the div cases test a totally different thing in Gecko compared to WebKit/Blink.
Depends on: 1512758
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.