907201 - Enable activating TI and IonMonkey for chrome scripts via about:config

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Till Schneidereit [:till]]

As discussed, Shumway (and other addons, I'm sure) wants ion support in chrome code. The patch here makes it possible to activate that and TI, but leaves both disabled by default.

Comment 1

[Till Schneidereit [:till]]

Attachment: Enable activating TI and IonMonkey for chrome scripts via about:config

Renames js.options.typeinference to js.options.typeinference.content, adds js.options.typeinference.chrome and js.options.ion.chrome, and wires up their interpretation correctly.

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 792858 [details] [diff] [review]
Enable activating TI and IonMonkey for chrome scripts via about:config

Review of attachment 792858 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine, but we also have to update dom/workers/RuntimeService.cpp..

::: dom/base/nsJSEnvironment.cpp
@@ +724,5 @@
>    // XXX components be covered by the chrome pref instead of the content one?
>    nsCOMPtr<nsIDOMWindow> contentWindow(do_QueryInterface(global));
>    nsCOMPtr<nsIDOMChromeWindow> chromeWindow(do_QueryInterface(global));
>  
> +  bool useTypeInference = Preferences::GetBool(chromeWindow || !contentWindow ?

I know the existing baseline code has it too (and I probably got it from the JM prefs..), but can you add parentheses around the || here and below? Makes it a bit more readable.

Comment 3

[Jet Villegas (inactive)]

+cc [Memshrink] so we keep an eye on the memory use here.

Comment 4

[Nicholas Nethercote [inactive]]

Memory consumption for TI in chrome compartments has been a problem in the past, but things have changed quite a bit since then.

Comment 5

[Till Schneidereit [:till]]

Attachment: Enable activating TI and IonMonkey for chrome scripts via about:config. v2

With moar RuntimeService.cpp changes.

Interestingly, chrome workers already used the ion.chrome preference, so that would have been usable, if it weren't for TI not being activated. Also, man RuntimeService.cpp handles this way nicer than nsJSEnvironment.cpp does.

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 793431 [details] [diff] [review]
Enable activating TI and IonMonkey for chrome scripts via about:config. v2

Review of attachment 793431 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

Comment 7

[Till Schneidereit [:till]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a5052eddee62

Thanks for the quick reviews!

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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