Bug 1160421 - Replace nsThreadPool with a custom thread pool implementation in DecodePool. r=tn

image/src/DecodePool.cpp

@@ -8,10 +8,12 @@
 #include <algorithm>
 
 #include "mozilla/ClearOnShutdown.h"
+#include "mozilla/Monitor.h"
 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
 #include "nsIObserverService.h"
 #include "nsIThreadPool.h"
+#include "nsThreadManager.h"
 #include "nsThreadUtils.h"
 #include "nsXPCOMCIDInternal.h"
 #include "prsystem.h"
@@ -107,49 +109,8 @@ private:
   nsRefPtr<Decoder> mDecoder;
 };
 
-class DecodeWorker : public nsRunnable
-{
-public:
-  explicit DecodeWorker(Decoder* aDecoder)
-    : mDecoder(aDecoder)
-  {
-    MOZ_ASSERT(mDecoder);
-  }
-
-  NS_IMETHOD Run() override
-  {
-    MOZ_ASSERT(!NS_IsMainThread());
-    DecodePool::Singleton()->Decode(mDecoder);
-    return NS_OK;
-  }
-
-private:
-  nsRefPtr<Decoder> mDecoder;
-};
-
 #ifdef MOZ_NUWA_PROCESS
 
-class DecodePoolNuwaListener final : public nsIThreadPoolListener
-{
-public:
-  NS_DECL_THREADSAFE_ISUPPORTS
-
-  NS_IMETHODIMP OnThreadCreated()
-  {
-    if (IsNuwaProcess()) {
-      NuwaMarkCurrentThread(static_cast<void(*)(void*)>(nullptr), nullptr);
-    }
-    return NS_OK;
-  }
-
-  NS_IMETHODIMP OnThreadShuttingDown() { return NS_OK; }
-
-private:
-  ~DecodePoolNuwaListener() { }
-};
-
-NS_IMPL_ISUPPORTS(DecodePoolNuwaListener, nsIThreadPoolListener)
-
 class RegisterDecodeIOThreadWithNuwaRunnable : public nsRunnable
 {
 public:
@@ -159,6 +120,7 @@ public:
     return NS_OK;
   }
 };
+
 #endif // MOZ_NUWA_PROCESS
 
 
@@ -170,6 +132,166 @@ public:
 
 NS_IMPL_ISUPPORTS(DecodePool, nsIObserver)
 
+struct Work
+{
+  enum class Type {
+    DECODE,
+    SHUTDOWN
+  } mType;
+
+  nsRefPtr<Decoder> mDecoder;
+};
+
+class DecodePoolImpl
+{
+public:
+  MOZ_DECLARE_REFCOUNTED_TYPENAME(DecodePoolImpl)
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodePoolImpl)
+
+  DecodePoolImpl()
+    : mMonitor("DecodePoolImpl")
+    , mShuttingDown(false)
+  { }
+
+  /// Initialize the current thread for use by the decode pool.
+  void InitCurrentThread()
+  {
+    MOZ_ASSERT(!NS_IsMainThread());
+
+    mThreadNaming.SetThreadPoolName(NS_LITERAL_CSTRING("ImgDecoder"));
+
+#ifdef MOZ_NUWA_PROCESS
+    if (IsNuwaProcess()) {
+      NuwaMarkCurrentThread(static_cast<void(*)(void*)>(nullptr), nullptr);
+    }
+#endif // MOZ_NUWA_PROCESS
+  }
+
+  /// Shut down the provided decode pool thread.
+  static void ShutdownThread(nsIThread* aThisThread)
+  {
+    // Threads have to be shut down from another thread, so we'll ask the
+    // main thread to do it for us.
+    nsCOMPtr<nsIRunnable> runnable =
+      NS_NewRunnableMethod(aThisThread, &nsIThread::Shutdown);
+    NS_DispatchToMainThread(runnable);
+  }
+
+  /**
+   * Requests shutdown. New work items will be dropped on the floor, and all
+   * decode pool threads will be shut down once existing work items have been
+   * processed.
+   */
+  void RequestShutdown()
+  {
+    MonitorAutoLock lock(mMonitor);
+    mShuttingDown = true;
+    mMonitor.NotifyAll();
+  }
+
+  /// Pushes a new decode work item.
+  void PushWork(Decoder* aDecoder)
+  {
+    nsRefPtr<Decoder> decoder(aDecoder);
+
+    MonitorAutoLock lock(mMonitor);
+
+    if (mShuttingDown) {
+      // Drop any new work on the floor if we're shutting down.
+      return;
+    }
+
+    mQueue.AppendElement(Move(decoder));
+    mMonitor.Notify();
+  }
+
+  /// Pops a new work item, blocking if necessary.
+  Work PopWork()
+  {
+    Work work;
+
+    MonitorAutoLock lock(mMonitor);
+
+    do {
+      if (!mQueue.IsEmpty()) {
+        // XXX(seth): This is NOT efficient, obviously, since we're removing an
+        // element from the front of the array. However, it's not worth
+        // implementing something better right now, because we are replacing
+        // this FIFO behavior with LIFO behavior very soon.
+        work.mType = Work::Type::DECODE;
+        work.mDecoder = mQueue.ElementAt(0);
+        mQueue.RemoveElementAt(0);
+
+#ifdef MOZ_NUWA_PROCESS
+        nsThreadManager::get()->SetThreadWorking();
+#endif // MOZ_NUWA_PROCESS
+
+        return work;
+      }
+
+      if (mShuttingDown) {
+        work.mType = Work::Type::SHUTDOWN;
+        return work;
+      }
+
+#ifdef MOZ_NUWA_PROCESS
+      nsThreadManager::get()->SetThreadIdle(nullptr);
+#endif // MOZ_NUWA_PROCESS
+
+      // Nothing to do; block until some work is available.
+      mMonitor.Wait();
+    } while (true);
+  }
+
+private:
+  ~DecodePoolImpl() { }
+
+  nsThreadPoolNaming mThreadNaming;
+
+  // mMonitor guards mQueue and mShuttingDown.
+  Monitor mMonitor;
+  nsTArray<nsRefPtr<Decoder>> mQueue;
+  bool mShuttingDown;
+};
+
+class DecodePoolWorker : public nsRunnable
+{
+public:
+  explicit DecodePoolWorker(DecodePoolImpl* aImpl) : mImpl(aImpl) { }
+
+  NS_IMETHOD Run()
+  {
+    MOZ_ASSERT(!NS_IsMainThread());
+
+    mImpl->InitCurrentThread();
+
+    nsCOMPtr<nsIThread> thisThread;
+    nsThreadManager::get()->GetCurrentThread(getter_AddRefs(thisThread));
+
+    do {
+      Work work = mImpl->PopWork();
+      switch (work.mType) {
+        case Work::Type::DECODE:
+          DecodePool::Singleton()->Decode(work.mDecoder);
+          break;
+
+        case Work::Type::SHUTDOWN:
+          DecodePoolImpl::ShutdownThread(thisThread);
+          return NS_OK;
+
+        default:
+          MOZ_ASSERT_UNREACHABLE("Unknown work type");
+      }
+    } while (true);
+
+    MOZ_ASSERT_UNREACHABLE("Exiting thread without Work::Type::SHUTDOWN");
+    return NS_OK;
+  }
+
+private:
+  nsRefPtr<DecodePoolImpl> mImpl;
+};
+
 /* static */ void
 DecodePool::Initialize()
 {
@@ -190,14 +312,10 @@ DecodePool::Singleton()
 }
 
 DecodePool::DecodePool()
-  : mMutex("image::DecodePool")
+  : mImpl(new DecodePoolImpl)
+  , mMutex("image::DecodePool")
 {
-  // Initialize the thread pool.
-  mThreadPool = do_CreateInstance(NS_THREADPOOL_CONTRACTID);
-  MOZ_RELEASE_ASSERT(mThreadPool,
-                     "Should succeed in creating image decoding thread pool");
-
-  mThreadPool->SetName(NS_LITERAL_CSTRING("ImageDecoder"));
+  // Determine the number of threads we want.
   int32_t prefLimit = gfxPrefs::ImageMTDecodingLimit();
   uint32_t limit;
   if (prefLimit <= 0) {
@@ -206,14 +324,15 @@ DecodePool::DecodePool()
     limit = static_cast<uint32_t>(prefLimit);
   }
 
-  mThreadPool->SetThreadLimit(limit);
-  mThreadPool->SetIdleThreadLimit(limit);
-
-#ifdef MOZ_NUWA_PROCESS
-  if (IsNuwaProcess()) {
-    mThreadPool->SetListener(new DecodePoolNuwaListener());
+  // Initialize the thread pool.
+  for (uint32_t i = 0 ; i < limit ; ++i) {
+    nsCOMPtr<nsIRunnable> worker = new DecodePoolWorker(mImpl);
+    nsCOMPtr<nsIThread> thread;
+    nsresult rv = NS_NewThread(getter_AddRefs(thread), worker);
+    MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv) && thread,
+                       "Should successfully create image decoding threads");
+    mThreads.AppendElement(Move(thread));
   }
-#endif
 
   // Initialize the I/O thread.
   nsresult rv = NS_NewNamedThread("ImageIO", getter_AddRefs(mIOThread));
@@ -243,17 +362,20 @@ DecodePool::Observe(nsISupports*, const char* aTopic, const char16_t*)
 {
   MOZ_ASSERT(strcmp(aTopic, "xpcom-shutdown-threads") == 0, "Unexpected topic");
 
-  nsCOMPtr<nsIThreadPool> threadPool;
+  nsCOMArray<nsIThread> threads;
   nsCOMPtr<nsIThread> ioThread;
 
   {
     MutexAutoLock lock(mMutex);
-    threadPool.swap(mThreadPool);
+    threads.AppendElements(mThreads);
+    mThreads.Clear();
     ioThread.swap(mIOThread);
   }
 
-  if (threadPool) {
-    threadPool->Shutdown();
+  mImpl->RequestShutdown();
+
+  for (int32_t i = 0 ; i < threads.Count() ; ++i) {
+    threads[i]->Shutdown();
   }
 
   if (ioThread) {
@@ -267,15 +389,7 @@ void
 DecodePool::AsyncDecode(Decoder* aDecoder)
 {
   MOZ_ASSERT(aDecoder);
-
-  nsCOMPtr<nsIRunnable> worker = new DecodeWorker(aDecoder);
-
-  // Dispatch to the thread pool if it exists. If it doesn't, we're currently
-  // shutting down, so it's OK to just drop the job on the floor.
-  MutexAutoLock threadPoolLock(mMutex);
-  if (mThreadPool) {
-    mThreadPool->Dispatch(worker, nsIEventTarget::DISPATCH_NORMAL);
-  }
+  mImpl->PushWork(aDecoder);
 }
 
 void
@@ -299,14 +413,6 @@ DecodePool::SyncDecodeIfPossible(Decoder* aDecoder)
   Decode(aDecoder);
 }
 
-already_AddRefed<nsIEventTarget>
-DecodePool::GetEventTarget()
-{
-  MutexAutoLock threadPoolLock(mMutex);
-  nsCOMPtr<nsIEventTarget> target = do_QueryInterface(mThreadPool);
-  return target.forget();
-}
-
 already_AddRefed<nsIEventTarget>
 DecodePool::GetIOEventTarget()
 {
@@ -315,14 +421,6 @@ DecodePool::GetIOEventTarget()
   return target.forget();
 }
 
-already_AddRefed<nsIRunnable>
-DecodePool::CreateDecodeWorker(Decoder* aDecoder)
-{
-  MOZ_ASSERT(aDecoder);
-  nsCOMPtr<nsIRunnable> worker = new DecodeWorker(aDecoder);
-  return worker.forget();
-}
-
 void
 DecodePool::Decode(Decoder* aDecoder)
 {

image/src/DecodePool.h

@@ -12,9 +12,11 @@
 
 #include "mozilla/Mutex.h"
 #include "mozilla/StaticPtr.h"
+#include "nsCOMArray.h"
 #include "nsCOMPtr.h"
 #include "nsIEventTarget.h"
 #include "nsIObserver.h"
+#include "nsRefPtr.h"
 
 class nsIThread;
 class nsIThreadPool;
@@ -23,6 +25,7 @@ namespace mozilla {
 namespace image {
 
 class Decoder;
+class DecodePoolImpl;
 
 /**
  * DecodePool is a singleton class that manages decoding of raster images. It
@@ -63,15 +66,6 @@ public:
    */
   void SyncDecodeIfPossible(Decoder* aDecoder);
 
-  /**
-   * Returns an event target interface to the DecodePool's underlying thread
-   * pool. Callers can use this event target to submit work to the image
-   * decoding thread pool.
-   *
-   * @return An nsIEventTarget interface to the thread pool.
-   */
-  already_AddRefed<nsIEventTarget> GetEventTarget();
-
   /**
    * Returns an event target interface to the DecodePool's I/O thread. Callers
    * who want to deliver data to workers on the DecodePool can use this event
@@ -81,17 +75,8 @@ public:
    */
   already_AddRefed<nsIEventTarget> GetIOEventTarget();
 
-  /**
-   * Creates a worker which can be used to attempt further decoding using the
-   * provided decoder.
-   *
-   * @return The new worker, which should be posted to the event target returned
-   *         by GetEventTarget.
-   */
-  already_AddRefed<nsIRunnable> CreateDecodeWorker(Decoder* aDecoder);
-
 private:
-  friend class DecodeWorker;
+  friend class DecodePoolWorker;
 
   DecodePool();
   virtual ~DecodePool();
@@ -102,9 +87,11 @@ private:
 
   static StaticRefPtr<DecodePool> sSingleton;
 
-  // mMutex protects mThreadPool and mIOThread.
+  nsRefPtr<DecodePoolImpl>    mImpl;
+
+  // mMutex protects mThreads and mIOThread.
   Mutex                     mMutex;
-  nsCOMPtr<nsIThreadPool>   mThreadPool;
+  nsCOMArray<nsIThread>     mThreads;
   nsCOMPtr<nsIThread>       mIOThread;
 };
 

image/src/Decoder.cpp

@@ -166,15 +166,7 @@ Decoder::Resume()
 {
   DecodePool* decodePool = DecodePool::Singleton();
   MOZ_ASSERT(decodePool);
-
-  nsCOMPtr<nsIEventTarget> target = decodePool->GetEventTarget();
-  if (MOZ_UNLIKELY(!target)) {
-    // We're shutting down and the DecodePool's thread pool has been destroyed.
-    return;
-  }
-
-  nsCOMPtr<nsIRunnable> worker = decodePool->CreateDecodeWorker(this);
-  target->Dispatch(worker, nsIEventTarget::DISPATCH_NORMAL);
+  decodePool->AsyncDecode(this);
 }
 
 bool

image/src/moz.build

@@ -68,6 +68,8 @@ LOCAL_INCLUDES += [
     '/layout/svg',
     # For URI-related functionality
     '/netwerk/base',
+    # DecodePool uses thread-related facilities.
+    '/xpcom/threads',
 ]
 
 # Because imgFrame.cpp includes "cairo.h"