Bug 1767875: Avoid ever waking up more than one thread at a time. r=smaug
authorBas Schouten <bschouten@mozilla.com>
Tue, 10 May 2022 23:52:31 +0000
changeset 616943 fdfffc18bd4b219cdd2eeda61b7c182679555018
parent 616942 20902b19a0ef6c867eeeabacd0fa7f933ffa572e
child 616944 604f4b32a0977eff93794a72b129f25c479cd2f7
push id39680
push userbszekely@mozilla.com
push dateWed, 11 May 2022 09:42:52 +0000
treeherdermozilla-central@87443e31b7c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1767875
milestone102.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 1767875: Avoid ever waking up more than one thread at a time. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D145962
xpcom/threads/TaskController.cpp
--- a/xpcom/threads/TaskController.cpp
+++ b/xpcom/threads/TaskController.cpp
@@ -243,16 +243,24 @@ void TaskController::RunPoolThread() {
           continue;
         }
 
         mPoolThreads[mThreadPoolIndex].mCurrentTask = task;
         mThreadableTasks.erase(task->mIterator);
         task->mIterator = mThreadableTasks.end();
         task->mInProgress = true;
 
+        if (!mThreadableTasks.empty()) {
+          // Ensure at least one additional thread is woken up if there are
+          // more threadable tasks to process. Notifying all threads at once
+          // isn't actually better for performance since they all need the
+          // GraphMutex to proceed anyway.
+          mThreadPoolCV.Notify();
+        }
+
         bool taskCompleted = false;
         {
           MutexAutoUnlock unlock(mGraphMutex);
           lastTask = nullptr;
           AUTO_PROFILE_FOLLOWING_TASK(task);
           taskCompleted = task->Run();
           ranTask = true;
         }
@@ -807,22 +815,20 @@ bool TaskController::DoExecuteNextTaskOn
         task->mCompleted = true;
 #ifdef DEBUG
         task->mIsInGraph = false;
 #endif
         // Clear dependencies to release references.
         task->mDependencies.clear();
 
         if (!mThreadableTasks.empty()) {
-          // Since this could have multiple dependencies thare are not
-          // restricted to the main thread. Let's wake up our thread pool.
-          // There is a cost to this, it's possible we will want to wake up
-          // only as many threads as we have unblocked tasks, but we currently
-          // have no way to determine that easily.
-          mThreadPoolCV.NotifyAll();
+          // We're going to wake up a single thread in our pool. This thread
+          // is responsible for waking up additional threads in the situation
+          // where more than one task became available.
+          mThreadPoolCV.Notify();
         }
       }
 
       mCurrentTasksMT.pop();
       return true;
     }
   }