Closed Bug 909389 Opened 6 years ago Closed 5 years ago

IonMonkey: Enable try-catch compilation

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(2 files)

Attached patch PatchSplinter Review
The fuzzers have been testing this for at least a week and once bug 908625 lands we should be able to turn on Ion try-catch compilation by default.

This patch is very small so that we can easily back it out if needed. I can remove the pref after this has been in nightlies for a week or so.
Attachment #795499 - Flags: review?(kvijayan)
Comment on attachment 795499 [details] [diff] [review]
Patch

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

Yay!
Attachment #795499 - Flags: review?(kvijayan) → review+
> This patch is very small so that we can easily back it out if needed. I can
> remove the pref after this has been in nightlies for a week or so.

I assume the CLI flag for this will become a no-op rather than being completely removed? (for backward compatibility / bisection reasons)
Can we perhaps add support for invisible flags, then? I really like this way of landing new stuff, but if it means that `js -h` gets cluttered up more and more with no-op flags, that's rather annoying, I think.
Invisible flags work for us too, we don't really need them in `js -h`.
It looks like this was a 1-13% win on Octane-gameboy. Strangely enough, it's more of a win on x64 and with GGC (probably because we're not discarding JIT code as often).
What's missing for [leave open] to remove? Is it the flags missing?
(In reply to Florian Bender from comment #9)
> What's missing for [leave open] to remove? Is it the flags missing?

Yup, I just have to make the pref a no-op and remove some unnecessary code. Will get to it soon.
Depends on: 925067
Depends on: 970252
Attachment #8496128 - Flags: review?(kvijayan)
Gary, can you remove the --ion-compile-try-catch flag from the fuzzing harness? It's been a no-op for more than a year.
Flags: needinfo?(gary)
(In reply to Jan de Mooij [:jandem] from comment #12)
> Gary, can you remove the --ion-compile-try-catch flag from the fuzzing
> harness? It's been a no-op for more than a year.

It's already been removed, apparently some time ago. Thanks for the headsup though!
Flags: needinfo?(gary)
Attachment #8496128 - Flags: review?(kvijayan) → review+
https://hg.mozilla.org/mozilla-central/rev/d997824b3cf5
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Finally! :) (pun intended)

That's something for "Firefox xy for developers" (https://developer.mozilla.org/en-US/Firefox/Releases/35), I think. --> dev-doc-needed
Keywords: dev-doc-needed
(In reply to Florian Bender from comment #16)
> That's something for "Firefox xy for developers"
> (https://developer.mozilla.org/en-US/Firefox/Releases/35), I think. -->
> dev-doc-needed

This landed more than a year ago. The last patch just removed the pref but it was enabled by default anyway :)
Keywords: dev-doc-needed
Oh yeah, cool, should have read the patch!
You need to log in before you can comment on or make changes to this bug.