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)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(1 file)

No description provided.
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)
Attachment #8988438 - Flags: review?(luke) → review+
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+
(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!
> 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
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #4) > Might also be worth commenting. Done.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: