But, it is super costly if we ends up calling lexicalGlobalObject(). So, we need to put JSGlobalObject* in JSFunction/InternalFunction. And trampoline for them will get objects and pass them to the host-function. Then, the host-function signature is changed to `hostFunction(CallFrame*, JSGlobalObject*)`. CallFrame* is necessary since we still need to access thisValue, arguments etc. residing on the stack.
I think this is the simplest way given the followings 1. InternalFunction is not so many 2. sizeof(JSFunction) is 40. Increasing to 48 is OK.
<rdar://problem/55881584>
The patch becomes too large, I'll spawn part of it.
Created attachment 380709 [details] Patch
Created attachment 380719 [details] Patch
Created attachment 381157 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Created attachment 381160 [details] Patch
Created attachment 381162 [details] Patch
(In reply to Build Bot from comment #7) > Thanks for the patch. If this patch contains new public API please make sure > it follows the guidelines for new WebKit2 GTK+ API. See > http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API This is unrelated.
Created attachment 381163 [details] Patch
Created attachment 381166 [details] Patch
Created attachment 381172 [details] Patch
Created attachment 381173 [details] Patch
Created attachment 381179 [details] Patch
Created attachment 381229 [details] Patch
Created attachment 381246 [details] Patch
Comment on attachment 381246 [details] Patch Attachment 381246 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/13145189 New failing tests: fast/events/attribute-listener-cloned-from-frameless-doc-context-2.html fast/events/attribute-listener-extracted-from-frameless-doc-context-2.html fast/dom/HTMLObjectElement/object-as-frame.html
Created attachment 381259 [details] Archive of layout-test-results from ews213 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews213 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Created attachment 381275 [details] Patch
Created attachment 381282 [details] Patch
Created attachment 381287 [details] Patch
Created attachment 381365 [details] Patch
Comment on attachment 381365 [details] Patch Attachment 381365 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/13150254 New failing tests: apiTests
Created attachment 381431 [details] Patch
Created attachment 381484 [details] Patch
Created attachment 381488 [details] Patch
Created attachment 381490 [details] Patch
Created attachment 381492 [details] Patch
Comment on attachment 381492 [details] Patch r=me This patch makes me happy. Good luck landing it :P.
(In reply to Geoffrey Garen from comment #30) > Comment on attachment 381492 [details] > Patch > > r=me > > This patch makes me happy. Good luck landing it :P. This removes many of ExecState::vm() calls (we now call JSGlobalObject::vm() instead). And it allows us to use LargeAllocation for any JSCells, and then, it allows us to introduce lower tier of IsoSubspace allocation (similar to the thing done in IsoHeap). And... We start applying IsoSubspaces to all the JSObjects!
The JSC failure is due to my bug I added in the last patch (using COMPILER(GCC_OR_CLANG)). We should use `COMPILER(GCC_COMPATIBLE)`.
Created attachment 381507 [details] Patch
Another really nice feature of this patch is that it creates a clearer API to WebCore and WebKit programmers.
rs=me. I have a few comments from the parts that looked interesting: Can you explain the m_functionConstructor/m_regExpConstructor changes in JSGlobalObject in the ChangeLog? I'm guessing it was for a bug somewhere that was using the wrong value or something. Can you add some comment to WebCore's ChangeLog explaining why we are making this change? I also didn't know about `__builtin_frame_address`, which is pretty cool!
(In reply to Geoffrey Garen from comment #34) > Another really nice feature of this patch is that it creates a clearer API > to WebCore and WebKit programmers. Yeah, we decouple lexical JSGlobalObject from CallFrame, making CallFrame actually just a CallFrame :)
(In reply to Keith Miller from comment #35) > rs=me. I have a few comments from the parts that looked interesting: > > Can you explain the m_functionConstructor/m_regExpConstructor changes in > JSGlobalObject in the ChangeLog? I'm guessing it was for a bug somewhere > that was using the wrong value or something. It is just used in some places to remove CallFrame* dependencies :) I'll note it. > > Can you add some comment to WebCore's ChangeLog explaining why we are making > this change? Yeah :) > > I also didn't know about `__builtin_frame_address`, which is pretty cool!
Created attachment 381517 [details] Patch
Created attachment 381523 [details] Patch
Hmm, I'm attempting to make 32bit JIT built, but seems that EWS for these builds is too slow, and I cannot get the result. I'll land this first and check the buildbot status after that, otherwise, this patch needs to be rebaselined forever...
Committed r251425: <https://trac.webkit.org/changeset/251425>
(In reply to Yusuke Suzuki from comment #40) > Hmm, I'm attempting to make 32bit JIT built, but seems that EWS for these > builds is too slow, and I cannot get the result. I'll land this first and > check the buildbot status after that, otherwise, this patch needs to be > rebaselined forever... https://bugs.webkit.org/show_bug.cgi?id=203242
LGTM too
I'll do follow-up patch to improve things more. 1. Add more API tests using vm.topCallFrame related things from C++ world 2. Add assertions which makes DECLARE_CALL_FRAME safer
Committed r251459: <https://trac.webkit.org/changeset/251459>
(In reply to Yusuke Suzuki from comment #44) > I'll do follow-up patch to improve things more. > > 1. Add more API tests using vm.topCallFrame related things from C++ world > 2. Add assertions which makes DECLARE_CALL_FRAME safer Do this in https://bugs.webkit.org/show_bug.cgi?id=203274
Another stabilizing patch https://bugs.webkit.org/show_bug.cgi?id=203285
(In reply to Yusuke Suzuki from comment #40) > Hmm, I'm attempting to make 32bit JIT built, but seems that EWS for these > builds is too slow, and I cannot get the result. I'll land this first and > check the buildbot status after that, otherwise, this patch needs to be > rebaselined forever... (bug Reopened) It seems that the EWS for are experimenting troubles. Unfortunately, it looks like the https://trac.webkit.org/r251425 is breaking the JSC test suite in https://build.webkit.org/waterfall?category=misc right now. Hopefully you could take a look into this.
(In reply to Pablo Saavedra from comment #48) > (In reply to Yusuke Suzuki from comment #40) > > Hmm, I'm attempting to make 32bit JIT built, but seems that EWS for these > > builds is too slow, and I cannot get the result. I'll land this first and > > check the buildbot status after that, otherwise, this patch needs to be > > rebaselined forever... > > (bug Reopened) > > It seems that the EWS for are experimenting troubles. Unfortunately, it > looks like the https://trac.webkit.org/r251425 is breaking the JSC test > suite in https://build.webkit.org/waterfall?category=misc right now. > > Hopefully you could take a look into this. The main issue from this patch is that there's a missing call to JIT::prepareCallOperation(VM&) in DFG::OSRExit::emitRestoreArguments() which would properly adjust the vm.topCallFrame value. This trips up OSR exits, resulting in crashes. Besides that, there are other places in DFG where a call operation doesn't invoke prepareCallOperation(), but without any consequences. I'd post a patch for it already, but r251468 broke things in a different way, so this is just a heads-up about the solution.
(In reply to Zan Dobersek from comment #49) > (In reply to Pablo Saavedra from comment #48) > > (In reply to Yusuke Suzuki from comment #40) > > > Hmm, I'm attempting to make 32bit JIT built, but seems that EWS for these > > > builds is too slow, and I cannot get the result. I'll land this first and > > > check the buildbot status after that, otherwise, this patch needs to be > > > rebaselined forever... > > > > (bug Reopened) > > > > It seems that the EWS for are experimenting troubles. Unfortunately, it > > looks like the https://trac.webkit.org/r251425 is breaking the JSC test > > suite in https://build.webkit.org/waterfall?category=misc right now. > > > > Hopefully you could take a look into this. > > The main issue from this patch is that there's a missing call to > JIT::prepareCallOperation(VM&) in DFG::OSRExit::emitRestoreArguments() which > would properly adjust the vm.topCallFrame value. This trips up OSR exits, > resulting in crashes. > > Besides that, there are other places in DFG where a call operation doesn't > invoke prepareCallOperation(), but without any consequences. > > I'd post a patch for it already, but r251468 broke things in a different > way, so this is just a heads-up about the solution. Thanks. I'm doing a bit mechanical way to find missing prepareCallOperation(VM&) in https://bugs.webkit.org/show_bug.cgi?id=203285 Can you try the part of this patch in your local 32bit JIT machine and test it?
32bit issues will be fixed in bug 203285
*** Bug 134932 has been marked as a duplicate of this bug. ***
*** Bug 202340 has been marked as a duplicate of this bug. ***
Committed 262166@main (7a682381645f): <https://commits.webkit.org/262166@main> Reviewed commits have been landed. Closing PR #11084 and removing active labels.