Bug 1283245 - Use js::Mutex and js::ConditionVariable instead of PRLock and PRCondVar in HelperThreads; r=terrence
authorNick Fitzgerald <fitzgen@gmail.com>
Thu, 30 Jun 2016 13:13:30 -0700
changeset 303316 c62dc1a368c81054c835b419f6ec5d88405654e9
parent 303315 e5ad05d9cad4ac105f5f580887202a5041e197b4
child 303317 105425978bcce64b8bfe2074b3000353a7c33003
push id19839
push usercbook@mozilla.com
push dateFri, 01 Jul 2016 09:19:59 +0000
treeherderfx-team@499d8875de7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1283245
milestone50.0a1
Bug 1283245 - Use js::Mutex and js::ConditionVariable instead of PRLock and PRCondVar in HelperThreads; r=terrence
js/src/vm/HelperThreads.cpp
js/src/vm/HelperThreads.h
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -627,42 +627,28 @@ GlobalHelperThreadState::ensureInitializ
     return true;
 }
 
 GlobalHelperThreadState::GlobalHelperThreadState()
  : cpuCount(0),
    threadCount(0),
    threads(nullptr),
    wasmCompilationInProgress(false),
-   numWasmFailedJobs(0),
-   helperLock(nullptr),
-   consumerWakeup(nullptr),
-   producerWakeup(nullptr),
-   pauseWakeup(nullptr)
+   numWasmFailedJobs(0)
 {
     cpuCount = GetCPUCount();
     threadCount = ThreadCountForCPUCount(cpuCount);
 
     MOZ_ASSERT(cpuCount > 0, "GetCPUCount() seems broken");
-
-    helperLock = PR_NewLock();
-    consumerWakeup = PR_NewCondVar(helperLock);
-    producerWakeup = PR_NewCondVar(helperLock);
-    pauseWakeup = PR_NewCondVar(helperLock);
 }
 
 void
 GlobalHelperThreadState::finish()
 {
     finishThreads();
-
-    PR_DestroyCondVar(consumerWakeup);
-    PR_DestroyCondVar(producerWakeup);
-    PR_DestroyCondVar(pauseWakeup);
-    PR_DestroyLock(helperLock);
 }
 
 void
 GlobalHelperThreadState::finishThreads()
 {
     if (!threads)
         return;
 
@@ -673,30 +659,30 @@ GlobalHelperThreadState::finishThreads()
     threads = nullptr;
 }
 
 void
 GlobalHelperThreadState::lock()
 {
     MOZ_ASSERT(!isLocked());
     AssertCurrentThreadCanLock(HelperThreadStateLock);
-    PR_Lock(helperLock);
+    helperLock.lock();
 #ifdef DEBUG
     lockOwner = PR_GetCurrentThread();
 #endif
 }
 
 void
 GlobalHelperThreadState::unlock()
 {
     MOZ_ASSERT(isLocked());
 #ifdef DEBUG
     lockOwner = nullptr;
 #endif
-    PR_Unlock(helperLock);
+    helperLock.unlock();
 }
 
 #ifdef DEBUG
 bool
 GlobalHelperThreadState::isLocked()
 {
     return lockOwner == PR_GetCurrentThread();
 }
@@ -705,35 +691,34 @@ GlobalHelperThreadState::isLocked()
 void
 GlobalHelperThreadState::wait(AutoLockHelperThreadState& locked, CondVar which,
                               TimeDuration timeout /* = TimeDuration::Forever() */)
 {
     MOZ_ASSERT(isLocked());
 #ifdef DEBUG
     lockOwner = nullptr;
 #endif
-    DebugOnly<PRStatus> status = PR_WaitCondVar(whichWakeup(which), DurationToPRInterval(timeout));
-    MOZ_ASSERT(status == PR_SUCCESS);
+    whichWakeup(which).wait_for(locked, timeout);
 #ifdef DEBUG
     lockOwner = PR_GetCurrentThread();
 #endif
 }
 
 void
 GlobalHelperThreadState::notifyAll(CondVar which)
 {
     MOZ_ASSERT(isLocked());
-    PR_NotifyAllCondVar(whichWakeup(which));
+    whichWakeup(which).notify_all();
 }
 
 void
 GlobalHelperThreadState::notifyOne(CondVar which)
 {
     MOZ_ASSERT(isLocked());
-    PR_NotifyCondVar(whichWakeup(which));
+    whichWakeup(which).notify_one();
 }
 
 bool
 GlobalHelperThreadState::hasActiveThreads()
 {
     MOZ_ASSERT(isLocked());
     if (!threads)
         return false;
--- a/js/src/vm/HelperThreads.h
+++ b/js/src/vm/HelperThreads.h
@@ -24,16 +24,17 @@
 #include "frontend/TokenStream.h"
 #include "jit/Ion.h"
 #include "threading/ConditionVariable.h"
 #include "threading/Mutex.h"
 
 namespace js {
 
 class AutoLockHelperThreadState;
+class AutoUnlockHelperThreadState;
 struct HelperThread;
 struct ParseTask;
 namespace jit {
   class IonBuilder;
 } // namespace jit
 namespace wasm {
   class FuncIR;
   class FunctionCompileResults;
@@ -45,16 +46,19 @@ enum class ParseTaskKind
 {
     Script,
     Module
 };
 
 // Per-process state for off thread work items.
 class GlobalHelperThreadState
 {
+    friend class AutoLockHelperThreadState;
+    friend class AutoUnlockHelperThreadState;
+
   public:
     // Number of CPUs to treat this machine as having when creating threads.
     // May be accessed without locking.
     size_t cpuCount;
 
     // Number of threads to create. May be accessed without locking.
     size_t threadCount;
 
@@ -246,27 +250,27 @@ class GlobalHelperThreadState
     bool checkTaskThreadLimit(size_t maxThreads) const;
 
   private:
 
     /*
      * Lock protecting all mutable shared state accessed by helper threads, and
      * used by all condition variables.
      */
-    PRLock* helperLock;
+    js::Mutex helperLock;
 #ifdef DEBUG
     mozilla::Atomic<PRThread*> lockOwner;
 #endif
 
     /* Condvars for threads waiting/notifying each other. */
-    PRCondVar* consumerWakeup;
-    PRCondVar* producerWakeup;
-    PRCondVar* pauseWakeup;
+    js::ConditionVariable consumerWakeup;
+    js::ConditionVariable producerWakeup;
+    js::ConditionVariable pauseWakeup;
 
-    PRCondVar* whichWakeup(CondVar which) {
+    js::ConditionVariable& whichWakeup(CondVar which) {
         switch (which) {
           case CONSUMER: return consumerWakeup;
           case PRODUCER: return producerWakeup;
           case PAUSE: return pauseWakeup;
           default: MOZ_CRASH("Invalid CondVar in |whichWakeup|");
         }
     }
 };
@@ -435,48 +439,68 @@ struct AutoEnqueuePendingParseTasksAfter
     explicit AutoEnqueuePendingParseTasksAfterGC(const gc::GCRuntime& gc) : gc_(gc) {}
     ~AutoEnqueuePendingParseTasksAfterGC();
 };
 
 /* Start a compression job for the specified token. */
 bool
 StartOffThreadCompression(ExclusiveContext* cx, SourceCompressionTask* task);
 
-class MOZ_RAII AutoLockHelperThreadState
+class MOZ_RAII AutoLockHelperThreadState : public LockGuard<Mutex>
 {
+    using Base = LockGuard<Mutex>;
+
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 
   public:
     explicit AutoLockHelperThreadState(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM)
+      : Base(HelperThreadState().helperLock)
     {
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
-        HelperThreadState().lock();
+
+#ifdef DEBUG
+        HelperThreadState().lockOwner = PR_GetCurrentThread();
+#endif
     }
 
     ~AutoLockHelperThreadState() {
-        HelperThreadState().unlock();
+#ifdef DEBUG
+        HelperThreadState().lockOwner = nullptr;
+#endif
     }
 };
 
 class MOZ_RAII AutoUnlockHelperThreadState
 {
+    // This is only in a Maybe so that we can update the DEBUG-only lockOwner
+    // thread in the proper order.
+    mozilla::Maybe<UnlockGuard<Mutex>> unlocked;
+
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 
   public:
 
     explicit AutoUnlockHelperThreadState(AutoLockHelperThreadState& locked
                                          MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
     {
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
-        HelperThreadState().unlock();
+
+#ifdef DEBUG
+        HelperThreadState().lockOwner = nullptr;
+#endif
+
+        unlocked.emplace(locked);
     }
 
-    ~AutoUnlockHelperThreadState()
-    {
-        HelperThreadState().lock();
+    ~AutoUnlockHelperThreadState() {
+        unlocked.reset();
+
+#ifdef DEBUG
+        HelperThreadState().lockOwner = PR_GetCurrentThread();
+#endif
     }
 };
 
 struct ParseTask
 {
     ParseTaskKind kind;
     ExclusiveContext* cx;
     OwningCompileOptions options;