b=928222 remove nsThreadPool per-thread event queues r=bsmedberg
☠☠ backed out by 5a9ac6fed6ff ☠ ☠
authorKarl Tomlinson <karlt+@karlt.net>
Thu, 24 Oct 2013 09:31:15 +1300
changeset 166665 242bb227928364796b91ede50e3ae8d6d1bf56af
parent 166664 95ec386c179cad702ff3b1571df56c4a7df4abdc
child 166666 b8c97df0418de30a4f189a65793898cc0393b967
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs928222
milestone27.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
b=928222 remove nsThreadPool per-thread event queues r=bsmedberg The way idle nsThreadPool threads wait with a timeout doesn't work well with shutdown from nsThreadManager. nsThreadPool doesn't need to use nsIThread for its threads. The nsIThread interface is not useful for threads running in an nsThreadPool. The nsIEventTarget on the nsIThreadPool should be used for dispatching events, not an interface on the individual threads, and the threads don't need an nsEventQueue because they use the nsEventQueue on the nsThreadPool. Shutdown of single event threads is easier than running nested event loops for nsIThreads, avoiding the multilevel nested event loop situations when several threads finish and are shutdown. While the ThreadFunc is running, a nested event loop is still used in Shutdown() in case some consumers might need it and because that is the documented API. This also simplifies thread creation, avoiding races that could mean there was temporarily an extra thread or more.
xpcom/threads/nsThreadPool.cpp
xpcom/threads/nsThreadPool.h
--- a/xpcom/threads/nsThreadPool.cpp
+++ b/xpcom/threads/nsThreadPool.cpp
@@ -7,16 +7,17 @@
 #include "nsIClassInfoImpl.h"
 #include "nsThreadPool.h"
 #include "nsThreadManager.h"
 #include "nsThread.h"
 #include "nsMemory.h"
 #include "nsAutoPtr.h"
 #include "prinrval.h"
 #include "prlog.h"
+#include "mozilla/DebugOnly.h"
 
 using namespace mozilla;
 
 #ifdef PR_LOGGING
 static PRLogModuleInfo *
 GetThreadPoolLog()
 {
   static PRLogModuleInfo *sLog;
@@ -36,18 +37,17 @@ GetThreadPoolLog()
 #define DEFAULT_THREAD_LIMIT 4
 #define DEFAULT_IDLE_THREAD_LIMIT 1
 #define DEFAULT_IDLE_THREAD_TIMEOUT PR_SecondsToInterval(60)
 
 NS_IMPL_ADDREF(nsThreadPool)
 NS_IMPL_RELEASE(nsThreadPool)
 NS_IMPL_CLASSINFO(nsThreadPool, nullptr, nsIClassInfo::THREADSAFE,
                   NS_THREADPOOL_CID)
-NS_IMPL_QUERY_INTERFACE3_CI(nsThreadPool, nsIThreadPool, nsIEventTarget,
-                            nsIRunnable)
+NS_IMPL_QUERY_INTERFACE2_CI(nsThreadPool, nsIThreadPool, nsIEventTarget)
 NS_IMPL_CI_INTERFACE_GETTER2(nsThreadPool, nsIThreadPool, nsIEventTarget)
 
 nsThreadPool::nsThreadPool()
   : mThreadLimit(DEFAULT_THREAD_LIMIT)
   , mIdleThreadLimit(DEFAULT_IDLE_THREAD_LIMIT)
   , mIdleThreadTimeout(DEFAULT_IDLE_THREAD_TIMEOUT)
   , mIdleCount(0)
   , mShutdown(false)
@@ -65,103 +65,122 @@ nsresult
 nsThreadPool::PutEvent(nsIRunnable *event)
 {
   // Avoid spawning a new thread while holding the event queue lock...
  
   bool spawnThread = false;
   {
     ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
 
-    LOG(("THRD-P(%p) put [%d %d %d]\n", this, mIdleCount, mThreads.Count(),
+    LOG(("THRD-P(%p) put [%d %u %d]\n", this, mIdleCount, mThreads.Length(),
          mThreadLimit));
-    MOZ_ASSERT(mIdleCount <= (uint32_t) mThreads.Count(), "oops");
+    MOZ_ASSERT(mIdleCount <= mThreads.Length(), "oops");
 
     // Make sure we have a thread to service this event.
-    if (mIdleCount == 0 && mThreads.Count() < (int32_t) mThreadLimit)
+    if (mIdleCount == 0 && mThreads.Length() < mThreadLimit) {
+      // Run() will replace this with the real PRThread*
+      mThreads.AppendElement<PRThread*>(nullptr);
       spawnThread = true;
-
-    mEvents.PutEvent(event);
+    } else {
+      mEvents.PutEvent(event);
+    }
   }
 
   LOG(("THRD-P(%p) put [spawn=%d]\n", this, spawnThread));
   if (!spawnThread)
     return NS_OK;
 
-  nsCOMPtr<nsIThread> thread;
-  nsThreadManager::get()->NewThread(0,
-                                    nsIThreadManager::DEFAULT_STACK_SIZE,
-                                    getter_AddRefs(thread));
-  NS_ENSURE_STATE(thread);
+  NS_ADDREF_THIS(); // Released in ThreadFunc
+  PRThread* thread = PR_CreateThread(PR_USER_THREAD, ThreadFunc, this,
+                                     PR_PRIORITY_NORMAL, PR_GLOBAL_THREAD,
+                                     PR_JOINABLE_THREAD, 0);
 
-  bool killThread = false;
   {
     ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
-    if (mThreads.Count() < (int32_t) mThreadLimit) {
-      mThreads.AppendObject(thread);
-    } else {
-      killThread = true;  // okay, we don't need this thread anymore
+    if (!thread) {
+      NS_WARNING("PR_CreateThread() failed");
+      NS_RELEASE_THIS();
+      mThreads.RemoveElement<PRThread*>(nullptr);
+      if (mThreads.IsEmpty())
+        return NS_ERROR_OUT_OF_MEMORY;
     }
-  }
-  LOG(("THRD-P(%p) put [%p kill=%d]\n", this, thread.get(), killThread));
-  if (killThread) {
-    // Pending events are processed on the current thread during
-    // nsIThread::Shutdown() execution, so if nsThreadPool::Dispatch() is called
-    // under caller's lock then deadlock could occur. This happens e.g. in case
-    // of nsStreamCopier. To prevent this situation, dispatch a shutdown event
-    // to the current thread instead of calling nsIThread::Shutdown() directly.
 
-    nsRefPtr<nsIRunnable> r = NS_NewRunnableMethod(thread,
-                                                   &nsIThread::Shutdown);
-    NS_DispatchToCurrentThread(r);
-  } else {
-    thread->Dispatch(this, NS_DISPATCH_NORMAL);
+    mEvents.PutEvent(event);
   }
 
   return NS_OK;
 }
 
 void
-nsThreadPool::ShutdownThread(nsIThread *thread)
+nsThreadPool::ShutdownThread(PRThread *thread)
 {
   LOG(("THRD-P(%p) shutdown async [%p]\n", this, thread));
 
   // This method is responsible for calling Shutdown on |thread|.  This must be
   // done from some other thread, so we use the main thread of the application.
 
   MOZ_ASSERT(!NS_IsMainThread(), "wrong thread");
 
-  nsRefPtr<nsIRunnable> r = NS_NewRunnableMethod(thread, &nsIThread::Shutdown);
+  class JoinEvent MOZ_FINAL : public nsRunnable
+  {
+  public:
+    explicit JoinEvent(PRThread *thread) : mThread(thread) { }
+
+    NS_IMETHODIMP Run() MOZ_OVERRIDE
+    {
+      DebugOnly<PRStatus> status = PR_JoinThread(mThread);
+      MOZ_ASSERT(status == PR_SUCCESS, "PR_JoinThread failed");
+      return NS_OK;
+    }
+
+  private:
+    PRThread *mThread;
+  };
+
+  nsRefPtr<nsIRunnable> r = new JoinEvent(thread);
   NS_DispatchToMainThread(r);
 }
 
-NS_IMETHODIMP
+/*static*/ void
+nsThreadPool::ThreadFunc(void *arg)
+{
+  auto self = static_cast<nsThreadPool*>(arg);  // strong reference
+  self->Run();
+  NS_RELEASE(self); // Reference added on thread creation
+}
+
+void
 nsThreadPool::Run()
 {
   LOG(("THRD-P(%p) enter\n", this));
 
   mThreadNaming.SetThreadPoolName(mName);
 
-  nsCOMPtr<nsIThread> current;
-  nsThreadManager::get()->GetCurrentThread(getter_AddRefs(current));
-
-  bool shutdownThreadOnExit = false;
-  bool exitThread = false;
-  bool wasIdle = false;
-  PRIntervalTime idleSince;
+  PRThread* current = PR_GetCurrentThread();
 
   nsCOMPtr<nsIThreadPoolListener> listener;
   {
     ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+
+    auto index = mThreads.IndexOf<PRThread*>(nullptr);
+    MOZ_ASSERT(index != mThreads.NoIndex, "mThreads entry has gone!");
+    mThreads[index] = current;
+
     listener = mListener;
   }
 
   if (listener) {
     listener->OnThreadCreated();
   }
 
+  bool shutdownThreadOnExit = false;
+  bool exitThread = false;
+  bool wasIdle = false;
+  PRIntervalTime idleSince;
+
   do {
     nsCOMPtr<nsIRunnable> event;
     {
       ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
       if (!mEvents.GetPendingEvent(getter_AddRefs(event))) {
         PRIntervalTime now     = PR_IntervalNow();
         PRIntervalTime timeout = PR_MillisecondsToInterval(mIdleThreadTimeout);
 
@@ -183,17 +202,17 @@ nsThreadPool::Run()
               wasIdle = true;
             }
           }
         }
 
         if (exitThread) {
           if (wasIdle)
             --mIdleCount;
-          shutdownThreadOnExit = mThreads.RemoveObject(current);
+          shutdownThreadOnExit = mThreads.RemoveElement(current);
         } else {
           PRIntervalTime delta = timeout - (now - idleSince);
           LOG(("THRD-P(%p) waiting [%d]\n", this, delta));
           mon.Wait(delta);
         }
       } else if (wasIdle) {
         wasIdle = false;
         --mIdleCount;
@@ -209,17 +228,17 @@ nsThreadPool::Run()
     listener->OnThreadShuttingDown();
   }
 
   if (shutdownThreadOnExit) {
     ShutdownThread(current);
   }
 
   LOG(("THRD-P(%p) leave\n", this));
-  return NS_OK;
+  return;
 }
 
 NS_IMETHODIMP
 nsThreadPool::Dispatch(nsIRunnable *event, uint32_t flags)
 {
   LOG(("THRD-P(%p) dispatch [%p %x]\n", this, event, flags));
 
   NS_ENSURE_STATE(!mShutdown);
@@ -251,38 +270,37 @@ nsThreadPool::IsOnCurrentThread(bool *re
 
   *result = false;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::Shutdown()
 {
-  nsCOMArray<nsIThread> threads;
+  nsTArray<PRThread*> threads;
   nsCOMPtr<nsIThreadPoolListener> listener;
   {
     ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
     mShutdown = true;
     mon.NotifyAll();
 
-    threads.AppendObjects(mThreads);
-    mThreads.Clear();
-
     // Swap in a null listener so that we release the listener at the end of
     // this method. The listener will be kept alive as long as the other threads
     // that were created when it was set.
     mListener.swap(listener);
+
+    while (mThreads.Length()) {
+      // It's important that we shutdown the threads while outside the event
+      // queue monitor.  Otherwise, we could end up dead-locking.
+      ReentrantMonitorAutoExit mon(mEvents.GetReentrantMonitor());
+      // ShutdownThread() will ensure there is another event to process
+      NS_ProcessNextEvent();
+    }
   }
 
-  // It's important that we shutdown the threads while outside the event queue
-  // monitor.  Otherwise, we could end up dead-locking.
-
-  for (int32_t i = 0; i < threads.Count(); ++i)
-    threads[i]->Shutdown();
-
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::GetThreadLimit(uint32_t *value)
 {
   *value = mThreadLimit;
   return NS_OK;
@@ -291,17 +309,17 @@ nsThreadPool::GetThreadLimit(uint32_t *v
 NS_IMETHODIMP
 nsThreadPool::SetThreadLimit(uint32_t value)
 {
   ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
   mThreadLimit = value;
   if (mIdleThreadLimit > mThreadLimit)
     mIdleThreadLimit = mThreadLimit;
 
-  if (static_cast<uint32_t>(mThreads.Count()) > mThreadLimit) {
+  if (mThreads.Length() > mThreadLimit) {
     mon.NotifyAll();  // wake up threads so they observe this change
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::GetIdleThreadLimit(uint32_t *value)
 {
@@ -364,15 +382,15 @@ nsThreadPool::SetListener(nsIThreadPoolL
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::SetName(const nsACString& aName)
 {
   {
     ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
-    if (mThreads.Count())
+    if (mThreads.Length())
       return NS_ERROR_NOT_AVAILABLE;
   }
 
   mName = aName;
   return NS_OK;
 }
--- a/xpcom/threads/nsThreadPool.h
+++ b/xpcom/threads/nsThreadPool.h
@@ -11,34 +11,34 @@
 #include "nsIThread.h"
 #include "nsIRunnable.h"
 #include "nsEventQueue.h"
 #include "nsCOMArray.h"
 #include "nsCOMPtr.h"
 #include "nsThreadUtils.h"
 #include "mozilla/Attributes.h"
 
-class nsThreadPool MOZ_FINAL : public nsIThreadPool,
-                               public nsIRunnable
+class nsThreadPool MOZ_FINAL : public nsIThreadPool
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIEVENTTARGET
   NS_DECL_NSITHREADPOOL
-  NS_DECL_NSIRUNNABLE
 
   nsThreadPool();
 
 private:
   ~nsThreadPool();
 
-  void ShutdownThread(nsIThread *thread);
+  void ShutdownThread(PRThread *thread);
+  static void ThreadFunc(void *arg);
+  void Run();
   nsresult PutEvent(nsIRunnable *event);
 
-  nsCOMArray<nsIThread> mThreads;
+  nsTArray<PRThread*>   mThreads;
   nsEventQueue          mEvents;
   uint32_t              mThreadLimit;
   uint32_t              mIdleThreadLimit;
   uint32_t              mIdleThreadTimeout;
   uint32_t              mIdleCount;
   nsCOMPtr<nsIThreadPoolListener> mListener;
   bool                  mShutdown;
   nsCString             mName;