Bug 1195767 - part 3 - modify nsThreadPool to use a non-reentrant monitor; r=gerald
authorNathan Froyd <froydnj@mozilla.com>
Thu, 03 Sep 2015 15:38:28 -0400
changeset 293856 0bd307e9015466b55c69d42c8c12aaa6aa590af2
parent 293855 8b01d290c47ce933f1b31457c13db2642af22c9a
child 293857 3fcd5e172ea2dac9e2b7a9f11c3d6775ed5cf87b
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald
bugs1195767
milestone43.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 1195767 - part 3 - modify nsThreadPool to use a non-reentrant monitor; r=gerald 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;