826741 - IonMonkey: Enable backtracking allocator by default

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Brian Hackett [Laid off!]]

With the patch in bug 826734, Ion does a little better on x86 benchmarks when using the backtracking allocator instead of lsra (1-2% v8/kraken improvement, smaller for ss) and the backtracking allocator significantly reduces the amount of spill code executed for both x86 and x64.  This improvement isn't gigantic, but will become larger as the benchmarks are optimized further and using a better allocator will be important for autotranslated code (emscripten/asm.js).  Enabling the backtracking allocator by default can be done after taking a couple steps:

- Examine ARM performance.  The allocator hasn't had any ARM tuning and with the current state of the ARM assembler it isn't possible I think to easily examine the generated ARM code and make improvements.  If the allocator does not do a good job on ARM out of the box, it could still be the default for x86/x64 until the assembler is fixed and regalloc issues diagnosed.

- Fuzzing.  This can wait until after bug 826734 lands.

Comment 1

[Christian Holler (:decoder)]

Working on fuzzing this now (mozilla-central with --ion-regalloc=backtracking).

Comment 2

[Christian Holler (:decoder)]

This configuration/flag is now being tested continuously on fuzzer-linux7.

Comment 3

[Christian Holler (:decoder)]

Nice, this was meant for bug 864919.

Comment 4

[Jan de Mooij [:jandem]]

I asked Hannes to add a Backtracking line on AWFY for the Mac shell builds, so hopefully we will have some useful numbers soon. Once we have that we can decide which tests need to be fixed before we can enable BT by default.

Locally I no longer see the Octane-Gameboy regression (bug 1063603). BT is a bit faster there now so that's great, but overall it's still slower on Octane.

Comment 5

[Brian Hackett [Laid off!]]

Attachment: patch

Not all regressions with the backtracking allocator have been fixed (mainly bug 1134510), but its performance on octane and the other benchmarks we track is pretty much in line with LSRA, and it would be good to turn it on by default after the merge tomorrow, before doing more work to improve it in the next release cycle.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8b1fd2a9b9e

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 8568114 [details] [diff] [review]
patch

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

Awesome.

Comment 7

[Brian Hackett [Laid off!]]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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