Bug 194583 – Cache the results of BytecodeGenerator::getVariablesUnderTDZ

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Saam Barati]

To speed up repeated computations

[Radar WebKit Bug Importer]

<rdar://problem/48028140>

[Saam Barati]

Created attachment 361900 [details]
patch

[Saam Barati]

Ugh, our bytecode-cache tests are slightly wrong. It expects that scripts never change -- however, my test is precisely a script that changes on each invocation.

[EWS Watchlist]

Comment on attachment 361900 [details]
patch

Attachment 361900 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/11132300

New failing tests:
microbenchmarks/cache-get-variables-under-tdz-in-bytecode-generator.js.bytecode-cache
apiTests

[Yusuke Suzuki]

Comment on attachment 361900 [details]
patch

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

r=me with one suggestion.

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:1317
> +        uint64_t m_tdzStackEpoch { 1 };
> +        uint64_t m_cachedVariablesUnderTDZEpoch { 0 };
> +        CompactVariableMap::Handle m_cachedVariablesUnderTDZ;

Why not clearing m_cachedVariablesUnderTDZ when changing tdz stack instead of tracking the change with epoch? (Adding `explicit operator bool()` to CompactVariableMap::Handle, and clear the cached handle instead of incrementing epoch).

[Saam Barati]

Comment on attachment 361900 [details]
patch

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

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:1317
>> +        CompactVariableMap::Handle m_cachedVariablesUnderTDZ;
> 
> Why not clearing m_cachedVariablesUnderTDZ when changing tdz stack instead of tracking the change with epoch? (Adding `explicit operator bool()` to CompactVariableMap::Handle, and clear the cached handle instead of incrementing epoch).

Sounds good. Not sure why I went with a more complicated implementation. This is much nicer.

[Saam Barati]

Created attachment 362073 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 362073 [details]
patch for landing

Clearing flags on attachment: 362073

Committed r241571: <https://trac.webkit.org/changeset/241571>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.