Bug 253412 - Reduce unnecessary work in VMEntryScope to speed up first VM entry.
Summary: Reduce unnecessary work in VMEntryScope to speed up first VM entry.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-03-05 09:04 PST by Mark Lam
Modified: 2023-03-06 01:16 PST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2023-03-05 09:04:20 PST
1. VMEntryScope entry and exit current check for the need to do a lot of work that are usually not needed.
   Instead of doing all these checks, we introduce a new VM::EntryScopeService concept where if one of these checks are needed, their sources will request an EntryScopeService.  With this, the VMEntryScope will only check if any such services have been requested (with a single bitfield), and can will only incur the cost of the check if at least one such services have been requested.
   The only exception to using these service requests is the resetting of the VM DateCache.  That is dependent on a global lastTimeZoneID because there is one DateCache per VM, and there can be multiple VMs.  Hence, lastTimeZoneID needs to be updated in an atomic way.  We also don't want the EntryScope service request to require a lock of any sort, nor have to deal with iterating all VMs to request a DateCache reset.  So, we'll just do a hasTimeZoneChange() check instead in addition to the service request check on entry.

2. Moved the servicing of the "EntryScopeService"s out to VM::executeEntryScopeServicesOnEntry() and VM::executeEntryScopeServicesOnExit() instead of keeping them inlined in VMEntryScope.
   This appears to have improved the microbenchmark result by ~2-4%.  Might be due to improved cache locality from moving the rare case out.

3. Also removed VM::clearScratchBuffers().
   We already always set the ScratchBuffer activeLength back to 0 after use.  So, clearScratchBuffers() was sure waste.
   Converted it into assertScratchBuffersAreEmpty() instead to enforce this invariant.

   Removing clearScratchBuffers() is responsible for almost all of the gains.

4. Rename VM::m_terminationInProgress to VM::m_hasTerminationRequest to better describe what it represents.  I wrote this code, and I found even myself confused by it.  So, I'm renaming it for clarity, and updating the comment about its treatment on VM exit for added clarity as well.

5. Added cpp-to-js-cached-call.js and cpp-to-js-call-as-first-entry.js microbenchmarks which tests repeated calls using CachedCall, and repeated calls at the first entry boundary.

The microbenchmark results are all follows,  As expected, only first entry is improved by this change.

                                          base                      new                                        
    
    cpp-to-js-call-as-first-entry       96.8583+-0.1895     ^     75.9927+-0.6285        ^ definitely 1.2746x faster
    cpp-to-js-call                      69.6459+-0.1565           69.5432+-0.0586        
    cpp-to-js-cached-call               79.8228+-0.5376     ?     80.4139+-0.6115        ?
Comment 1 Radar WebKit Bug Importer 2023-03-05 09:04:42 PST
<rdar://problem/106258354>
Comment 2 Mark Lam 2023-03-05 09:08:38 PST
Pull request: https://github.com/WebKit/WebKit/pull/11085
Comment 3 EWS 2023-03-06 01:16:53 PST
Committed 261260@main (3b78627af7fa): <https://commits.webkit.org/261260@main>

Reviewed commits have been landed. Closing PR #11085 and removing active labels.