Bug 1195767 - part 3 - modify nsThreadPool to use a non-reentrant monitor; r=gerald, a=sledru
authorNathan Froyd <froydnj@mozilla.com>
Thu, 03 Sep 2015 15:38:28 -0400
changeset 289246 2adf669d9a575282d011222bc66f9a2f46858033
parent 289245 92bbcea7ea93c041898d0e10ff3cf4daea623261
child 289247 27de5abd6cff7fc9a6b60ed2cef67872681f48d8
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)
reviewersgerald, sledru
bugs1195767
milestone42.0a2
Bug 1195767 - part 3 - modify nsThreadPool to use a non-reentrant monitor; r=gerald, a=sledru There's no reason nsThreadPool needs to use a reentrant monitor for locking its event queue. Having it use a non-reentrant one should be slightly more efficient, both in the general operation of the monitor, and that we're not performing redundant locking in methods like nsThreadPool::Run. This change also eliminates the only usage of nsEventQueue::GetReentrantMonitor.
xpcom/threads/nsThreadPool.cpp
xpcom/threads/nsThreadPool.h
--- a/xpcom/threads/nsThreadPool.cpp
+++ b/xpcom/threads/nsThreadPool.cpp
@@ -43,23 +43,25 @@ NS_IMPL_ADDREF(nsThreadPool)
 NS_IMPL_RELEASE(nsThreadPool)
 NS_IMPL_CLASSINFO(nsThreadPool, nullptr, nsIClassInfo::THREADSAFE,
                   NS_THREADPOOL_CID)
 NS_IMPL_QUERY_INTERFACE_CI(nsThreadPool, nsIThreadPool, nsIEventTarget,
                            nsIRunnable)
 NS_IMPL_CI_INTERFACE_GETTER(nsThreadPool, nsIThreadPool, nsIEventTarget)
 
 nsThreadPool::nsThreadPool()
-  : mThreadLimit(DEFAULT_THREAD_LIMIT)
+  : mMonitor("[nsThreadPool.mMonitor]")
+  , mThreadLimit(DEFAULT_THREAD_LIMIT)
   , mIdleThreadLimit(DEFAULT_IDLE_THREAD_LIMIT)
   , mIdleThreadTimeout(DEFAULT_IDLE_THREAD_TIMEOUT)
   , mIdleCount(0)
   , mStackSize(nsIThreadManager::DEFAULT_STACK_SIZE)
   , mShutdown(false)
 {
+  LOG(("THRD-P(%p) constructor!!!\n", this));
 }
 
 nsThreadPool::~nsThreadPool()
 {
   // Threads keep a reference to the nsThreadPool until they return from Run()
   // after removing themselves from mThreads.
   MOZ_ASSERT(mThreads.IsEmpty());
 }
@@ -74,34 +76,34 @@ nsThreadPool::PutEvent(nsIRunnable* aEve
 nsresult
 nsThreadPool::PutEvent(already_AddRefed<nsIRunnable>&& aEvent)
 {
   // Avoid spawning a new thread while holding the event queue lock...
 
   bool spawnThread = false;
   uint32_t stackSize = 0;
   {
-    ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+    MonitorAutoLock mon(mMonitor);
 
     if (NS_WARN_IF(mShutdown)) {
       return NS_ERROR_NOT_AVAILABLE;
     }
     LOG(("THRD-P(%p) put [%d %d %d]\n", this, mIdleCount, mThreads.Count(),
          mThreadLimit));
     MOZ_ASSERT(mIdleCount <= (uint32_t)mThreads.Count(), "oops");
 
     // Make sure we have a thread to service this event.
     if (mThreads.Count() < (int32_t)mThreadLimit &&
         // Spawn a new thread if we don't have enough idle threads to serve
         // pending events immediately.
-        mEvents.Count() >= mIdleCount) {
+        mEvents.Count(mon) >= mIdleCount) {
       spawnThread = true;
     }
 
-    mEvents.PutEvent(Move(aEvent));
+    mEvents.PutEvent(Move(aEvent), mon);
     stackSize = mStackSize;
   }
 
   LOG(("THRD-P(%p) put [spawn=%d]\n", this, spawnThread));
   if (!spawnThread) {
     return NS_OK;
   }
 
@@ -110,17 +112,17 @@ nsThreadPool::PutEvent(already_AddRefed<
                                     stackSize,
                                     getter_AddRefs(thread));
   if (NS_WARN_IF(!thread)) {
     return NS_ERROR_UNEXPECTED;
   }
 
   bool killThread = false;
   {
-    ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+    MonitorAutoLock mon(mMonitor);
     if (mThreads.Count() < (int32_t)mThreadLimit) {
       mThreads.AppendObject(thread);
     } else {
       killThread = true;  // okay, we don't need this thread anymore
     }
   }
   LOG(("THRD-P(%p) put [%p kill=%d]\n", this, thread.get(), killThread));
   if (killThread) {
@@ -152,43 +154,43 @@ nsThreadPool::ShutdownThread(nsIThread* 
 
   nsCOMPtr<nsIRunnable> r = NS_NewRunnableMethod(aThread, &nsIThread::Shutdown);
   NS_DispatchToMainThread(r);
 }
 
 NS_IMETHODIMP
 nsThreadPool::Run()
 {
-  LOG(("THRD-P(%p) enter\n", this));
+  mThreadNaming.SetThreadPoolName(mName);
 
-  mThreadNaming.SetThreadPoolName(mName);
+  LOG(("THRD-P(%p) enter %s\n", this, mName.BeginReading()));
 
   nsCOMPtr<nsIThread> current;
   nsThreadManager::get()->GetCurrentThread(getter_AddRefs(current));
 
   bool shutdownThreadOnExit = false;
   bool exitThread = false;
   bool wasIdle = false;
   PRIntervalTime idleSince;
 
   nsCOMPtr<nsIThreadPoolListener> listener;
   {
-    ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+    MonitorAutoLock mon(mMonitor);
     listener = mListener;
   }
 
   if (listener) {
     listener->OnThreadCreated();
   }
 
   do {
     nsCOMPtr<nsIRunnable> event;
     {
-      ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
-      if (!mEvents.GetPendingEvent(getter_AddRefs(event))) {
+      MonitorAutoLock mon(mMonitor);
+      if (!mEvents.GetPendingEvent(getter_AddRefs(event), mon)) {
         PRIntervalTime now     = PR_IntervalNow();
         PRIntervalTime timeout = PR_MillisecondsToInterval(mIdleThreadTimeout);
 
         // If we are shutting down, then don't keep any idle threads
         if (mShutdown) {
           exitThread = true;
         } else {
           if (wasIdle) {
@@ -210,26 +212,27 @@ nsThreadPool::Run()
 
         if (exitThread) {
           if (wasIdle) {
             --mIdleCount;
           }
           shutdownThreadOnExit = mThreads.RemoveObject(current);
         } else {
           PRIntervalTime delta = timeout - (now - idleSince);
-          LOG(("THRD-P(%p) waiting [%d]\n", this, delta));
+          LOG(("THRD-P(%p) %s waiting [%d]\n", this, mName.BeginReading(), delta));
           mon.Wait(delta);
+          LOG(("THRD-P(%p) done waiting\n", this));
         }
       } else if (wasIdle) {
         wasIdle = false;
         --mIdleCount;
       }
     }
     if (event) {
-      LOG(("THRD-P(%p) running [%p]\n", this, event.get()));
+      LOG(("THRD-P(%p) %s running [%p]\n", this, mName.BeginReading(), event.get()));
       event->Run();
     }
   } while (!exitThread);
 
   if (listener) {
     listener->OnThreadShuttingDown();
   }
 
@@ -276,17 +279,17 @@ nsThreadPool::Dispatch(already_AddRefed<
     PutEvent(Move(aEvent));
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::IsOnCurrentThread(bool* aResult)
 {
-  ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+  MonitorAutoLock mon(mMonitor);
   if (NS_WARN_IF(mShutdown)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   nsIThread* thread = NS_GetCurrentThread();
   for (uint32_t i = 0; i < static_cast<uint32_t>(mThreads.Count()); ++i) {
     if (mThreads[i] == thread) {
       *aResult = true;
@@ -298,17 +301,17 @@ nsThreadPool::IsOnCurrentThread(bool* aR
 }
 
 NS_IMETHODIMP
 nsThreadPool::Shutdown()
 {
   nsCOMArray<nsIThread> threads;
   nsCOMPtr<nsIThreadPoolListener> listener;
   {
-    ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+    MonitorAutoLock mon(mMonitor);
     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
@@ -331,17 +334,18 @@ nsThreadPool::GetThreadLimit(uint32_t* a
 {
   *aValue = mThreadLimit;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::SetThreadLimit(uint32_t aValue)
 {
-  ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+  MonitorAutoLock mon(mMonitor);
+  LOG(("THRD-P(%p) thread limit [%u]\n", this, aValue));
   mThreadLimit = aValue;
   if (mIdleThreadLimit > mThreadLimit) {
     mIdleThreadLimit = mThreadLimit;
   }
 
   if (static_cast<uint32_t>(mThreads.Count()) > mThreadLimit) {
     mon.NotifyAll();  // wake up threads so they observe this change
   }
@@ -353,17 +357,18 @@ nsThreadPool::GetIdleThreadLimit(uint32_
 {
   *aValue = mIdleThreadLimit;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::SetIdleThreadLimit(uint32_t aValue)
 {
-  ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+  MonitorAutoLock mon(mMonitor);
+  LOG(("THRD-P(%p) idle thread limit [%u]\n", this, aValue));
   mIdleThreadLimit = aValue;
   if (mIdleThreadLimit > mThreadLimit) {
     mIdleThreadLimit = mThreadLimit;
   }
 
   // Do we need to kill some idle threads?
   if (mIdleCount > mIdleThreadLimit) {
     mon.NotifyAll();  // wake up threads so they observe this change
@@ -376,67 +381,67 @@ nsThreadPool::GetIdleThreadTimeout(uint3
 {
   *aValue = mIdleThreadTimeout;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::SetIdleThreadTimeout(uint32_t aValue)
 {
-  ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+  MonitorAutoLock mon(mMonitor);
   uint32_t oldTimeout = mIdleThreadTimeout;
   mIdleThreadTimeout = aValue;
 
   // Do we need to notify any idle threads that their sleep time has shortened?
   if (mIdleThreadTimeout < oldTimeout && mIdleCount > 0) {
     mon.NotifyAll();  // wake up threads so they observe this change
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::GetThreadStackSize(uint32_t* aValue)
 {
-  ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+  MonitorAutoLock mon(mMonitor);
   *aValue = mStackSize;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::SetThreadStackSize(uint32_t aValue)
 {
-  ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+  MonitorAutoLock mon(mMonitor);
   mStackSize = aValue;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::GetListener(nsIThreadPoolListener** aListener)
 {
-  ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+  MonitorAutoLock mon(mMonitor);
   NS_IF_ADDREF(*aListener = mListener);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::SetListener(nsIThreadPoolListener* aListener)
 {
   nsCOMPtr<nsIThreadPoolListener> swappedListener(aListener);
   {
-    ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+    MonitorAutoLock mon(mMonitor);
     mListener.swap(swappedListener);
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadPool::SetName(const nsACString& aName)
 {
   {
-    ReentrantMonitorAutoEnter mon(mEvents.GetReentrantMonitor());
+    MonitorAutoLock mon(mMonitor);
     if (mThreads.Count()) {
       return NS_ERROR_NOT_AVAILABLE;
     }
   }
 
   mName = aName;
   return NS_OK;
 }
--- a/xpcom/threads/nsThreadPool.h
+++ b/xpcom/threads/nsThreadPool.h
@@ -11,16 +11,17 @@
 #include "nsIThread.h"
 #include "nsIRunnable.h"
 #include "nsEventQueue.h"
 #include "nsCOMArray.h"
 #include "nsCOMPtr.h"
 #include "nsThreadUtils.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/AlreadyAddRefed.h"
+#include "mozilla/Monitor.h"
 
 class nsThreadPool final
   : public nsIThreadPool
   , public nsIRunnable
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIEVENTTARGET
@@ -36,17 +37,18 @@ public:
 private:
   ~nsThreadPool();
 
   void ShutdownThread(nsIThread* aThread);
   nsresult PutEvent(nsIRunnable* aEvent);
   nsresult PutEvent(already_AddRefed<nsIRunnable>&& aEvent);
 
   nsCOMArray<nsIThread> mThreads;
-  nsEventQueue          mEvents;
+  mozilla::Monitor      mMonitor;
+  nsEventQueueBase<mozilla::Monitor> mEvents;
   uint32_t              mThreadLimit;
   uint32_t              mIdleThreadLimit;
   uint32_t              mIdleThreadTimeout;
   uint32_t              mIdleCount;
   uint32_t              mStackSize;
   nsCOMPtr<nsIThreadPoolListener> mListener;
   bool                  mShutdown;
   nsCString             mName;