[JSC] Optimize JSON.parse with small data by changing Identifier pool mechanism

• Component: New Bugs
• Status: RESOLVED FIXED

[Yusuke Suzuki]

[JSC] Optimize JSON.parse with small data by changing Identifier pool mechanism

[Yusuke Suzuki]

Created attachment 431622 [details]
Patch

[Mark Lam]

Comment on attachment 431622 [details]
Patch

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

r=me with some suggestions.

> Source/JavaScriptCore/ChangeLog:10
> +        if JSON.parse is invoked for small data. This patch changes this pool mechanism so that we do not allocate
> +        Identifiers until we really need.

I suggest adding a bit more detail:
"This patch changes this pool mechanism so that we do not allocate Identifiers until we really need." => "This patch changes this pool mechanism so that we do not waste effort allocating null Identifiers to pre-populate the recent identifiers pool. Instead, we now use a m_recentIdentifiersIndex uint8_t array to indicate whether there's a cached recent identifier for each given first character."

> Source/JavaScriptCore/runtime/LiteralParser.cpp:174
> +    if (indexPlusOne) {

I see.  The reason for using indexPlusOne instead if index is so that 0 can mean no entry.  I think this is worth a comment.

[Yusuke Suzuki]

Comment on attachment 431622 [details]
Patch

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

Thanks

>> Source/JavaScriptCore/ChangeLog:10
>> +        Identifiers until we really need.
> 
> I suggest adding a bit more detail:
> "This patch changes this pool mechanism so that we do not allocate Identifiers until we really need." => "This patch changes this pool mechanism so that we do not waste effort allocating null Identifiers to pre-populate the recent identifiers pool. Instead, we now use a m_recentIdentifiersIndex uint8_t array to indicate whether there's a cached recent identifier for each given first character."

Changed.

>> Source/JavaScriptCore/runtime/LiteralParser.cpp:174
>> +    if (indexPlusOne) {
> 
> I see.  The reason for using indexPlusOne instead if index is so that 0 can mean no entry.  I think this is worth a comment.

Added.

[Yusuke Suzuki]

Committed r278971 (238897@main): <https://commits.webkit.org/238897@main>

[Radar WebKit Bug Importer]

<rdar://problem/79429814>

[Yusuke Suzuki]

This improved JetStream2/FlightPlanner by 25% in some devices too!