OdinMonkey: Use backtracking allocator for asmjs style code

RESOLVED FIXED in mozilla31

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: h4writer, Assigned: h4writer)

Tracking

unspecified
mozilla31
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(relnote-firefox 31+)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
The backtracking allocator generates better code, so we should eventually start using it. There is only downside, it takes longer than LSRA to find a solution. 

For regular IonMonkey we are also looking to enable it. Though we can't by default since the increased compilation time results in being stuck in baseline longer. We have a burst of compilation requests at usecount 1000. Increasing the compilation time a bit, makes the last scheduled compile starting much later. Therefore we are looking to enable it as a second optimization level. For normal code it does see some decreases in performance (i.e. richards) that need to get sorted out, but looks promising.

For asm.js style code it seems always an improvement to enable backtracking. Since we compile in parallel we loose less time and only during compilation (before running). I see an improvement of 45000 to 50000 on octane-zlib.

@Luke: I know you carefully measured the compilation process for odinmonkey. Would it be terrible to increase compilation time for better code? And can you quantify how much of a decrease in compilation time would hurt us?
(Assignee)

Comment 1

4 years ago
Created attachment 8391113 [details] [diff] [review]
Use backtracking on AsmJS style code
Assignee: nobody → hv1989
Attachment #8391113 - Flags: review?(luke)
(Assignee)

Comment 2

4 years ago
According to sunfish bug 968931 is the only remaining known bug for backtracking. So we need to fix that first, before we can flip the switch.
(From what I see it involves VMCalls, which I think is done differently on asm.js. So maybe we don't even have to fix it for asm.js release actually).
Depends on: 968931

Comment 3

4 years ago
I've measured backtracking's effect on compile time several times and it's always been a tolerable increase so I'm in favor of enabling backtracking once the known bugs are fixed.

There will almost certainly be bugs lurking that will be discovered by fuzzers or demos when asm.js is enabled so let's land this at the beginning of a release cycle (which is fortunately soon).  Also, we should verify that the major demos aren't broken which I can do.

Comment 4

4 years ago
Comment on attachment 8391113 [details] [diff] [review]
Use backtracking on AsmJS style code

r+ with the above caveats
Attachment #8391113 - Flags: review?(luke) → review+
(Assignee)

Comment 5

4 years ago
Comment on attachment 8391113 [details] [diff] [review]
Use backtracking on AsmJS style code

We want to land this as soon as possible. But it would be good to have some fuzzing on this patch, since it is a major change to use backtracking. Now if possible you can guide the fuzzers to only look at code that has "use asm"
Attachment #8391113 - Flags: feedback?(gary)
(Assignee)

Updated

4 years ago
Attachment #8391113 - Flags: feedback?(choller)
Depends on: 988022
Comment on attachment 8391113 [details] [diff] [review]
Use backtracking on AsmJS style code

Nothing found on one core on Linux, overnight.
Attachment #8391113 - Flags: feedback?(gary) → feedback+
Comment on attachment 8391113 [details] [diff] [review]
Use backtracking on AsmJS style code

I would have to setup a specific fuzzing environment to test this (ASM.js specifically), that would probably take a while and jsfunfuzz covers the ASM.js parts of the engine a lot better, so I'd say go ahead with gkw's feedback+ unless you think that's not sufficient :)
Attachment #8391113 - Flags: feedback?(choller)
(Assignee)

Comment 8

4 years ago
Thanks. I've just pushed.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf4d2479f33e

Comment 9

4 years ago
Emscripten test suite looks ok, as well as a few hours of fuzzing.
https://hg.mozilla.org/mozilla-central/rev/bf4d2479f33e
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
(Assignee)

Comment 11

3 years ago
Not sure how this works, but I want to nominate this for the release notes. (It is the intention to have a blogpost explaining this change on https://blog.mozilla.org/javascript/ before it hits aurora)
relnote-firefox: --- → ?
Keywords: relnote
That sounds quite technical to me but I understand if you want to highlight your work on performance in Firefox.
Anyway, added in the release note for 31.
relnote-firefox: ? → 31+
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.