1713335 - Dispatch helper tasks to an external thread pool if one is supplied

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Jon Coppeard (:jonco)]

The plan is to make this work as closely as possible to the way the current helper thread system works.

Comment 6

[Jon Coppeard (:jonco)]

Attachment: Bug 1713335 - Add a pref to control use of XPCOM thread pool, default off r?KrisWright

Comment 7

[Jon Coppeard (:jonco)]

Attachment: Bug 1713335 - Rely on the embedding to chose whether to use an external pool r?sfink

The previous patch added a pref to control use of the external thread pool. I
don't think we need an environment variable as well, although I could be
persuaded to keep this.

Depends on D116812

Comment 8

[Jon Coppeard (:jonco)]

Attachment: Bug 1713335 - Use HelperThreadState::isInitialized instead of CanUseExtraThreads in a few places r?sfink

This is a stronger condition because we assert CanUseExtraThreads during
initialization. It's also closer to to the original intent.

Depends on D116813

Comment 9

[Jon Coppeard (:jonco)]

Attachment: Bug 1713335 - Tidy GlobalHelperThreadState::findHighestPriorityTask to use this rather than the global r?sfink

I missed this when moving findHighestPriorityTask out of HelperThread previously.

Depends on D116814

Comment 10

[Jon Coppeard (:jonco)]

Attachment: Bug 1713335 - Split out methods to check whether there any tasks we can run of each kind r?sfink

Depends on D116815

Comment 11

[Jon Coppeard (:jonco)]

Attachment: Bug 1713335 - Dispatch tasks to external thread pool if present r?sfink

Tasks are dispatched by queueing a generic task for the pool. When this is
executed it will run the highest priority task pending, if any.

In a busy system it's possible that there will be some delay between
dispatching this task and it starting running. There is also a limit on the
number of tasks that are run at once for each task kind. The patch therefore
limits the number of tasks dispatched to the number of avilable threads.

Depends on D116816

Comment 12

[Jon Coppeard (:jonco)]

Attachment: Bug 1713335 - Wait for all pending tasks to run when shutting down r?sfink

This also renames the waitForAllThreads method to the more accurate waitForAllTasks.

Depends on D117004

Comment 13

[Jon Coppeard (:jonco)]

Attachment: Bug 1713335 - Shutdown helper thread system with lock held r?sfink

This allows any external tasks that happen to run after the helper thread
system has shut down to correctly detect that. This shouldn't happen with the
next patch but it seems safer to do it this way.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77a4909e2915
Add a pref to control use of XPCOM thread pool, default off r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/321c6ab4a6a9
Rely on the embedding to chose whether to use an external pool r=sfink
https://hg.mozilla.org/integration/autoland/rev/fbc9b15ecc8f
Use HelperThreadState::isInitialized instead of CanUseExtraThreads in a few places r=sfink
https://hg.mozilla.org/integration/autoland/rev/1fe10e461405
Tidy GlobalHelperThreadState::findHighestPriorityTask to use this rather than the global r=sfink
https://hg.mozilla.org/integration/autoland/rev/996dad41f69b
Split out methods to check whether there any tasks we can run of each kind r=sfink
https://hg.mozilla.org/integration/autoland/rev/b6e426c592ae
Dispatch tasks to external thread pool if present r=sfink
https://hg.mozilla.org/integration/autoland/rev/81b9dab609f6
Shutdown helper thread system with lock held r=sfink
https://hg.mozilla.org/integration/autoland/rev/241321e38305
Wait for all pending tasks to run when shutting down r=sfink

Comment 15

[Atila Butkovits]

Backed out for causing SM bustages.

Backout link: https://hg.mozilla.org/integration/autoland/rev/83e750055a651fc780befc8257e5749dda92d901

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=Dl-DTbmoQieBPVO8pPxsPw.0&revision=241321e3830544532bdde13b62974c6fc6635019&searchStr=linux%2Cx64%2Cdebug%2Cspidermonkey%2Cbuilds%2Chazard-linux64-shell-haz%2Fdebug%2Ch

Failure log: https://treeherder.mozilla.org/logviewer?job_id=342252532&repo=autoland&lineNumber=5873

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1184c5a4b2e8
Add a pref to control use of XPCOM thread pool, default off r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/b9b2e74e7530
Rely on the embedding to chose whether to use an external pool r=sfink
https://hg.mozilla.org/integration/autoland/rev/5a395f40ac52
Use HelperThreadState::isInitialized instead of CanUseExtraThreads in a few places r=sfink
https://hg.mozilla.org/integration/autoland/rev/18e3f8bf6404
Tidy GlobalHelperThreadState::findHighestPriorityTask to use this rather than the global r=sfink
https://hg.mozilla.org/integration/autoland/rev/9c68d8591ef3
Split out methods to check whether there any tasks we can run of each kind r=sfink
https://hg.mozilla.org/integration/autoland/rev/21a7a9dbf79d
Dispatch tasks to external thread pool if present r=sfink
https://hg.mozilla.org/integration/autoland/rev/a460cd7e818d
Shutdown helper thread system with lock held r=sfink
https://hg.mozilla.org/integration/autoland/rev/85042c2c36ba
Wait for all pending tasks to run when shutting down r=sfink

Comment 17

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on browser_policy_hardware_acceleration.js

backout: https://hg.mozilla.org/integration/autoland/rev/fde692ed0ff1025232dbbd7e46a4169846fc8180

push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=Az2ynaRBS0KPSBQCDjKvyA.0&revision=85042c2c36baf3730785ab305cef85d41d8abc58

push where the tier 1 failures appeared: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=P-UQRi47SW-zVt_U9ruXZg.0&revision=d925fb1c95fd897dd6e8efc4e61f4f65293deba5&group_state=expanded&searchStr=Windows%2C10%2Cx64%2CWebRender%2Cdebug%2CMochitests%2Ctest-windows10-64-qr%2Fdebug-mochitest-browser-chrome-e10s%2Cbc5 with the retriggers and backfills pointing to this bug.

failure logs:

• TEST-UNEXPECTED-FAIL | browser/components/enterprisepolicies/tests/browser/hardware_acceleration/browser_policy_hardware_acceleration.js | Hardware acceleration disabled -
• Assertion failure: staticPrefValue == preferenceValue (Preference 'layers.acceleration.disabled' got modified since StaticPrefs::layers_acceleration_disabled_AtStartup_DoNotUseDirectly was initialized. Consider using an always mirror kind instead), at /builds/worker/workspace/obj-build/dist/include/mozilla/StaticPrefList_layers.h:8

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d51dcc6a24db
Add a pref to control use of XPCOM thread pool, default off r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/adc6a4a58aad
Rely on the embedding to chose whether to use an external pool r=sfink
https://hg.mozilla.org/integration/autoland/rev/a10d0b84ea40
Use HelperThreadState::isInitialized instead of CanUseExtraThreads in a few places r=sfink
https://hg.mozilla.org/integration/autoland/rev/0fe894ecc8a9
Tidy GlobalHelperThreadState::findHighestPriorityTask to use this rather than the global r=sfink
https://hg.mozilla.org/integration/autoland/rev/91b32f604312
Split out methods to check whether there any tasks we can run of each kind r=sfink
https://hg.mozilla.org/integration/autoland/rev/b9ddcd04f43d
Dispatch tasks to external thread pool if present r=sfink
https://hg.mozilla.org/integration/autoland/rev/63c15ae1b2fd
Shutdown helper thread system with lock held r=sfink
https://hg.mozilla.org/integration/autoland/rev/1d9496c8460a
Wait for all pending tasks to run when shutting down r=sfink

Comment 19

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/d51dcc6a24db
https://hg.mozilla.org/mozilla-central/rev/adc6a4a58aad
https://hg.mozilla.org/mozilla-central/rev/a10d0b84ea40
https://hg.mozilla.org/mozilla-central/rev/0fe894ecc8a9
https://hg.mozilla.org/mozilla-central/rev/91b32f604312
https://hg.mozilla.org/mozilla-central/rev/b9ddcd04f43d
https://hg.mozilla.org/mozilla-central/rev/63c15ae1b2fd
https://hg.mozilla.org/mozilla-central/rev/1d9496c8460a