Bug 1456115 - Re-serialize inbound NotifyPull. r=jya
authorPaul Adenot <paul@paul.cx>
Thu, 12 Apr 2018 14:23:03 +0200
changeset 469068 4a1df66695dd96b715503b2a27359471de6c57e4
parent 469067 290879198eb4a7d2b7ecfd5ba07b38ea31710529
child 469069 956b62a9c5a437c2bfdfac6605332283f9c14086
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya
bugs1456115
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
dom/media/MediaStreamGraph.cpp
dom/media/MediaStreamGraph.h
dom/media/MediaStreamGraphImpl.h
dom/media/MediaStreamListener.h
media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
--- 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)