Bug 227013 - Move setting of scratch buffer active lengths to the runtime functions.
Summary: Move setting of scratch buffer active lengths to the runtime functions.
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: 2021-06-14 22:41 PDT by Mark Lam
Modified: 2021-07-04 00:15 PDT (History)
6 users (show)

See Also:


Attachments
proposed patch. (31.52 KB, patch)
2021-06-14 22:46 PDT, Mark Lam
keith_miller: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2021-06-14 22:41:20 PDT
We previously emit JIT'ed code to set and unset the ScratchBuffer active length around calls into C++ runtime functions.  This was needed because the runtime functions may allow GC to run, and GC needs to be able to scan the values stored in the ScratchBuffer.

Instead of emitting JIT code to set and unset the active length, we can do it in the runtime function.
Comment 1 Radar WebKit Bug Importer 2021-06-14 22:41:49 PDT
<rdar://problem/79325068>
Comment 2 Mark Lam 2021-06-14 22:46:23 PDT
Created attachment 431404 [details]
proposed patch.
Comment 3 Keith Miller 2021-06-15 08:34:13 PDT
Comment on attachment 431404 [details]
proposed patch.

r=me. How do we get the scratch register buffer when writing to it in C++? If it’s not via the m_scratchBuffer member maybe we should have an assert that it’s set?
Comment 4 Keith Miller 2021-06-15 08:34:27 PDT
Comment on attachment 431404 [details]
proposed patch.

r=me. How do we get the scratch register buffer when writing to it in C++? If it’s not via the m_scratchBuffer member maybe we should have an assert that it’s set?
Comment 5 Mark Lam 2021-06-15 09:04:17 PDT
(In reply to Keith Miller from comment #4)
> Comment on attachment 431404 [details]
> proposed patch.
> 
> r=me. 

Thanks.

> How do we get the scratch register buffer when writing to it in C++?
> If it’s not via the m_scratchBuffer member maybe we should have an assert
> that it’s set?

```
inline ActiveScratchBufferScope::ActiveScratchBufferScope(VM& vm, size_t activeScratchBufferSizeInJSValues)
    : m_scratchBuffer(vm.scratchBufferForSize(activeScratchBufferSizeInJSValues * sizeof(EncodedJSValue)))
{
    // Tell GC mark phase how much of the scratch buffer is active during the call operation this scope is used in.
    if (m_scratchBuffer)
        m_scratchBuffer->u.m_activeLength = activeScratchBufferSizeInJSValues * sizeof(EncodedJSValue);
}
```

We call vm.scratchBufferForSize() with the exact same size that the JIT calls it with.  Hence, we'll always get the same ScratchBuffer.

As for accesses to the scratch buffer, if the C++ runtime used to write stuff to the ScratchBuffer, it does so the same way as before i.e. the JIT code would have passed in the buffer as a pointer.  I'm not changing anything there.
Comment 6 Mark Lam 2021-06-15 09:10:02 PDT
Landed in r278875: <http://trac.webkit.org/r278875>.