Bug 1444537 - Part 2. Shutting down the decode pool should make animated decoders bail early. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 27 Mar 2018 10:57:01 -0400
changeset 410191 1c5d4e9652092cb2cbec3c42656486a09b84ec9f
parent 410190 4d66a4a931e4dccedce652d3d93799c685dd3eb2
child 410192 17fd70df9e43f40186b40b91760001848d133d51
push id101408
push useraosmond@gmail.com
push dateTue, 27 Mar 2018 14:57:15 +0000
treeherdermozilla-inbound@1c5d4e965209 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1444537
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 1444537 - Part 2. Shutting down the decode pool should make animated decoders bail early. r=tnikkel 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);
 
@@ -237,17 +243,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;
@@ -427,16 +433,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.