1613440 - Prevent new ad hoc additions of NS_NewNamedThread

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Kris Wright :KrisWright]

Unless there is a very specific reason not to, we want to encourage async work to go to the background thread pool (bug 1595241) instead of going into another new one-off thread. There are a lot of benefits to using this pool, and it also benefits the future scheduler to reduce our overall thread count and have them funneling into some common mechanism.

To cap the creation of new threads, we want to introduce a mechanism that deprecates new additions of NS_NewNamedThread [1]. I can see us doing this with a clang plugin and a whitelist of all the existing instances of NS_NewNamedThread.

[1] https://searchfox.org/mozilla-central/rev/3a0a8e2762821c6afc1d235b3eb3dde63ad3b01a/xpcom/threads/nsThreadUtils.h#58,64

Comment 1

[Markus Stange [:mstange]]

(It would be good to remove NS_NewThread by that point, otherwise people might be tempted to use it instead of NS_NewNamedThread to get around the error.)

Comment 2

[Kris Wright :KrisWright]

(In reply to Markus Stange [:mstange] from comment #1)

(It would be good to remove NS_NewThread by that point, otherwise people might be tempted to use it instead of NS_NewNamedThread to get around the error.)

This is a good point, and I think fixing bug 1539944 should be a part of this process.

Comment 3

[Kris Wright :KrisWright]

Attachment: Bug 1613440 - Add new clang plugin to deprecate NS_NewNamedThread

Creates a NoNewThreadsChecker plugin that looks for the function and checks to see if it's allowed. This is still WIP and will break your build.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/459a77fff516
Add new clang plugin to deprecate NS_NewNamedThread r=andi

Comment 5

[Dorel Luca [:dluca]]

Backed out changeset 459a77fff516 (bug 1613440) for Clang bustage on tools/extra/clang-tidy/mozilla/Utils.h. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=290242622&repo=autoland&lineNumber=6666

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=459a77fff5166bb4fa82262a5af5d169883dcd95&selectedJob=290242622

Backout:
https://hg.mozilla.org/integration/autoland/rev/868f3fcb3a011832c47ad297b9e993ef22d03317

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f400f164b394
Add new clang plugin to deprecate NS_NewNamedThread r=andi

Comment 7

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/f400f164b394