1913757 - Consider reusing LifoAllocs for Ion compilations

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, task, P3)

Description

[Jan de Mooij [:jandem]]

The JitRuntime has a list of Ion compilations to be freed off-thread once we have a full batch. It might make sense to reuse LifoAlloc memory from this list when doing a new Ion compilation.

I hacked up a patch for this and it looks promising on Speedometer 3, with some high confidence 2-3% improvements on subtests:

https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=509a1004a45ec5aa0d98dc4ddf4d877e1a2071e4&originalSignature=5131097&newSignature=5131097&framework=13&application=firefox&originalRevision=e5a1360ad1d61ad4846b70e7155a7749e3282a5c&page=1

JetStream has some 4-5% regressions though so I need to see what's going on there.

Comment 1

[Denis Palmeiro [:denispal]]

I rebased this patch and ran it again and it has some very good results on the P6 and A55.

Comment 2

[Denis Palmeiro [:denispal]]

Adding some desktop sp3 and jetstream2 numbers:
https://treeherder.mozilla.org/perfherder/compare?originalProject=try&newProject=try&newRevision=e5400564ac40be6301afc509e20d758058ecc561&framework=13&originalRevision=9871f814113bb13b63849cf3e5bb2f307e4d32d4&page=1

There are some mixed results in JS2 subtests but generally all with low confidence, however I don't think this should be a blocker for this work as it looks to be mostly just noise.

Comment 3

[Jan de Mooij [:jandem]]

I've cleaned up my patch and I'm perf-testing some minor variations on Try. The prototype patch had to disable some assertions in the LifoAlloc code but that's no longer needed.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1913757 - Try to reuse LifoAllocs for Ion compilations. r?iain!

When an Ion compilation task is finished, we add it to the runtime's ionFreeTaskBatch_.
With this patch we check this batch and try to reuse LifoAllocs.

This improves some Speedometer 3 subtests by a few percent on Android.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e478167d68b
Try to reuse LifoAllocs for Ion compilations. r=iain

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/8e478167d68b

Comment 7

[Mayank Bansal]

Highlight

2%-2.4% overall improvement on AWFY-A55-SP3! (The numbers vary a bit though)

Almost all the sub-tests improved. Some examples
4% on charts-observable
5.3% on Editor CodeMirror
5.3% on todomvc-Vue-DeletingAllItems

Comment 8

[Andra Esanu (needinfo me)]

(In reply to Pulsebot from comment #5)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e478167d68b
Try to reuse LifoAllocs for Ion compilations. r=iain

Perfherder has detected a browsertime performance change from push 8e478167d68b2ce4c8c9d42d89ef16f94c46a8da.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
7%	speedometer VueJS-TodoMVC/DeletingAllItems/Sync	linux1804-64-shippable-qr	fission webrender	2.02 -> 1.87	Before/After
7%	speedometer VueJS-TodoMVC/DeletingAllItems/Sync	linux1804-64-shippable-qr	fission webrender	2.02 -> 1.88	Before/After
6%	speedometer3 TodoMVC-Preact-Complex-DOM/CompletingAllItems/Sync	android-hw-a55-14-0-aarch64-shippable	fission webrender	3.53 -> 3.32
6%	speedometer VueJS-TodoMVC/DeletingAllItems	linux1804-64-shippable-qr	fission webrender	4.99 -> 4.71
5%	speedometer3 TodoMVC-Svelte-Complex-DOM/Adding100Items/Sync	android-hw-a55-14-0-aarch64-shippable	fission webrender	7.06 -> 6.73
...	...	...	...	...	...
2%	speedometer3 score	android-hw-a55-14-0-aarch64-shippable	fission webrender	6.82 -> 6.95

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 42964

For more information on performance sheriffing please see our FAQ.

Comment 9

[Mayank Bansal]

Its not obvious from the above perf-alert comment, but this patch lead to a 2.03% increase in Jetstrem2 on Windows.