Bug 152662

Summary: StringBuilder often creates two StringImpls.
Product: WebKit Reporter: Andreas Kling <kling>
Component: Web Template FrameworkAssignee: Andreas Kling <kling>
Status: REOPENED ---    
Severity: Normal CC: benjamin, cdumez, cmarcelo, commit-queue, kling, simon.fraser
Priority: P2 Keywords: Performance
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Proposed patch none

Description Andreas Kling 2016-01-03 01:25:45 PST 
Sometimes StringBuilder will keep the internal buffer used during building, and the returned "built" StringImpl will be a substring that retains the buffer. This way we keep two StringImpls around instead of one.
Comment 1 Andreas Kling 2016-01-03 01:29:15 PST 
Created attachment 268134 [details]
Proposed patch

Here's one way of making this better: adjusting the length field of the buffer and then returning it as the final string. This way we keep the slightly oversized buffer allocation, but don't allocate an extra StringImpl.
Comment 2 WebKit Commit Bot 2016-01-03 08:12:42 PST 
Comment on attachment 268134 [details]
Proposed patch

Clearing flags on attachment: 268134

Committed r194510: <http://trac.webkit.org/changeset/194510>
Comment 3 WebKit Commit Bot 2016-01-03 08:12:47 PST 
All reviewed patches have been landed.  Closing bug.
Comment 4 Simon Fraser (smfr) 2016-01-03 09:36:00 PST 
This broke the windows build:

C:\cygwin\home\buildbot\slave\win-debug\build\Source\WTF\wtf\text\StringBuilder.cpp(63): error C2248: 'WTF::StringImpl::m_length': cannot access private member declared in class 'WTF::StringImpl' [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WTF\wtf\WTF.vcxproj]
Comment 5 Simon Fraser (smfr) 2016-01-03 09:37:08 PST 
It also broke two tests:

Tests that failed:
  StringBuilderTest.Append
  StringBuilderTest.ToStringPreserveCapacity
Comment 6 Andreas Kling 2016-01-03 12:37:52 PST 
Reverted r194510 for reason:

Broke 2 API tests, no time to investigate right now

Committed r194519: <http://trac.webkit.org/changeset/194519>