Closed Bug 907201 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: till, Assigned: till)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Attachment #792858 - Flags: review?(jdemooij)
Blocks: 885526
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.
Attachment #792858 - Flags: review?(jdemooij)
+cc [Memshrink] so we keep an eye on the memory use here.
Whiteboard: [Memshrink]
Memory consumption for TI in chrome compartments has been a problem in the past, but things have changed quite a bit since then.
Whiteboard: [Memshrink]
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.
Attachment #793431 - Flags: review?(jdemooij)
Attachment #792858 - Attachment is obsolete: true
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!
Attachment #793431 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5052eddee62

Thanks for the quick reviews!
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/a5052eddee62
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Depends on: 909404
Blocks: 909404
No longer depends on: 909404
Blocks: 911970
You need to log in before you can comment on or make changes to this bug.