We can change to using SharedThreadPools for the decoder and state machine threads. * This is the first major step towards running our decoders asynchronously, and reducing the number of threads we need. * This means we won't spin up and shutdown decode threads every time a decoder fills its audio and video MediaQueues. The thread pool naturally keeps its thread alive for 60s after they become idle. Constantly creating and destroying decode threads showed up in a profile in bug 965761. * With this, we don't need to use the StateMachineTracker to track decode threads, as this functionality is handled by the SharedThreadPool. * We'll use a SharedThreadPool for the decode threads. It will have a thread limit of 25. This means we'll have the same basic behaviour as is currently provided by the StateMachineTracker's "request decode thread" functionality.

We'll need to setup the thread pools in MediaDecoderStateMachine::Init(), otherwise we won't have any threads to decode with. So MediaSourceDecoder will need to call MediaDecoderStateMachine::Init() on its startup path, else it won't have any threads to decode with. So copy the regular MediaDecoder::Load() sequence and create and init the state machine in MediaSourceDecoder::Load().

Add the ability to specify the number of threads in a SharedThreadPool. We want this so that we can use a SharedThreadPool of size 1 thread to use as the state machine thread. This means we can get the singletonness of the state machine thread automatically managed by the SharedThreadPool infrastructure.

The media code has assertions throughout to ensure code is running on the threads we think it's supposed to be running on. We'll want to keep this behaviour when we switch to using thread pools instead of hordes of individually managed threads. The easy way is to implement nsIThreadPool::IsCurrentThreadOn(). I see there's a stern looking comment there saying we need to think carefully about what nsIThreadPool::IsCurrentThreadOn() is supposed to mean... Since we'll only be using this in assertions in debug builds, I'm happy to make the implementation #ifdef DEBUG if we don't want the implementation being exposed in release builds.

Implement MediaTaskQueue::IsCurrentThreadIn() and expose nsIEventTarget::IsCurrentThreadIn() on the SharedThreadPool, so the MediaDecoderStateMachine and friends can use these in MediaDecoderStateMachine::OnDecodeThread() etc.

Use MediaTaskQueue, running on a SharedThreadPool for decode threads. We basically just dispatch a runnable to call MediaDecoderStateMachine::DecodeThreadRun() on the task queue, instead of creating a new thread every time we want to run this function. Once DecodeThreadRun() returns (because our MediaQueues are full, or whatever) we'll free up a thread for the next decoder. This matches the existing behaviour provided by the StateMachineTracker.

* Instead of having the StateMachineTracker enforce the singletoness of the state machine thread, use a SharedThreadPool of size 1. We now don't need custom threadsafe singleton code for the state machine thread. * Change MediaDecoderStateMachine::GetStateMachineThread() to be non-static and return an nsIEventTarget from the state machine thread' SharedThreadPool. * Can remove StateMachineTracker completely now. * We don't need to track the state machine thread in the MediaShutdownManager. I will however add code in a later patch to enforce that all SharedThreadPools have been destroyed before our XPCOM shutdown listener exits however.

Add an attribute to nsIThreadPool to set the reserved stack size of its threads. Media threads set their stack size, as we've had problems with running out of address space in the past, so we need this.

Spin the event loop in our xpcom-shutdown observer until all the SharedThreadPools have completely shutdown. We can spin because we shutdown the hashtable of SharedThreadPools in an event dispatched to the main thread.

Greenish so far: https://tbpl.mozilla.org/?tree=Try&rev=e2d0b269bcfc

Note these patches are based on my just-landed patches in bug 962385.

Comment on attachment 8370537 [details] [diff] [review] Patch 5v1: Use SharedThreadPool for decode threads Review of attachment 8370537 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/MediaDecoderStateMachine.cpp @@ +1184,5 @@ > + > + nsresult rv = mReader->Init(cloneReader); > + NS_ENSURE_SUCCESS(rv, rv); > + > + return NS_OK; The old code seemed more succinct.

Comment on attachment 8370535 [details] [diff] [review] Patch 3v1: Implement nsIThreadPool::IsCurrentThreadOn() I don't mind having the implementation in release builds, but because it's a lock and a loop it's going to be slow, so we should try not to actually use it in release builds.

Comment on attachment 8370540 [details] [diff] [review] Patch 7v1: Add threadStackSize attribute to nsIThreadPool It seems a little strange to me that SetThreadStackSize is entering the monitor but GetThreadStackSize isn't and we're using mStackSize value outside the monitor in nsThreadPool::PutEvent. I do think we have to synchronize it, unless we're going to make it an init-only property. And in that case we should either consistently use the monitor or use an Atomic. The monitor is better in this case.

With more synchronization, via the monitor as suggested.

Try: https://tbpl.mozilla.org/?tree=Try&rev=1db91ce7fe2a

(In reply to Chris Pearce (:cpearce) from comment #15) > Try: > https://tbpl.mozilla.org/?tree=Try&rev=1db91ce7fe2a B2G ICS Emulator timeout: WARNING: waitpid failed pid:723 errno:10: file ../../../gecko/ipc/chromium/src/base/process_uB2GRunner TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_standalone.html | application timed out after 330.0 seconds with no output

There is two problems here: 1. I stopped calling MediaDecoderReader::OnDecodeThreadStart() and OnDecodeThreadEnd(). We should still do that for B2G's benefit. 2. I think I need to wait for the decode task queue to be idle before calling MediaDecoderReader::ReleaseMediaResources() in MediaDecoderStateMachine::RunStateMachine()'s DORMANT case. I'm already doing this when I call ReleaseMediaResources() in RunStateMachine()'s SHUTDOWN case, and all the crashes I've seen are crashing in the call to ReleaseMediaResources() in RunStateMachine()'s DORMANT case where I didn't do this. Let's see: https://tbpl.mozilla.org/?tree=Try&rev=b4b841419093

We need to do this to ensure we save power on B2G devices. Not needed any more technically on Windows (except in the webaudio case) but won't hurt.

Await decode task completion when entering DORMANT state. This matches the behaviour of the SHUTDOWN state machine case, and prevents the failure on B2G emulator mochitest chunk 3 & 4.

Once more, because I'm paranoid: https://tbpl.mozilla.org/?tree=Try&rev=f899e2e2f9ee

(In reply to Chris Pearce (:cpearce) from comment #20) > Once more, because I'm paranoid: > https://tbpl.mozilla.org/?tree=Try&rev=f899e2e2f9ee Turns out my paranoia is justified. There's still a fairly frequent failure in this push on Linux mtest-1: 8600 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_bug448534.html | Test timed out. 8605 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_bug463162.xhtml | Test timed out. 8611 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_bug495145.html | Test timed out. 8617 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_bug495300.html | Test timed out. I have a patch to fix it in bug 973379. Once that lands, this can land.

The remaining orange should have been fixed by bug 973379 landing a few minutes ago. Landed on m-i for Firefox 30. https://hg.mozilla.org/integration/mozilla-inbound/rev/0f78e7bb808c https://hg.mozilla.org/integration/mozilla-inbound/rev/406b7fde02e2 https://hg.mozilla.org/integration/mozilla-inbound/rev/d71ac8b9f7de https://hg.mozilla.org/integration/mozilla-inbound/rev/7102981f5c72 https://hg.mozilla.org/integration/mozilla-inbound/rev/891419fc6190 https://hg.mozilla.org/integration/mozilla-inbound/rev/654700db7152 https://hg.mozilla.org/integration/mozilla-inbound/rev/2c760d0a1a3b https://hg.mozilla.org/integration/mozilla-inbound/rev/4ba0ec65173f https://hg.mozilla.org/integration/mozilla-inbound/rev/c1c582be4d20 https://hg.mozilla.org/integration/mozilla-inbound/rev/c9d80d79b817

https://hg.mozilla.org/mozilla-central/rev/0f78e7bb808c https://hg.mozilla.org/mozilla-central/rev/406b7fde02e2 https://hg.mozilla.org/mozilla-central/rev/d71ac8b9f7de https://hg.mozilla.org/mozilla-central/rev/7102981f5c72 https://hg.mozilla.org/mozilla-central/rev/891419fc6190 https://hg.mozilla.org/mozilla-central/rev/654700db7152 https://hg.mozilla.org/mozilla-central/rev/2c760d0a1a3b https://hg.mozilla.org/mozilla-central/rev/4ba0ec65173f https://hg.mozilla.org/mozilla-central/rev/c1c582be4d20 https://hg.mozilla.org/mozilla-central/rev/c9d80d79b817