Bug 90247 – Use StringBuilder in SegmentedString::toString()

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Kwang Yul Seo]

Use a StringBuilder instead of String concatenation because StringBuilder is generally faster.

[Kwang Yul Seo]

Created attachment 150082 [details]
Patch

[Adam Barth]

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?

[Kwang Yul Seo]

(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

[Adam Barth]

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.

[WebKit Review Bot]

Comment on attachment 150082 [details]
Patch

Clearing flags on attachment: 150082

Committed r121523: <http://trac.webkit.org/changeset/121523>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.