Bug 73457 – 8 bit string work slows down Kraken json-stringify-tinderbox

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

It appears that json-stringify-tinderbox slows down in part due to StringBuilder converting 8 bit string to 16 bit strings.

[Michael Saboff]

Created attachment 117258 [details]
Patch

[Michael Saboff]

Forgot to mention that this change eliminates the slowdown to json-stringify-tinderbox.  It does introduce a 4.4% slowdown to json-parse-financial.  That will be addressed in a subsequent patch to StringImpl::equal.

The attached proposed patch along with the proposed StringImpl::equal give Kraken a slight improvement (.5% using the arithmetic mean and 1% using the geometric mean) when compared to r100509 using bencher.

[Early Warning System Bot]

Comment on attachment 117258 [details]
Patch

Attachment 117258 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10704273

[Gyuyoung Kim]

Comment on attachment 117258 [details]
Patch

Attachment 117258 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10697288

[Michael Saboff]

Created attachment 117266 [details]
Patch updated with StringBuilder::characters() named correctly

The prior performance comments hold for this patch.

[WebKit Review Bot]

Comment on attachment 117266 [details]
Patch updated with StringBuilder::characters() named correctly

Attachment 117266 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10696506

[Michael Saboff]

Created attachment 117457 [details]
Patch with speculative fix for chromium linux

[Filip Pizlo]

Can you include the perf effect in the ChangeLog, and post the results here?

Makes it easier to track things down in case somewhere there is a slow-down.

[Michael Saboff]

(In reply to comment #8)
> Can you include the perf effect in the ChangeLog, and post the results here?
> 
> Makes it easier to track things down in case somewhere there is a slow-down.

Added the following to the ChangeLog:

        This change eliminates 5% of the 7% slowdown to json-stringify-tinderbox.
        This change introduces a 4.8% slowdown to json-parse-financial.
        Both of these remaining slowdowns will be addressed in a subsequent
        patch to StringImpl::equal.

[Sam Weinig]

Comment on attachment 117457 [details]
Patch with speculative fix for chromium linux

View in context: https://bugs.webkit.org/attachment.cgi?id=117457&action=review

> Source/JavaScriptCore/wtf/text/StringBuilder.cpp:156
> +    else if (m_buffer->hasOneRef())
> +            m_buffer = StringImpl::reallocate(m_buffer.release(), requiredLength, m_bufferCharacters16);

Indentation issue.

[Michael Saboff]

Created attachment 117697 [details]
Patch with fix for Win build failure

This also includes the requested performance comment and indentation fix.

[Michael Saboff]

Committed r102017: <http://trac.webkit.org/changeset/102017>

[Ryosuke Niwa]

It appears that this patch regressed StringBuilder:
http://build.webkit.org/builders/Chromium%20Linux%20Release%20%28Tests%29/builds/26827/steps/run-api-tests/logs/stdio

[ RUN      ] StringBuilderTest.Append
Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp:53: Failure
Value of: String(builder.characters(), builder.length())
  Actual: 0123456789abcd7
Expected: String(expected)
Which is: 0123456789abcdefg
Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp:53: Failure
Value of: String(builder.characters(), builder.length())
  Actual: 0123456789abcd7
Expected: String(expected)
Which is: 0123456789abcdefg
Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp:53: Failure
Value of: String(builder.characters(), builder.length())
  Actual: 0123456789abcd7
Expected: String(expected)
Which is: 0123456789abcdefg#
Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp:53: Failure
Value of: String(builder.characters(), builder.length())
  Actual: 0123456789abcd7
Expected: String(expected)
Which is: 0123456789abcdefg#
Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp:53: Failure
Value of: String(builder.characters(), builder.length())
  Actual: 0123456789abcdefg#0123456789abcd7
Expected: String(expected)
Which is: 0123456789abcdefg#0123456789abcdefg#XYZ
[  FAILED  ] StringBuilderTest.Append (1 ms)
[----------] 1 test from StringBuilderTest (1 ms total)

[ RUN      ] StringBuilderTest.ToStringPreserveCapacity
Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp:153: Failure
Value of: string1
  Actual: 0123456789abcdefghijklmnopqrstuvwxyz
Expected: String("0123456789abcdefghijklmnopqrstuvwxyzABCDEF")
Which is: 0123456789abcdefghijklmnopqrstuvwxyzABCDEF
[  FAILED  ] StringBuilderTest.ToStringPreserveCapacity (0 ms)