1895846 - Make SharedThreadPool less volatile

Status: ASSIGNED

(Core :: XPCOM, task)

Description

[Jens Stutte [:jstutte]]

Currently SharedThreadPool shuts down a pool as soon as consumers do not hold a reference to it. In case of consumers acquiring a new reference frequently without keeping it across uses, this can lead to many thread shutdowns and re-creations. MediaPDecoder seems to be an example of such usage. It seems in WebRTC code we added some AddRef calls here and there to account for this.

It might be easier for consumers, if SharedThreadPool keeps an ever-used pool alive until shutdown. If it is really not used for a while, all its threads will be closed and we would just consume the thread pool's object memory.

Comment 1

[Jens Stutte [:jstutte]]

Attachment: WIP: Bug 1895846 - Streamline SharedThreadPool a bit and audit its uses for thread numbers and timeouts. r?#xpcom-reviewers

SharedThreadPool's interface makes consumers believe that they do not
need to keep a reference to the obtained thread pool alive and that
SharedThreadPool will manage the shutdown behavior.

The latter is kind of true, but the first is not: as soon as the last
reference to the same named pool is dropped, it will be destroyed. For
consumers that repeatedly do something on a pool with the same name but
do not store the reference long enough across those uses, this will
result in frequent re-creations of the same named pool, resulting in
more frequent thread spin-ups than needed.

Given that the nsThreadPool object itself is pretty lightweight and
its threads will time out and be closed eventually if the pool is idle
for long enough, it is probably better to keep it around until
shutdown, benefitting from its idle timeouts for the next usage by the
same consumer if it arrives soon enough. We seem to not have any use
that is causing an infinite number of different thread pool names such
that this could really matter for memory usage.

We also make no use of SharedThreadPool's ability to change the thread
limit on each retreival, thus it get's removed. Together with that we
revisit some thread limits and timeout values.

Depends on D209197

Comment 2

[Jens Stutte [:jstutte]]

Looking at how SharedThreadPool manages its shutdown I came across:

nsThreadPool::PutEvent bails out on shutdown regardless of the source of an event.

if (NS_WARN_IF(mShutdown && !IsOnCurrentThreadInfallible())) might be a better choice, together with avoiding to spawn a new thread below when mShutdown (the one we are running on can just take the event).

which might deserve an own bug.

Comment 4

[Jens Stutte [:jstutte]]

We might also want to consider renaming SharedThreadPool.