1595241 - (convert-one-off-thread-to-pool) [meta] Convert various one-off threads to use the background thread pool

Status: NEW

(Core :: XPCOM, enhancement)

Description

[Kris Wright :KrisWright]

With the introduction of the background thread pool, it's now possible to convert various one-off threads to dispatch via the API [1]. This bug is to track moving tasks to the background pool.

https://searchfox.org/mozilla-central/rev/2a10f4812f3f7c7645d253a4fe52f26bf58e20e8/xpcom/threads/nsThreadUtils.h#1704

Comment 1

[Nicholas Nethercote [inactive]]

Kris, can you give a brief description why we would want to convert these one-off threads to the new API? (E.g. memory usage, code consistency, etc.) Thanks!

Comment 2

[Kris Wright :KrisWright]

(In reply to Nicholas Nethercote [:njn] from comment #1)

Kris, can you give a brief description why we would want to convert these one-off threads to the new API? (E.g. memory usage, code consistency, etc.) Thanks!

Of course! There's a lot of benefits to using a general-purpose thread pool over creating your own threads.
There's a few problems that come up first with spinning up one-off threads that make it more difficult to maintain a clear global threading picture. Handling offthread work on a case-by-case basis means we have a lot of code that does a similar thing in a lot of different ways. Given the need to reduce thread overhead (see Bug 1476432) the idea of simply creating a new thread whenever some offthread job needs to be done is not ideal. Even if the creator of the thread shuts the thread down after they're done, there's still the additional overhead of winding up and spinning down a thread or pool to do a certain task versus creating just one pool at startup which accepts a variety of jobs. Using a general-purpose pool instead benefits our thread overhead by reducing the amount of threads capable of being active at once on a single content process - with the additional benefit of improving the uniformity of general background work.

In the past, we've used the stream transport service as a sort of general-purpose pool for this reason, but this is also not ideal. In these cases the stream transport service is not necessarily used for its intended purpose and it's caused the amount of existing stream transport threads in a content process to be inflated (it's not uncommon in debugging to find myself on the 200+th instance of a stream transport thread, which is not ideal for the same reasons listed above).

Additionally, this API significantly simplifies the process of dispatching background jobs. There's no need to hold a reference to a thread or event target in simple dispatch cases, and when a job requires a background task queue to manipulate, the task queues handle their own shutdown. A simpler API benefits anyone looking to do any work offthread in the future.

Comment 3

[Nathan Froyd [:froydnj]]

We (Bas and bz) are also looking at scheduling work more intelligently, and funneling everything through a common interface helps with that. Also, if we ever wanted to experiment (e.g. start offloading tasks onto, say, the Rayon style pool, or something else running in Rust generally), it's easier to do when everything is coming through a common interface.

Comment 4

[Nicholas Nethercote [inactive]]

Thank you for the answers. So, in summary, the potential benefits of using the new API include the following.

• Avoids the time cost of spinning up one-off threads and multiple thread pools.
• Avoids the memory cost of too many threads and thread pools.
• Avoids the debugging difficulties of too many threads.
• Avoids the code comprehension and debugging difficulties caused by confusingly named/located threads, such as non-stream transport threads in the stream transport pool.
• Improves code consistency.
• Improves code simplicity/readability when background jobs are involved.
• Opens the possibility of better thread scheduling.

Nice!