1527822 - Consider inlining cross-realm native calls in IonBuilder

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P1)

Description

[Jan de Mooij [:jandem]]

We don't inline cross-realm scripted/native calls in IonBuilder, but there are many natives we could easily inline cross-realm (the obvious ones are the typedArray.length getter, Math functions, etc).

Comment 1

[Jan de Mooij [:jandem]]

Note to self: the realm check in IonBuilder::createThis is also a bit too pessimistic, many of the cases there are perfectly fine with a cross-realm function.

Comment 2

[Jan de Mooij [:jandem]]

As a start we should probably add the "abort if cross-realm" check to the inlining code for each native function, instead of checking it globally, and then we can incrementally remove the checks from the safe ones.

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #2)

As a start we should probably add the "abort if cross-realm" check to the inlining code for each native function, instead of checking it globally, and then we can incrementally remove the checks from the safe ones.

This isn't trivial because there's no reliable way to get the target realm from the callInfo passed to all the IonBuilder::inlineFoo methods. One option is to add a |Realm* targetRealm| argument to all of them. That might be nice anyway to remind people of the cross-realm case.

This will be pretty tedious to do but we should just get it over with now that we're fixing cross-realm issues.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1527822 - Allow inlining some cross-realm native calls in IonBuilder. r?anba!

This adds the basic infrastructure and uses it for some Math natives and the
Array constructor.

Comment 5

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #3)

This isn't trivial because there's no reliable way to get the target realm from the callInfo passed to all the IonBuilder::inlineFoo methods. One option is to add a |Realm* targetRealm| argument to all of them. That might be nice anyway to remind people of the cross-realm case.

I ended up implementing a less verbose option: I added a CanInlineCrossRealm function that's just a big switch-stament returning false for most natives. It's even more explicit and makes it easy to incrementally whitelist things in the future.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/deb3594da5b1
Allow inlining some cross-realm native calls in IonBuilder. r=anba

Comment 7

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/deb3594da5b1