Summary: | Use StringBuilder in SegmentedString::toString() | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kwang Yul Seo <skyul> | ||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, eric, webkit.review.bot, yurys | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Kwang Yul Seo
2012-06-28 21:56:15 PDT
Created attachment 150082 [details]
Patch
Comment on attachment 150082 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150082&action=review > Source/WebCore/ChangeLog:9 > + Use a StringBuilder instead of String concatenation because StringBuilder is generally faster. > + No new tests. Covered by existing tests. Does this actually make things faster? If so, by how much? (In reply to comment #2) > (From update of attachment 150082 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=150082&action=review > > > Source/WebCore/ChangeLog:9 > > + Use a StringBuilder instead of String concatenation because StringBuilder is generally faster. > > + No new tests. Covered by existing tests. > > Does this actually make things faster? If so, by how much? The only caller of SegmentedString::toString is XMLDocumentParser::append(const SegmentedString&). This method retrieves parseString from the argument s as in the following: void XMLDocumentParser::append(const SegmentedString& s) { String parseString = s.toString(); ... if (m_parserPaused) { m_pendingSrc.append(s); return; } ... } Because callers of XMLDocumentParser::append usually pass a String, the SegmentedString s usually consists of a single String. In this case, this patch might not improve performance. However, if the parser is paused, s is appended to m_pendingSrc. When the parser is resumed, m_pendingSrc is appended again. Now we have a SegmentedString with multiple substrings. I guess this patch improves the performance in this case. I admit this is a micro optimization, so I am not sure if I can measure the performance improvement in a statistically valid method. If StringBuilder with a single append performs worse than a Single string concatenation, I am skeptical of this being a noticeable improvement Comment on attachment 150082 [details]
Patch
Ok. I'm not sure whether this is actually a perf win, but I'm willing to say we should do this on general principles.
Comment on attachment 150082 [details] Patch Clearing flags on attachment: 150082 Committed r121523: <http://trac.webkit.org/changeset/121523> All reviewed patches have been landed. Closing bug. |