Open Bug 1595241 (convert-one-off-thread-to-pool) Opened 2 years ago Updated 5 months ago

[meta] Convert various one-off threads to use the background thread pool

Categories

(Core :: XPCOM, enhancement)

enhancement
Not set
normal

Tracking

()

People

(Reporter: KrisWright, Unassigned)

References

(Depends on 37 open bugs)

Details

(Keywords: meta)

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

Depends on: 1595242
Summary: Convert various one-off threads to use the background thread pool → [meta] Convert various one-off threads to use the background thread pool
Alias: convert-one-off-thread-to-pool
Depends on: 1604870
Depends on: 1604877
Depends on: 1604883

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!

Flags: needinfo?(kwright)

(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.

Flags: needinfo?(kwright)

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.

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!

Depends on: 1587504
Depends on: 1607354
Depends on: 1607356
Depends on: 1607603
Depends on: 1583857
Depends on: 1613440
Depends on: 1618933
Blocks: 1618934
No longer blocks: 1618934
Depends on: 1618934
Depends on: 1620366
Depends on: 1620367, 1620368
Depends on: 1620369
Depends on: 1620370
Depends on: 1620371
Depends on: 1620372
Depends on: 1620373
Depends on: 1620374
Depends on: 1620375
Depends on: 1620376
Depends on: 1620377
Depends on: 1620378
Depends on: 1620379
Depends on: 1620380
Depends on: 1620381
Depends on: 1620382
Depends on: 1620383
Depends on: 1620384
Depends on: 1620385
Depends on: 1620386
Depends on: 1620387
Depends on: 1620388
Depends on: 1620389
Depends on: 1620390
Depends on: 1620391
Depends on: 1620392
Depends on: 1620393
Depends on: 1620394
Depends on: 1620395
Depends on: 1620396
Depends on: 1620397
Depends on: 1620398
Depends on: 1620399
Depends on: 1626346
No longer depends on: 1638918
Depends on: 1647628
You need to log in before you can comment on or make changes to this bug.