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

--HG--
extra : rebase_source : cfb41f861089bce9e10446bee81c13f8565ba90e
This commit is contained in:
Paul Adenot 2018-04-12 14:23:03 +02:00
Родитель 1c6c907315
Коммит f412eb58ae
5 изменённых файлов: 4 добавлений и 48 удалений

Просмотреть файл

@ -1159,21 +1159,9 @@ MediaStreamGraphImpl::UpdateGraph(GraphTime aEndBlockingDecisions)
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) {
@ -1490,10 +1478,6 @@ public:
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) {
@ -2773,9 +2757,7 @@ SourceMediaStream::SetPullEnabled(bool aEnabled)
}
bool
SourceMediaStream::PullNewData(
StreamTime aDesiredUpToTime,
nsTArray<RefPtr<SourceMediaStream::NotifyPullPromise>>& aPromises)
SourceMediaStream::PullNewData(StreamTime aDesiredUpToTime)
{
TRACE_AUDIO_CALLBACK();
MutexAutoLock lock(mMutex);
@ -2798,7 +2780,7 @@ SourceMediaStream::PullNewData(
MediaStreamListener* l = mListeners[j];
{
MutexAutoUnlock unlock(mMutex);
aPromises.AppendElement(l->AsyncNotifyPull(GraphImpl(), t));
l->NotifyPull(GraphImpl(), t);
}
}
return true;
@ -3613,7 +3595,6 @@ MediaStreamGraphImpl::MediaStreamGraphImpl(GraphDriverType aDriverRequested,
, mStreamOrderDirty(false)
, mLatencyLog(AsyncLatencyLogger::Get())
, mAbstractMainThread(aMainThread)
, mThreadPool(GetMediaThreadPool(MediaThreadType::MSG_CONTROL))
, mSelfRef(this)
, mOutputChannels(std::min<uint32_t>(8, CubebUtils::MaxNumberOfChannels()))
#ifdef DEBUG

Просмотреть файл

@ -12,7 +12,6 @@
#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"
@ -717,13 +716,10 @@ public:
* 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.

Просмотреть файл

@ -819,7 +819,6 @@ public:
RefPtr<AsyncLatencyLogger> mLatencyLog;
AudioMixer mMixer;
const RefPtr<AbstractThread> mAbstractMainThread;
RefPtr<SharedThreadPool> mThreadPool;
// used to limit graph shutdown time
// Only accessed on the main thread.

Просмотреть файл

@ -61,14 +61,6 @@ public:
* 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,

Просмотреть файл

@ -2216,18 +2216,6 @@ public:
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()
{