Closed Bug 1398140 Opened 7 years ago Closed 7 years ago

Consider removing the Ion helper thread pausing

Categories

(Core :: JavaScript Engine: JIT, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We have this mechanism where we allow 1 helper thread at a time to do Ion compilation. If a higher priority task comes in, we can pause other compilation threads, process the new task on a new thread, then unpause the paused thread(s).

This adds quite a lot of complexity (to code that's already complicated enough!). It also has some issues:

* We would like to Ion-compile scripts in parallel, especially when the CPU has 4+ cores.

* Pausing compilation threads means these threads are not available for other tasks (GC, Wasm, parsing).

* IonBuilderHasHigherPriority includes the size and warm up count of the outer script, but due to inlining it's not clear whether that's very useful.

Removing the pausing mechanism improves shell benchmarks a bit locally. I'll check how this affects Speedometer on AWFY.
Flags: needinfo?(jdemooij)
Attached patch PatchSplinter Review
Mostly just removing code:

 5 files changed, 9 insertions(+), 185 deletions(-)

This improves shell benchmarks locally. I'm not sure about Speedometer (my AWFY-Try run failed for some reason). I think we should just land it and see what happens on AWFY.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8906512 - Flags: review?(luke)
Comment on attachment 8906512 [details] [diff] [review]
Patch

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

Great!
Attachment #8906512 - Flags: review?(luke) → review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab8c75e0d422
Remove Ion helper thread pausing mechanism. r=luke
For posterity: some minor regressions on AWFY but overall this was an improvement. 7-9% on Kraken crypto-ccm, 2.2% on Octane mandreel, small win on Sunspider. Some of the improvements are bigger on 32-bit (> 11% on Kraken crypto-ccm). Not too bad for a code removal patch.

Will be interesting to see if this has any effect on the (slower) QF hardware or on Talos.
https://hg.mozilla.org/mozilla-central/rev/ab8c75e0d422
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.