Closed
Bug 1471844
Opened 6 years ago
Closed 6 years ago
Implement realm switching for optimized DOM calls in Ion
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(1 file)
(deleted),
patch
|
luke
:
review+
bzbarsky
:
feedback+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•6 years ago
|
||
This should have no perf overhead for same-realm calls.
The patch adds a |global| property to the JS shell's FakeDOMObject so we can test this.
Attachment #8988438 -
Flags: review?(luke)
Attachment #8988438 -
Flags: feedback?(bzbarsky)
Updated•6 years ago
|
Attachment #8988438 -
Flags: review?(luke) → review+
Comment 2•6 years ago
|
||
Comment on attachment 8988438 [details] [diff] [review]
Implement cx->realm switching for optimized DOM calls in Ion
Worth documenting that in visitCallDOMNative we're just using argJSContext as a scratch register in the cross-realm case for the switch, because it hasn't had the JSContext placed in it yet, but maybe that's obvious? I guess the important thing is that we don't put that code after the loadJSContext call, and that's the part that really needs to be clear.
What happens if the callee throws? We will hit the masm.branchIfFalseBool and branch, never hitting the code output by the masm.switchToRealm(gen->realm->realmPtr(), ReturnReg) bit. Do we not need to restore the old Realm in that case? Similar question about the getter and setter cases.
Attachment #8988438 -
Flags: feedback?(bzbarsky) → feedback+
Assignee | ||
Comment 3•6 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #2)
> Worth documenting that in visitCallDOMNative we're just using argJSContext
> as a scratch register in the cross-realm case for the switch, because it
> hasn't had the JSContext placed in it yet, but maybe that's obvious?
Sure, I can add a comment; might avoid some confusion.
> What happens if the callee throws? We will hit the masm.branchIfFalseBool
> and branch, never hitting the code output by the
> masm.switchToRealm(gen->realm->realmPtr(), ReturnReg) bit. Do we not need
> to restore the old Realm in that case?
We'll enter the JIT exception handler where we set cx->realm to the frame's realm:
https://searchfox.org/mozilla-central/rev/d2966246905102b36ef5221b0e3cbccf7ea15a86/js/src/jit/JitFrames.cpp#676-678
Thanks for the quick feedback!
Comment 4•6 years ago
|
||
> We'll enter the JIT exception handler where we set cx->realm to the frame's realm:
Aha. Might also be worth commenting.
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b715408b60b
Implement cx->realm switching for optimized DOM calls in Ion. f=bz r=luke
Assignee | ||
Comment 6•6 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #4)
> Might also be worth commenting.
Done.
Comment 7•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•