Closed
Bug 948183
Opened 11 years ago
Closed 11 years ago
OdinMonkey: don't notifyAll when starting an AsmJSParallelTask
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: luke, Assigned: luke)
Details
Attachments
(1 file)
|
1.82 KB,
patch
|
sstangl
:
review+
|
Details | Diff | Splinter Review |
When there are are more than a few (~6) worker threads, notifyAll() ends up causing significant contention for the WorkerThreadState lock (as each worker wakes up and tries to take the lock so it can check the various queues). On a 32-HW-thread machine, 2.5s is spent on the main thread waiting to get the WorkerThreadState during asm.js compilation of Epic Citadel! Changing the broadcast to single notify drops the contention down to a few ms and also drops compilation time from ~10s to ~5s. There are probably other places where we should change notifyAll to notifyOne, but this requires careful auditing so I'll save that for a separate bug.
Attachment #8344964 -
Flags: review?(sstangl)
|
Assignee | |
Updated•11 years ago
|
Summary: OdinMonkey: don't notifyAll when starting a parse task → OdinMonkey: don't notifyAll when starting an AsmJSParallelTask
|
||
Updated•11 years ago
|
Attachment #8344964 -
Flags: review?(sstangl) → review+
|
|
Assignee | |
Comment 1•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f1be240664e8
|
||
Comment 2•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f1be240664e8
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•