IonMonkey: Enable backtracking allocator by default

RESOLVED FIXED in Firefox 39

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
5 years ago
9 months ago

People

(Reporter: bhackett, Assigned: bhackett)

Tracking

(Depends on: 3 bugs)

Other Branch
mozilla39
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Depends on: 826734
Working on fuzzing this now (mozilla-central with --ion-regalloc=backtracking).
Depends on: 864919
Depends on: 864966
Depends on: 865024
Depends on: 865031
Depends on: 900437
This configuration/flag is now being tested continuously on fuzzer-linux7.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Nice, this was meant for bug 864919.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

4 years ago
Depends on: 914141

Updated

4 years ago
Depends on: 921263
Depends on: 880091
Depends on: 884630
Depends on: 900683
Depends on: 875656
Depends on: 868206
Depends on: 891156
Depends on: 873660
Depends on: 906858
Depends on: 880512
Depends on: 885885
Depends on: 890414
Depends on: 923799

Updated

4 years ago
Depends on: 931487

Updated

4 years ago
Depends on: 932465

Updated

4 years ago
Depends on: 936891

Updated

4 years ago
Depends on: 924920

Updated

4 years ago
Depends on: 926904

Updated

4 years ago
Depends on: 938468

Updated

4 years ago
Blocks: 939893

Updated

4 years ago
No longer blocks: 939893
Depends on: 939893

Updated

4 years ago
Depends on: 948838
(Assignee)

Updated

4 years ago
Depends on: 983752

Updated

3 years ago
Depends on: 1030378
Assignee: general → nobody

Updated

3 years ago
Depends on: 1063603

Updated

3 years ago
Depends on: 1067610

Updated

3 years ago
Depends on: 1071403

Updated

3 years ago
Depends on: 1066659
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.
Component: JavaScript Engine → JavaScript Engine: JIT
OS: Mac OS X → All
Hardware: x86 → All

Updated

3 years ago
Depends on: 1102843

Updated

3 years ago
Depends on: 999538

Updated

3 years ago
See Also: → bug 934502

Updated

3 years ago
Blocks: 1112641
Depends on: 934502
See Also: bug 934502
(Assignee)

Updated

3 years ago
Depends on: 1123874

Updated

3 years ago
Blocks: 1124002
(Assignee)

Updated

3 years ago
Depends on: 1124377
(Assignee)

Updated

3 years ago
Depends on: 1127908
(Assignee)

Updated

3 years ago
Depends on: 1132904

Updated

3 years ago
Depends on: 1134510
(Assignee)

Comment 5

3 years ago
Created attachment 8568114 [details] [diff] [review]
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
Assignee: nobody → bhackett1024
Attachment #8568114 - Flags: review?(jdemooij)
(Assignee)

Updated

3 years ago
Depends on: 1135834
Blocks: 1134638
Comment on attachment 8568114 [details] [diff] [review]
patch

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

Awesome.
Attachment #8568114 - Flags: review?(jdemooij) → review+
(Assignee)

Comment 7

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/acc238be19a5
https://hg.mozilla.org/mozilla-central/rev/acc238be19a5
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39

Updated

3 years ago
Blocks: 1136898
Depends on: 1139019
Depends on: 1139466
You need to log in before you can comment on or make changes to this bug.