1571012 - make GraphDriver stop using a singleton SharedThreadPool for background tasks

Status: NEW

(Core :: Audio/Video, task, P3)

Description

[Nathan Froyd [:froydnj]]

GraphDriver acquires a SharedThreadPool with a single thread, and then later sets the idle timeout on that pool to some value. This seems a bit silly: a thread pool with a single thread is just LazyIdleThread. It's also somewhat wasteful to continually reset the timeout.

But we can't use LazyIdleThread because our thread pools support dispatching from any thread, whereas LazyIdleThread only supports dispatching from the thread that created it:

https://searchfox.org/mozilla-central/source/xpcom/threads/LazyIdleThread.cpp#364

(I'm not completely sure why we have that restriction, but we do.)

There are a couple options to fix this:

1. Make the SharedThreadPool being acquired actually shared with something else in media:

https://searchfox.org/mozilla-central/source/dom/media/VideoUtils.cpp#211

I guess that doesn't really do anything for the idle timeout, but ideally these pools would be live long enough that it wouldn't matter?
2. Actually fix LazyIdleThread so we could just use it in this context.

jesup, do you remember why we had this restriction on LazyIdleThread?

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

The restriction is in the first checkin (in dom/indexedDB) of LazyIdleThread back in 2010 by bent. I suspect there are a number of mFoo's that it doesn't lock to access during Dispatch that could cause TSAN or other races.

Looks like LazyIdleThread is used in 8 or 9 places. I don't think the perf hit of locking whatever needs locking would be problematic, though a raptor run or (better) perhaps a microbenchmark of Dispatch() times might serve as a double-check.

Comment 2

[Michael Froman [:mjf]]

Marking this P3 for now. Nathan, feel free to modify if you disagree.