Bug 165857 - [JSC] Optimize Kraken stringify
Summary: [JSC] Optimize Kraken stringify
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-14 10:23 PST by Yusuke Suzuki
Modified: 2016-12-15 02:29 PST (History)
7 users (show)

See Also:


Attachments
Patch (2.67 KB, patch)
2016-12-14 10:25 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (2.59 KB, patch)
2016-12-14 10:41 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (4.88 KB, patch)
2016-12-15 00:25 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (5.73 KB, patch)
2016-12-15 00:50 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2016-12-14 10:23:46 PST
In Kraken's stringify test, roughly 30% of time is used by StringBuilder::appendQuotedJSONString.
I think we have room for optimization in this function.

I have a patch that shows 1% performance improvement in Linux box, but I need to test it with macOS clang.
Comment 1 Yusuke Suzuki 2016-12-14 10:25:11 PST
Created attachment 297100 [details]
Patch

WIP: Linux box shows 1% improvement
Comment 2 Yusuke Suzuki 2016-12-14 10:41:34 PST
Created attachment 297101 [details]
Patch

WIP: Linux box shows 4.6% improvement
Comment 3 Yusuke Suzuki 2016-12-15 00:25:07 PST
Created attachment 297179 [details]
Patch
Comment 4 Yusuke Suzuki 2016-12-15 00:50:33 PST
Created attachment 297183 [details]
Patch
Comment 5 Darin Adler 2016-12-15 01:18:35 PST
Comment on attachment 297183 [details]
Patch

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

> Source/WTF/wtf/text/StringBuilder.cpp:429
> +        if (LIKELY(character != '"' && character != '\\' && character > 0x1F)) {

For 8-bit characters can we make it even faster with a 256 byte array?
Comment 6 Yusuke Suzuki 2016-12-15 02:02:43 PST
Comment on attachment 297183 [details]
Patch

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

>> Source/WTF/wtf/text/StringBuilder.cpp:429
>> +        if (LIKELY(character != '"' && character != '\\' && character > 0x1F)) {
> 
> For 8-bit characters can we make it even faster with a 256 byte array?

I've created additional two versions.

1. Bit packed array version.

451         static constexpr const uint64_t masks[4] = {
452             0xfffffffb00000000ULL,
453             0xffffffffefffffffULL,
454             0xffffffffffffffffULL,
455             0xffffffffffffffffULL,
456         };
457
458         if (masks[character >> 6]  & (1ULL << (character & 0b111111))) {
459             *output++ = character;
460             continue;
461         }

2. 256 bytes array version.

The result on MBP.

                                   256array                   branch                   packed    

json-stringify-tinderbox        27.237+-0.212             27.157+-0.196      ?      27.239+-0.213

Overall, they show the same results. I think this is because branch prediction works well.
So I think the current version is OK.

BTW, if we put it on the Aarch64 cores, it could show the different results because Intel cores has sophisticated branch predictions.
Comment 7 WebKit Commit Bot 2016-12-15 02:29:31 PST
Comment on attachment 297183 [details]
Patch

Clearing flags on attachment: 297183

Committed r209858: <http://trac.webkit.org/changeset/209858>
Comment 8 WebKit Commit Bot 2016-12-15 02:29:36 PST
All reviewed patches have been landed.  Closing bug.