Bug 1444537 - Part 2: Shutting down the decode pool should make animated decoders bail early. r=tnikkel, a=jcristau
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 27 Mar 2018 10:57:01 -0400
changeset 462912 a9b81a287bd326bbb6df92092d59b9bcb66db73d
parent 462911 9c6d282fa4b4c91ddfd7054e95cff07eecaa893c
child 462913 901288518dfd72bef560f8003f2eabcf75767aad
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel, jcristau
bugs1444537
milestone60.0
Bug 1444537 - Part 2: Shutting down the decode pool should make animated decoders bail early. r=tnikkel, a=jcristau When we shutdown the decode pool threads, it does not do a simple join with the main thread. It will actually process the main thread event loop, which can cause a bad series of events. The refresh tick could still be running and advancing our animated images, causing the animated decoders to continue running, which in turn prevents the decoder threads from finishing shutting down, and the main thread from joining them. Now we check on each frame whether or not the decoder should just stop decoding more frames because the decode pool has started shutdown. If it has, it will stop immediately.
image/AnimationSurfaceProvider.cpp
image/DecodePool.cpp
image/DecodePool.h
--- a/image/AnimationSurfaceProvider.cpp
+++ b/image/AnimationSurfaceProvider.cpp
@@ -289,17 +289,20 @@ AnimationSurfaceProvider::CheckForNewFra
       justGotFirstFrame = true;
     }
   }
 
   if (justGotFirstFrame) {
     AnnounceSurfaceAvailable();
   }
 
-  return continueDecoding;
+  // If we are shutting down, we want to ensure we release the thread as soon
+  // as possible. The animation may advance even during shutdown, which keeps
+  // us decoding, and thus blocking the decode pool during teardown.
+  return continueDecoding && !DecodePool::Singleton()->IsShuttingDown();
 }
 
 bool
 AnimationSurfaceProvider::CheckForNewFrameAtTerminalState()
 {
   mDecodingMutex.AssertCurrentThreadOwns();
   MOZ_ASSERT(mDecoder);
 
@@ -339,17 +342,20 @@ AnimationSurfaceProvider::CheckForNewFra
       justGotFirstFrame = true;
     }
   }
 
   if (justGotFirstFrame) {
     AnnounceSurfaceAvailable();
   }
 
-  return continueDecoding;
+  // If we are shutting down, we want to ensure we release the thread as soon
+  // as possible. The animation may advance even during shutdown, which keeps
+  // us decoding, and thus blocking the decode pool during teardown.
+  return continueDecoding && !DecodePool::Singleton()->IsShuttingDown();
 }
 
 void
 AnimationSurfaceProvider::AnnounceSurfaceAvailable()
 {
   mFramesMutex.AssertNotCurrentThreadOwns();
   MOZ_ASSERT(mImage);
 
--- a/image/DecodePool.cpp
+++ b/image/DecodePool.cpp
@@ -112,16 +112,22 @@ public:
       mMonitor.NotifyAll();
     }
 
     for (uint32_t i = 0 ; i < threads.Length() ; ++i) {
       threads[i]->Shutdown();
     }
   }
 
+  bool IsShuttingDown() const
+  {
+    MonitorAutoLock lock(mMonitor);
+    return mShuttingDown;
+  }
+
   /// Pushes a new decode work item.
   void PushWork(IDecodingTask* aTask)
   {
     MOZ_ASSERT(aTask);
     RefPtr<IDecodingTask> task(aTask);
 
     MonitorAutoLock lock(mMonitor);
 
@@ -238,17 +244,17 @@ private:
     Work work;
     work.mType = Work::Type::SHUTDOWN;
     return work;
   }
 
   nsThreadPoolNaming mThreadNaming;
 
   // mMonitor guards everything below.
-  Monitor mMonitor;
+  mutable Monitor mMonitor;
   nsTArray<RefPtr<IDecodingTask>> mHighPriorityQueue;
   nsTArray<RefPtr<IDecodingTask>> mLowPriorityQueue;
   nsTArray<nsCOMPtr<nsIThread>> mThreads;
   PRIntervalTime mIdleTimeout;
   uint8_t mMaxIdleThreads;   // Maximum number of workers when idle.
   uint8_t mAvailableThreads; // How many new threads can be created.
   uint8_t mIdleThreads; // How many created threads are waiting.
   bool mShuttingDown;
@@ -428,16 +434,22 @@ DecodePool::Observe(nsISupports*, const 
 
   if (ioThread) {
     ioThread->Shutdown();
   }
 
   return NS_OK;
 }
 
+bool
+DecodePool::IsShuttingDown() const
+{
+  return mImpl->IsShuttingDown();
+}
+
 void
 DecodePool::AsyncRun(IDecodingTask* aTask)
 {
   MOZ_ASSERT(aTask);
   mImpl->PushWork(aTask);
 }
 
 bool
--- a/image/DecodePool.h
+++ b/image/DecodePool.h
@@ -50,16 +50,20 @@ public:
 
   /// Returns the singleton instance.
   static DecodePool* Singleton();
 
   /// @return the number of processor cores we have available. This is not the
   /// same as the number of decoding threads we're actually using.
   static uint32_t NumberOfCores();
 
+  /// True if the DecodePool is being shutdown. This may only be called by
+  /// threads from the pool to check if they should keep working or not.
+  bool IsShuttingDown() const;
+
   /// Ask the DecodePool to run @aTask asynchronously and return immediately.
   void AsyncRun(IDecodingTask* aTask);
 
   /**
    * Run @aTask synchronously if the task would prefer it. It's up to the task
    * itself to make this decision; @see IDecodingTask::ShouldPreferSyncRun(). If
    * @aTask doesn't prefer it, just run @aTask asynchronously and return
    * immediately.