Bug 1456115 - Re-serialize inbound NotifyPull. r?jya
We made NotifyPull parallel to try to lower the load, and we initially measured
an improvement. However, we did the measurements with a profiler that did an
aggregation of the results. Our results had an high variance, so the mean load
was in fact not meaningful.
More careful measurement performed without doing any aggregation show that,
under load, relying on the fact that the scheduler schedules the tasks on time
is too risky, and that the code is fast enough to not have to parallelize.
MozReview-Commit-ID: CMhSn8Sc0OO
--- a/dom/media/MediaStreamGraph.cpp
+++ b/dom/media/MediaStreamGraph.cpp
@@ -1154,31 +1154,19 @@ MediaStreamGraphImpl::UpdateGraph(GraphT
// means the driver would have been blocking indefinitly, but the graph has
// been woken up right after having been to sleep.
MOZ_ASSERT(aEndBlockingDecisions >= mStateComputedTime);
UpdateStreamOrder();
bool ensureNextIteration = false;
- // Grab pending stream input and compute blocking time
- AutoTArray<RefPtr<SourceMediaStream::NotifyPullPromise>, 64> promises;
for (MediaStream* stream : mStreams) {
if (SourceMediaStream* is = stream->AsSourceStream()) {
- ensureNextIteration |= is->PullNewData(aEndBlockingDecisions, promises);
- }
- }
-
- // Wait until all PullEnabled stream's listeners have completed.
- if (!promises.IsEmpty()) {
- AwaitAll(do_AddRef(mThreadPool), promises);
- }
-
- for (MediaStream* stream : mStreams) {
- if (SourceMediaStream* is = stream->AsSourceStream()) {
+ ensureNextIteration |= is->PullNewData(aEndBlockingDecisions);
is->ExtractPendingInput();
}
if (stream->mFinished) {
// The stream's not suspended, and since it's finished, underruns won't
// stop it playing out. So there's no blocking other than what we impose
// here.
GraphTime endTime = stream->GetStreamTracks().GetAllTracksEnd() +
stream->mTracksStartTime;
@@ -1485,20 +1473,16 @@ public:
// objects owning streams, or for expiration of mGraph->mShutdownTimer,
// which won't otherwise release its reference on the graph until
// nsTimerImpl::Shutdown(), which runs after xpcom-shutdown-threads.
{
MonitorAutoLock mon(mGraph->mMonitor);
mGraph->SetCurrentDriver(nullptr);
}
- // Do not hold on our threadpool, global shutdown will hang otherwise as
- // it waits for all thread pools to shutdown.
- mGraph->mThreadPool = nullptr;
-
// Safe to access these without the monitor since the graph isn't running.
// We may be one of several graphs. Drop ticket to eventually unblock shutdown.
if (mGraph->mShutdownTimer && !mGraph->mForceShutdownTicket) {
MOZ_ASSERT(false,
"AudioCallbackDriver took too long to shut down and we let shutdown"
" continue - freezing and leaking");
// The timer fired, so we may be deeper in shutdown now. Block any further
@@ -2768,19 +2752,17 @@ SourceMediaStream::SetPullEnabled(bool a
}
SourceMediaStream* mStream;
bool mEnabled;
};
GraphImpl()->AppendMessage(MakeUnique<Message>(this, aEnabled));
}
bool
-SourceMediaStream::PullNewData(
- StreamTime aDesiredUpToTime,
- nsTArray<RefPtr<SourceMediaStream::NotifyPullPromise>>& aPromises)
+SourceMediaStream::PullNewData(StreamTime aDesiredUpToTime)
{
TRACE_AUDIO_CALLBACK();
MutexAutoLock lock(mMutex);
if (!mPullEnabled || mFinished) {
return false;
}
// Compute how much stream time we'll need assuming we don't block
// the stream at all.
@@ -2793,17 +2775,17 @@ SourceMediaStream::PullNewData(
GraphImpl()->MediaTimeToSeconds(current)));
if (t <= current) {
return false;
}
for (uint32_t j = 0; j < mListeners.Length(); ++j) {
MediaStreamListener* l = mListeners[j];
{
MutexAutoUnlock unlock(mMutex);
- aPromises.AppendElement(l->AsyncNotifyPull(GraphImpl(), t));
+ l->NotifyPull(GraphImpl(), t);
}
}
return true;
}
void
SourceMediaStream::ExtractPendingInput()
{
@@ -3608,17 +3590,16 @@ MediaStreamGraphImpl::MediaStreamGraphIm
, mPostedRunInStableStateEvent(false)
, mDetectedNotRunning(false)
, mPostedRunInStableState(false)
, mRealtime(aDriverRequested != OFFLINE_THREAD_DRIVER)
, mNonRealtimeProcessing(false)
, mStreamOrderDirty(false)
, mLatencyLog(AsyncLatencyLogger::Get())
, mAbstractMainThread(aMainThread)
- , mThreadPool(GetMediaThreadPool(MediaThreadType::MSG_CONTROL))
, mSelfRef(this)
, mOutputChannels(std::min<uint32_t>(8, CubebUtils::MaxNumberOfChannels()))
#ifdef DEBUG
, mCanRunMessagesSynchronously(false)
#endif
{
if (mRealtime) {
if (aDriverRequested == AUDIO_THREAD_DRIVER) {
--- a/dom/media/MediaStreamGraph.h
+++ b/dom/media/MediaStreamGraph.h
@@ -7,17 +7,16 @@
#define MOZILLA_MEDIASTREAMGRAPH_H_
#include "AudioStream.h"
#include "MainThreadUtils.h"
#include "MediaStreamTypes.h"
#include "StreamTracks.h"
#include "VideoSegment.h"
#include "mozilla/LinkedList.h"
-#include "mozilla/MozPromise.h"
#include "mozilla/Mutex.h"
#include "mozilla/TaskQueue.h"
#include "nsAutoPtr.h"
#include "nsAutoRef.h"
#include "nsIRunnable.h"
#include "nsTArray.h"
#include <speex/speex_resampler.h>
@@ -712,23 +711,20 @@ public:
// MediaStreamGraph thread only
void DestroyImpl() override;
// Call these on any thread.
/**
* Call all MediaStreamListeners to request new data via the NotifyPull API
* (if enabled).
* aDesiredUpToTime (in): end time of new data requested.
- * aPromises (out): NotifyPullPromises if async API is enabled.
*
* Returns true if new data is about to be added.
*/
- typedef MozPromise<bool, bool, true /* is exclusive */ > NotifyPullPromise;
- bool PullNewData(StreamTime aDesiredUpToTime,
- nsTArray<RefPtr<NotifyPullPromise>>& aPromises);
+ bool PullNewData(StreamTime aDesiredUpToTime);
/**
* Extract any state updates pending in the stream, and apply them.
*/
void ExtractPendingInput();
/**
* These add/remove DirectListeners, which allow bypassing the graph and any
--- a/dom/media/MediaStreamGraphImpl.h
+++ b/dom/media/MediaStreamGraphImpl.h
@@ -814,17 +814,16 @@ public:
*/
bool mStreamOrderDirty;
/**
* Hold a ref to the Latency logger
*/
RefPtr<AsyncLatencyLogger> mLatencyLog;
AudioMixer mMixer;
const RefPtr<AbstractThread> mAbstractMainThread;
- RefPtr<SharedThreadPool> mThreadPool;
// used to limit graph shutdown time
// Only accessed on the main thread.
nsCOMPtr<nsITimer> mShutdownTimer;
private:
virtual ~MediaStreamGraphImpl();
--- a/dom/media/MediaStreamListener.h
+++ b/dom/media/MediaStreamListener.h
@@ -56,24 +56,16 @@ public:
* calls to add or remove MediaStreamListeners. It is not allowed to block
* for any length of time.
* aDesiredTime is the stream time we would like to get data up to. Data
* beyond this point will not be played until NotifyPull runs again, so there's
* not much point in providing it. Note that if the stream is blocked for
* some reason, then data before aDesiredTime may not be played immediately.
*/
virtual void NotifyPull(MediaStreamGraph* aGraph, StreamTime aDesiredTime) {}
- virtual RefPtr<SourceMediaStream::NotifyPullPromise> AsyncNotifyPull(
- MediaStreamGraph* aGraph,
- StreamTime aDesiredTime)
- {
- NotifyPull(aGraph, aDesiredTime);
- return SourceMediaStream::NotifyPullPromise::CreateAndResolve(true,
- __func__);
- }
enum Blocking {
BLOCKED,
UNBLOCKED
};
/**
* Notify that the blocking status of the stream changed. The initial state
* is assumed to be BLOCKED.
--- a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ -2211,28 +2211,16 @@ public:
// Implement MediaStreamListener
void NotifyPull(MediaStreamGraph* aGraph,
StreamTime aDesiredTime) override
{
NotifyPullImpl(aDesiredTime);
}
- RefPtr<SourceMediaStream::NotifyPullPromise> AsyncNotifyPull(
- MediaStreamGraph* aGraph,
- StreamTime aDesiredTime) override
- {
- RefPtr<PipelineListener> self = this;
- return InvokeAsync(mTaskQueue, __func__, [self, aDesiredTime]() {
- self->NotifyPullImpl(aDesiredTime);
- return SourceMediaStream::NotifyPullPromise::CreateAndResolve(true,
- __func__);
- });
- }
-
private:
~PipelineListener()
{
NS_ReleaseOnMainThreadSystemGroup("MediaPipeline::mConduit",
mConduit.forget());
}
void NotifyPullImpl(StreamTime aDesiredTime)