Bug 1191684 - Remove unnecessary calls to NotifyAll() on the decoder monitor since no one calls Wait(). r=cpearce.
authorJW Wang <jwwang@mozilla.com>
Mon, 10 Aug 2015 09:54:48 +0800
changeset 288696 f77ab4c12d923e997e5e9db6116766e3ab9f3339
parent 288695 6ac77d3c4dc3b0ae936ae2bcabd23b20e737d65f
child 288697 c8cfe14cf31a42b53ccdb8cfd8869da3f28c64ed
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1191684
milestone42.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 1191684 - Remove unnecessary calls to NotifyAll() on the decoder monitor since no one calls Wait(). r=cpearce.
dom/media/MediaDecoder.cpp
dom/media/MediaDecoder.h
dom/media/MediaDecoderStateMachine.cpp
dom/media/MediaDecoderStateMachine.h
--- a/dom/media/MediaDecoder.cpp
+++ b/dom/media/MediaDecoder.cpp
@@ -1008,35 +1008,32 @@ void MediaDecoder::ChangeState(PlayState
   MOZ_ASSERT(NS_IsMainThread());
   ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
 
   if (mNextState == aState) {
     mNextState = PLAY_STATE_PAUSED;
   }
 
   if (mPlayState == PLAY_STATE_SHUTDOWN) {
-    GetReentrantMonitor().NotifyAll();
     return;
   }
 
   DECODER_LOG("ChangeState %s => %s",
               ToPlayStateStr(mPlayState), ToPlayStateStr(aState));
   mPlayState = aState;
 
   if (mPlayState == PLAY_STATE_PLAYING) {
     ConstructMediaTracks();
   } else if (IsEnded()) {
     RemoveMediaTracks();
   }
 
   CancelDormantTimer();
   // Start dormant timer if necessary
   StartDormantTimer();
-
-  GetReentrantMonitor().NotifyAll();
 }
 
 void MediaDecoder::UpdateLogicalPosition(MediaDecoderEventVisibility aEventVisibility)
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (mShuttingDown)
     return;
 
--- a/dom/media/MediaDecoder.h
+++ b/dom/media/MediaDecoder.h
@@ -1093,25 +1093,21 @@ protected:
   // Media duration set explicitly by JS. At present, this is only ever present
   // for MSE.
   Canonical<Maybe<double>> mExplicitDuration;
 
   // Set to one of the valid play states.
   // This can only be changed on the main thread while holding the decoder
   // monitor. Thus, it can be safely read while holding the decoder monitor
   // OR on the main thread.
-  // Any change to the state on the main thread must call NotifyAll on the
-  // monitor so the decode thread can wake up.
   Canonical<PlayState> mPlayState;
 
   // This can only be changed on the main thread while holding the decoder
   // monitor. Thus, it can be safely read while holding the decoder monitor
   // OR on the main thread.
-  // Any change to the state must call NotifyAll on the monitor.
-  // This can only be PLAY_STATE_PAUSED or PLAY_STATE_PLAYING.
   Canonical<PlayState> mNextState;
 
   // True if the decoder is seeking.
   Canonical<bool> mLogicallySeeking;
 
   // True if the media is same-origin with the element. Data can only be
   // passed to MediaStreams when this is true.
   Canonical<bool> mSameOriginMedia;
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -655,19 +655,16 @@ MediaDecoderStateMachine::Push(VideoData
   MOZ_ASSERT(aSample);
   // TODO: Send aSample to MSG and recalculate readystate before pushing,
   // otherwise AdvanceFrame may pop the sample before we have a chance
   // to reach playing.
   aSample->mFrameID = ++mCurrentFrameID;
   VideoQueue().Push(aSample);
   UpdateNextFrameStatus();
   DispatchDecodeTasksIfNeeded();
-
-  // XXXbholley - Is this still necessary?
-  mDecoder->GetReentrantMonitor().NotifyAll();
 }
 
 void
 MediaDecoderStateMachine::PushFront(VideoData* aSample)
 {
   MOZ_ASSERT(OnTaskQueue());
   MOZ_ASSERT(aSample);
 
@@ -689,19 +686,16 @@ MediaDecoderStateMachine::OnAudioPopped(
 void
 MediaDecoderStateMachine::OnVideoPopped(const MediaData* aSample)
 {
   MOZ_ASSERT(OnTaskQueue());
   ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
   mDecoder->UpdatePlaybackOffset(aSample->mOffset);
   UpdateNextFrameStatus();
   DispatchVideoDecodeTaskIfNeeded();
-  // Notify the decode thread that the video queue's buffers may have
-  // free'd up space for more frames.
-  mDecoder->GetReentrantMonitor().NotifyAll();
 }
 
 void
 MediaDecoderStateMachine::OnNotDecoded(MediaData::Type aType,
                                        MediaDecoderReader::NotDecodedReason aReason)
 {
   MOZ_ASSERT(OnTaskQueue());
   ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
@@ -773,17 +767,16 @@ MediaDecoderStateMachine::OnNotDecoded(M
   }
   switch (mState) {
     case DECODER_STATE_BUFFERING:
     case DECODER_STATE_DECODING: {
       if (MaybeFinishDecodeFirstFrame()) {
         return;
       }
       CheckIfDecodeComplete();
-      mDecoder->GetReentrantMonitor().NotifyAll();
       // Schedule the state machine to notify track ended as soon as possible.
       if (mAudioCaptured) {
         ScheduleStateMachine();
       }
       return;
     }
     case DECODER_STATE_SEEKING: {
       if (!mCurrentSeek.Exists()) {
@@ -1088,17 +1081,16 @@ void MediaDecoderStateMachine::MaybeStar
   StartAudioThread();
 
   // Tell DecodedStream to start playback with specified start time and media
   // info. This is consistent with how we create AudioSink in StartAudioThread().
   if (mAudioCaptured) {
     mDecodedStream->StartPlayback(GetMediaTime(), mInfo);
   }
 
-  mDecoder->GetReentrantMonitor().NotifyAll();
   DispatchDecodeTasksIfNeeded();
 }
 
 void MediaDecoderStateMachine::UpdatePlaybackPositionInternal(int64_t aTime)
 {
   MOZ_ASSERT(OnTaskQueue());
   SAMPLE_LOG("UpdatePlaybackPositionInternal(%lld)", aTime);
   AssertCurrentThreadInMonitor();
@@ -1255,23 +1247,21 @@ void MediaDecoderStateMachine::SetDorman
     // Note that we do not wait for the decode task queue to go idle before
     // queuing the ReleaseMediaResources task - instead, we disconnect promises,
     // reset state, and put a ResetDecode in the decode task queue. Any tasks
     // that run after ResetDecode are supposed to run with a clean slate. We rely
     // on that in other places (i.e. seeking), so it seems reasonable to rely on
     // it here as well.
     nsCOMPtr<nsIRunnable> r = NS_NewRunnableMethod(mReader, &MediaDecoderReader::ReleaseMediaResources);
     DecodeTaskQueue()->Dispatch(r.forget());
-    mDecoder->GetReentrantMonitor().NotifyAll();
   } else if ((aDormant != true) && (mState == DECODER_STATE_DORMANT)) {
     mDecodingFrozenAtStateDecoding = true;
     ScheduleStateMachine();
     mDecodingFirstFrame = true;
     SetState(DECODER_STATE_DECODING_NONE);
-    mDecoder->GetReentrantMonitor().NotifyAll();
   }
 }
 
 void MediaDecoderStateMachine::Shutdown()
 {
   MOZ_ASSERT(OnTaskQueue());
 
   // Once we've entered the shutdown state here there's no going back.
@@ -1904,20 +1894,16 @@ MediaDecoderStateMachine::DecodeError()
   }
 
   // Change state to error, which will cause the state machine to wait until
   // the MediaDecoder shuts it down.
   SetState(DECODER_STATE_ERROR);
   ScheduleStateMachine();
   DECODER_WARN("Decode error, changed state to ERROR");
 
-  // XXXbholley - Is anybody actually waiting on this monitor, or is it just
-  // a leftover from when we used to do sync dispatch for the below?
-  mDecoder->GetReentrantMonitor().NotifyAll();
-
   // MediaDecoder::DecodeError notifies the owner, and then shuts down the state
   // machine.
   nsCOMPtr<nsIRunnable> event =
     NS_NewRunnableMethod(mDecoder, &MediaDecoder::DecodeError);
   AbstractThread::MainThread()->Dispatch(event.forget());
 }
 
 void
@@ -2351,18 +2337,16 @@ nsresult MediaDecoderStateMachine::RunSt
                     OutOfDecodedVideo(), VideoRequestStatus());
         return NS_OK;
       }
 
       DECODER_LOG("Changed state from BUFFERING to DECODING");
       DECODER_LOG("Buffered for %.3lfs", (now - mBufferingStart).ToSeconds());
       StartDecoding();
 
-      // Notify to allow blocked decoder thread to continue
-      mDecoder->GetReentrantMonitor().NotifyAll();
       NS_ASSERTION(IsStateMachineScheduled(), "Must have timer scheduled");
       return NS_OK;
     }
 
     case DECODER_STATE_SEEKING: {
       if (mPendingSeek.Exists()) {
         InitiateSeek();
       }
@@ -3071,19 +3055,16 @@ void MediaDecoderStateMachine::OnAudioSi
 {
   MOZ_ASSERT(OnTaskQueue());
   ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
   MOZ_ASSERT(!mAudioCaptured, "Should be disconnected when capturing audio.");
 
   mAudioSinkPromise.Complete();
   ResyncAudioClock();
   mAudioCompleted = true;
-
-  // Kick the decode thread; it may be sleeping waiting for this to finish.
-  mDecoder->GetReentrantMonitor().NotifyAll();
 }
 
 void MediaDecoderStateMachine::OnAudioSinkError()
 {
   MOZ_ASSERT(OnTaskQueue());
   ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
   MOZ_ASSERT(!mAudioCaptured, "Should be disconnected when capturing audio.");
 
--- a/dom/media/MediaDecoderStateMachine.h
+++ b/dom/media/MediaDecoderStateMachine.h
@@ -107,19 +107,18 @@ extern PRLogModuleInfo* gMediaDecoderLog
 extern PRLogModuleInfo* gMediaSampleLog;
 
 /*
   The state machine class. This manages the decoding and seeking in the
   MediaDecoderReader on the decode task queue, and A/V sync on the shared
   state machine thread, and controls the audio "push" thread.
 
   All internal state is synchronised via the decoder monitor. State changes
-  are either propagated by NotifyAll on the monitor (typically when state
-  changes need to be propagated to non-state machine threads) or by scheduling
-  the state machine to run another cycle on the shared state machine thread.
+  are propagated by scheduling the state machine to run another cycle on the
+  shared state machine thread.
 
   See MediaDecoder.h for more details.
 */
 class MediaDecoderStateMachine
 {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaDecoderStateMachine)
 public:
   typedef MediaDecoderReader::AudioDataPromise AudioDataPromise;
@@ -881,18 +880,16 @@ private:
   // Queue of audio frames. This queue is threadsafe, and is accessed from
   // the audio, decoder, state machine, and main threads.
   MediaQueue<MediaData> mAudioQueue;
   // Queue of video frames. This queue is threadsafe, and is accessed from
   // the decoder, state machine, and main threads.
   MediaQueue<MediaData> mVideoQueue;
 
   // The decoder monitor must be obtained before modifying this state.
-  // NotifyAll on the monitor must be called when the state is changed so
-  // that interested threads can wake up and alter behaviour if appropriate
   // Accessed on state machine, audio, main, and AV thread.
   Watchable<State> mState;
 
   // The task queue in which we run decode tasks. This is referred to as
   // the "decode thread", though in practise tasks can run on a different
   // thread every time they're called.
   TaskQueue* DecodeTaskQueue() const { return mReader->OwnerThread(); }