Bug 202340

Summary: [JSC] Consider replacing ExecState with JSGlobalObject
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: fpizlo, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   

Yusuke Suzuki
Reported 2019-09-27 17:57:18 PDT
...
Attachments
Yusuke Suzuki
Comment 1 2019-09-27 17:59:57 PDT
After considering and discussing with Saam, I think this is a bit tricky. For host functions, we first need to get JSGlobalObject* to continue executing since many C++ function requires it. But getting JSGlobalObject* is not cheap. 1. Load Callee from ExecState 2. Load StructureID from Callee 3. Load Structure from StructureIDTable 4. Load GlobalObject from Structure Previously we defer this lookup until we actually need it. We just pass around ExecState*. And we call ExecState::lexicalGlobalObject only when we really need it. But if we pass around JSGlobalObject*, we first need to get it and I think this makes performance worse than now.
Yusuke Suzuki
Comment 2 2019-09-27 18:05:57 PDT
(In reply to Yusuke Suzuki from comment #1) > After considering and discussing with Saam, I think this is a bit tricky. > For host functions, we first need to get JSGlobalObject* to continue > executing since many C++ function requires it. But getting JSGlobalObject* > is not cheap. > > 1. Load Callee from ExecState > 2. Load StructureID from Callee > 3. Load Structure from StructureIDTable > 4. Load GlobalObject from Structure > > Previously we defer this lookup until we actually need it. We just pass > around ExecState*. And we call ExecState::lexicalGlobalObject only when we > really need it. > But if we pass around JSGlobalObject*, we first need to get it and I think > this makes performance worse than now. Another way is passing ExecState* and JSGlobalObject* to host native function. We can achieve that by embedding JSGlobalObject* pointer into native-function-call trampoline. But this causes another problem. Currently, we have stub directory in VM for this trampoline code. So only one code per VM per function is generated. And since # of functions are statically bound, these trampoline code increase only when we create multiple VMs. And even more, once global GC is done, we can share them per process. On the other hand, if we embed JSGlobalObject* to this trampoline, we need to create a trampoline per JSGlobalObject. If we create multiple frames, or navigates, then we get more and more trampolines. This trampoline code is a bit critical JIT code. We must succeed to generate this JIT code, otherwise, we cannot call this native host function. This is large difference from the usual JS JIT code, its generation can fail, and fall back to interpreter. Given that, I don't think creating trampolines per JSGlobalObject is good direction.
Yusuke Suzuki
Comment 3 2019-09-27 18:07:35 PDT
I'm now rather considering about just using the current way. We ensure that Callee is always small cell. And keep this invariant. When creating IsoSubspace, pass classInfo, and check this does not implement their own getCallData/getConstructData. If it is implemented, we can just disable sharing tier for this type. Then, we can keep things simple while introducing sharing tier.
Yusuke Suzuki
Comment 4 2019-09-27 19:38:43 PDT
(In reply to Yusuke Suzuki from comment #3) > I'm now rather considering about just using the current way. We ensure that > Callee is always small cell. And keep this invariant. > When creating IsoSubspace, pass classInfo, and check this does not implement > their own getCallData/getConstructData. If it is implemented, we can just > disable sharing tier for this type. Then, we can keep things simple while > introducing sharing tier. Talked with Saam. Another solution is putting VM& to TLS. A/B test for `ExecState::vm()` for LargeAllocation is telling that it may be possible that it has perf regression in Speedometer2 (0.54% with 98% probability). We could try TLS way instead.
Yusuke Suzuki
Comment 5 2019-11-20 01:14:18 PST
*** This bug has been marked as a duplicate of bug 202392 ***
Note You need to log in before you can comment on or make changes to this bug.