Bug 1280064 - Do not use PRLock in GCRuntime and PRCondVar in GCHelperState; r=terrence
authorNick Fitzgerald <fitzgen@gmail.com>
Tue, 28 Jun 2016 17:12:54 -0700
changeset 345119 c36f5c0bed2c4068260c87107082b75eb3ab9b6e
parent 345118 310e0872791410454b67ec5edebb49e654055cbf
child 345120 0bd3ecc9e6cfc6924594c6388fa3f50dca34eeff
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1280064
milestone50.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 1280064 - Do not use PRLock in GCRuntime and PRCondVar in GCHelperState; r=terrence
js/src/gc/GCRuntime.h
js/src/jsgc.cpp
js/src/jsgc.h
js/src/vm/Runtime.h
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -692,29 +692,29 @@ class GCRuntime
 
 #endif // DEBUG
 
     void assertCanLock() {
         MOZ_ASSERT(!currentThreadOwnsGCLock());
     }
 
     void lockGC() {
-        PR_Lock(lock);
+        lock.lock();
 #ifdef DEBUG
         MOZ_ASSERT(!lockOwner);
         lockOwner = PR_GetCurrentThread();
 #endif
     }
 
     void unlockGC() {
 #ifdef DEBUG
         MOZ_ASSERT(lockOwner == PR_GetCurrentThread());
         lockOwner = nullptr;
 #endif
-        PR_Unlock(lock);
+        lock.unlock();
     }
 
 #ifdef DEBUG
     bool isAllocAllowed() { return noGCOrAllocationCheck == 0; }
     void disallowAlloc() { ++noGCOrAllocationCheck; }
     void allowAlloc() {
         MOZ_ASSERT(!isAllocAllowed());
         --noGCOrAllocationCheck;
@@ -1345,17 +1345,18 @@ class GCRuntime
      */
     int inUnsafeRegion;
 
     size_t noGCOrAllocationCheck;
     size_t noNurseryAllocationCheck;
 #endif
 
     /* Synchronize GC heap access between main thread and GCHelperState. */
-    PRLock* lock;
+    friend class js::AutoLockGC;
+    js::Mutex lock;
 #ifdef DEBUG
     mozilla::Atomic<PRThread*> lockOwner;
 #endif
 
     BackgroundAllocTask allocTask;
     GCHelperState helperState;
 
     /*
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -880,17 +880,17 @@ GCRuntime::GCRuntime(JSRuntime* rt) :
     mallocBytesUntilGC(0),
     mallocGCTriggered(false),
     alwaysPreserveCode(false),
 #ifdef DEBUG
     inUnsafeRegion(0),
     noGCOrAllocationCheck(0),
     noNurseryAllocationCheck(0),
 #endif
-    lock(nullptr),
+    lock(),
     allocTask(rt, emptyChunks_),
     helperState(rt)
 {
     setGCMode(JSGC_MODE_GLOBAL);
 }
 
 #ifdef JS_GC_ZEAL
 
@@ -1038,26 +1038,19 @@ GCRuntime::parseAndSetZeal(const char* s
  */
 static const uint64_t JIT_SCRIPT_RELEASE_TYPES_PERIOD = 20;
 
 bool
 GCRuntime::init(uint32_t maxbytes, uint32_t maxNurseryBytes)
 {
     InitMemorySubsystem();
 
-    lock = PR_NewLock();
-    if (!lock)
-        return false;
-
     if (!rootsHash.init(256))
         return false;
 
-    if (!helperState.init())
-        return false;
-
     /*
      * Separate gcMaxMallocBytes from gcMaxBytes but initialize to maxbytes
      * for default backward API compatibility.
      */
     AutoLockGC lock(rt);
     MOZ_ALWAYS_TRUE(tunables.setParameter(JSGC_MAX_BYTES, maxbytes, lock));
     setMaxMallocBytes(maxbytes);
 
@@ -1125,21 +1118,16 @@ GCRuntime::finish()
     }
 
     zones.clear();
 
     FreeChunkPool(rt, fullChunks_);
     FreeChunkPool(rt, availableChunks_);
     FreeChunkPool(rt, emptyChunks_);
 
-    if (lock) {
-        PR_DestroyLock(lock);
-        lock = nullptr;
-    }
-
     FinishTrace();
 }
 
 void
 GCRuntime::finishRoots()
 {
     if (rootsHash.initialized())
         rootsHash.clear();
@@ -3260,38 +3248,21 @@ js::GetCPUCount()
 # else
         long n = sysconf(_SC_NPROCESSORS_ONLN);
         ncpus = (n > 0) ? unsigned(n) : 1;
 # endif
     }
     return ncpus;
 }
 
-bool
-GCHelperState::init()
-{
-    if (!(done = PR_NewCondVar(rt->gc.lock)))
-        return false;
-
-    return true;
-}
-
 void
 GCHelperState::finish()
 {
-    if (!rt->gc.lock) {
-        MOZ_ASSERT(state_ == IDLE);
-        return;
-    }
-
     // Wait for any lingering background sweeping to finish.
     waitBackgroundSweepEnd();
-
-    if (done)
-        PR_DestroyCondVar(done);
 }
 
 GCHelperState::State
 GCHelperState::state()
 {
     MOZ_ASSERT(rt->gc.currentThreadOwnsGCLock());
     return state_;
 }
@@ -3314,24 +3285,24 @@ GCHelperState::startBackgroundThread(Sta
         if (!HelperThreadState().gcHelperWorklist().append(this))
             noOOM.crash("Could not add to pending GC helpers list");
     }
 
     HelperThreadState().notifyAll(GlobalHelperThreadState::PRODUCER);
 }
 
 void
-GCHelperState::waitForBackgroundThread()
+GCHelperState::waitForBackgroundThread(js::AutoLockGC& lock)
 {
     MOZ_ASSERT(CurrentThreadCanAccessRuntime(rt));
 
 #ifdef DEBUG
     rt->gc.lockOwner = nullptr;
 #endif
-    PR_WaitCondVar(done, PR_INTERVAL_NO_TIMEOUT);
+    done.wait(lock.guard());
 #ifdef DEBUG
     rt->gc.lockOwner = PR_GetCurrentThread();
 #endif
 }
 
 void
 GCHelperState::work()
 {
@@ -3357,17 +3328,17 @@ GCHelperState::work()
         break;
       }
 
     }
 
     setState(IDLE);
     thread = nullptr;
 
-    PR_NotifyAllCondVar(done);
+    done.notify_all();
 }
 
 void
 GCRuntime::queueZonesForBackgroundSweep(ZoneList& zones)
 {
     AutoLockHelperThreadState helperLock;
     AutoLockGC lock(rt);
     backgroundSweepZones.transferFrom(zones);
@@ -3423,17 +3394,17 @@ GCHelperState::startBackgroundShrink(con
     }
 }
 
 void
 GCHelperState::waitBackgroundSweepEnd()
 {
     AutoLockGC lock(rt);
     while (state() == SWEEPING)
-        waitForBackgroundThread();
+        waitForBackgroundThread(lock);
     if (!rt->gc.isIncrementalGCInProgress())
         rt->gc.assertBackgroundSweepingFinished();
 }
 
 void
 GCHelperState::doSweep(AutoLockGC& lock)
 {
     // The main thread may call queueZonesForBackgroundSweep() or
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -15,16 +15,17 @@
 #include "mozilla/TypeTraits.h"
 
 #include "jslock.h"
 
 #include "js/GCAPI.h"
 #include "js/SliceBudget.h"
 #include "js/Vector.h"
 
+#include "threading/ConditionVariable.h"
 #include "vm/NativeObject.h"
 
 namespace js {
 
 unsigned GetCPUCount();
 
 enum ThreadType
 {
@@ -844,26 +845,26 @@ class GCHelperState
     };
 
     // Associated runtime.
     JSRuntime* const rt;
 
     // Condvar for notifying the main thread when work has finished. This is
     // associated with the runtime's GC lock --- the worker thread state
     // condvars can't be used here due to lock ordering issues.
-    PRCondVar* done;
+    js::ConditionVariable done;
 
     // Activity for the helper to do, protected by the GC lock.
     State state_;
 
     // Thread which work is being performed on, or null.
     PRThread* thread;
 
     void startBackgroundThread(State newState);
-    void waitForBackgroundThread();
+    void waitForBackgroundThread(js::AutoLockGC& lock);
 
     State state();
     void setState(State state);
 
     bool shrinkFlag;
 
     friend class js::gc::ArenaLists;
 
@@ -874,23 +875,22 @@ class GCHelperState
         js_free(array);
     }
 
     void doSweep(AutoLockGC& lock);
 
   public:
     explicit GCHelperState(JSRuntime* rt)
       : rt(rt),
-        done(nullptr),
+        done(),
         state_(IDLE),
         thread(nullptr),
         shrinkFlag(false)
     { }
 
-    bool init();
     void finish();
 
     void work();
 
     void maybeStartBackgroundSweep(const AutoLockGC& lock);
     void startBackgroundShrink(const AutoLockGC& lock);
 
     /* Must be called without the GC lock taken. */
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -1749,34 +1749,47 @@ class MOZ_RAII AutoLockGC
         lock();
     }
 
     ~AutoLockGC() {
         unlock();
     }
 
     void lock() {
-        runtime_->lockGC();
+        MOZ_ASSERT(lockGuard_.isNothing());
+        lockGuard_.emplace(runtime_->gc.lock);
+#ifdef DEBUG
+        runtime_->gc.lockOwner = PR_GetCurrentThread();
+#endif
     }
 
     void unlock() {
-        runtime_->unlockGC();
+        MOZ_ASSERT(lockGuard_.isSome());
+#ifdef DEBUG
+        runtime_->gc.lockOwner = nullptr;
+#endif
+        lockGuard_.reset();
 #ifdef DEBUG
         wasUnlocked_ = true;
 #endif
     }
 
+    js::LockGuard<js::Mutex>& guard() {
+        return lockGuard_.ref();
+    }
+
 #ifdef DEBUG
     bool wasUnlocked() {
         return wasUnlocked_;
     }
 #endif
 
   private:
     JSRuntime* runtime_;
+    mozilla::Maybe<js::LockGuard<js::Mutex>> lockGuard_;
 #ifdef DEBUG
     bool wasUnlocked_;
 #endif
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 
     AutoLockGC(const AutoLockGC&) = delete;
     AutoLockGC& operator=(const AutoLockGC&) = delete;
 };