Bug 1207232: Let lockOwner be Atomic; r=terrence
authorBenjamin Bouvier <benj@benj.me>
Tue, 22 Sep 2015 19:26:21 +0200
changeset 264029 36d363bb1d734d60280a4ea96cbfa75db98a4c4f
parent 264028 81fca7e4e6ef66dd79e48ef54f31b09d941090a3
child 264030 748829f0a151de937c35d6f43e27a14a39bdb324
push id65505
push userbenj@benj.me
push dateWed, 23 Sep 2015 18:09:04 +0000
treeherdermozilla-inbound@36d363bb1d73 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1207232
milestone44.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 1207232: Let lockOwner be Atomic; r=terrence
js/src/gc/GCRuntime.h
js/src/jsgc.cpp
js/src/vm/HelperThreads.cpp
js/src/vm/HelperThreads.h
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -668,36 +668,38 @@ class GCRuntime
 
     void requestMinorGC(JS::gcreason::Reason reason);
 
 #ifdef DEBUG
 
     bool onBackgroundThread() { return helperState.onBackgroundThread(); }
 
     bool currentThreadOwnsGCLock() {
-        return lockOwner == PR_GetCurrentThread();
+        return lockOwner.value == PR_GetCurrentThread();
     }
 
 #endif // DEBUG
 
     void assertCanLock() {
         MOZ_ASSERT(!currentThreadOwnsGCLock());
     }
 
     void lockGC() {
         PR_Lock(lock);
-        MOZ_ASSERT(!lockOwner);
 #ifdef DEBUG
-        lockOwner = PR_GetCurrentThread();
+        MOZ_ASSERT(!lockOwner.value);
+        lockOwner.value = PR_GetCurrentThread();
 #endif
     }
 
     void unlockGC() {
-        MOZ_ASSERT(lockOwner == PR_GetCurrentThread());
-        lockOwner = nullptr;
+#ifdef DEBUG
+        MOZ_ASSERT(lockOwner.value == PR_GetCurrentThread());
+        lockOwner.value = nullptr;
+#endif
         PR_Unlock(lock);
     }
 
 #ifdef DEBUG
     bool isAllocAllowed() { return noGCOrAllocationCheck == 0; }
     void disallowAlloc() { ++noGCOrAllocationCheck; }
     void allowAlloc() {
         MOZ_ASSERT(!isAllocAllowed());
@@ -1293,17 +1295,17 @@ class GCRuntime
      */
     int inUnsafeRegion;
 
     size_t noGCOrAllocationCheck;
 #endif
 
     /* Synchronize GC heap access between main thread and GCHelperState. */
     PRLock* lock;
-    mozilla::DebugOnly<PRThread*> lockOwner;
+    mozilla::DebugOnly<mozilla::Atomic<PRThread*>> lockOwner;
 
     BackgroundAllocTask allocTask;
     GCHelperState helperState;
 
     /*
      * During incremental sweeping, this field temporarily holds the arenas of
      * the current AllocKind being swept in order of increasing free space.
      */
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -1171,17 +1171,16 @@ GCRuntime::GCRuntime(JSRuntime* rt) :
     mallocBytesUntilGC(0),
     mallocGCTriggered(false),
     alwaysPreserveCode(false),
 #ifdef DEBUG
     inUnsafeRegion(0),
     noGCOrAllocationCheck(0),
 #endif
     lock(nullptr),
-    lockOwner(nullptr),
     allocTask(rt, emptyChunks_),
     helperState(rt)
 {
     setGCMode(JSGC_MODE_GLOBAL);
 }
 
 #ifdef JS_GC_ZEAL
 
@@ -3404,21 +3403,21 @@ GCHelperState::startBackgroundThread(Sta
 }
 
 void
 GCHelperState::waitForBackgroundThread()
 {
     MOZ_ASSERT(CurrentThreadCanAccessRuntime(rt));
 
 #ifdef DEBUG
-    rt->gc.lockOwner = nullptr;
+    rt->gc.lockOwner.value = nullptr;
 #endif
     PR_WaitCondVar(done, PR_INTERVAL_NO_TIMEOUT);
 #ifdef DEBUG
-    rt->gc.lockOwner = PR_GetCurrentThread();
+    rt->gc.lockOwner.value = PR_GetCurrentThread();
 #endif
 }
 
 void
 GCHelperState::work()
 {
     MOZ_ASSERT(CanUseExtraThreads());
 
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -487,19 +487,16 @@ GlobalHelperThreadState::ensureInitializ
 }
 
 GlobalHelperThreadState::GlobalHelperThreadState()
  : cpuCount(0),
    threadCount(0),
    threads(nullptr),
    asmJSCompilationInProgress(false),
    helperLock(nullptr),
-#ifdef DEBUG
-   lockOwner(nullptr),
-#endif
    consumerWakeup(nullptr),
    producerWakeup(nullptr),
    pauseWakeup(nullptr),
    numAsmJSFailedJobs(0),
    asmJSFailedFunction(nullptr)
 {
     cpuCount = GetCPUCount();
     threadCount = ThreadCountForCPUCount(cpuCount);
@@ -540,51 +537,51 @@ GlobalHelperThreadState::finishThreads()
 
 void
 GlobalHelperThreadState::lock()
 {
     MOZ_ASSERT(!isLocked());
     AssertCurrentThreadCanLock(HelperThreadStateLock);
     PR_Lock(helperLock);
 #ifdef DEBUG
-    lockOwner = PR_GetCurrentThread();
+    lockOwner.value = PR_GetCurrentThread();
 #endif
 }
 
 void
 GlobalHelperThreadState::unlock()
 {
     MOZ_ASSERT(isLocked());
 #ifdef DEBUG
-    lockOwner = nullptr;
+    lockOwner.value = nullptr;
 #endif
     PR_Unlock(helperLock);
 }
 
 #ifdef DEBUG
 bool
 GlobalHelperThreadState::isLocked()
 {
-    return lockOwner == PR_GetCurrentThread();
+    return lockOwner.value == PR_GetCurrentThread();
 }
 #endif
 
 void
 GlobalHelperThreadState::wait(CondVar which, uint32_t millis)
 {
     MOZ_ASSERT(isLocked());
 #ifdef DEBUG
-    lockOwner = nullptr;
+    lockOwner.value = nullptr;
 #endif
     DebugOnly<PRStatus> status =
         PR_WaitCondVar(whichWakeup(which),
                        millis ? PR_MillisecondsToInterval(millis) : PR_INTERVAL_NO_TIMEOUT);
     MOZ_ASSERT(status == PR_SUCCESS);
 #ifdef DEBUG
-    lockOwner = PR_GetCurrentThread();
+    lockOwner.value = PR_GetCurrentThread();
 #endif
 }
 
 void
 GlobalHelperThreadState::notifyAll(CondVar which)
 {
     MOZ_ASSERT(isLocked());
     PR_NotifyAllCondVar(whichWakeup(which));
--- a/js/src/vm/HelperThreads.h
+++ b/js/src/vm/HelperThreads.h
@@ -237,19 +237,17 @@ class GlobalHelperThreadState
 
   private:
 
     /*
      * Lock protecting all mutable shared state accessed by helper threads, and
      * used by all condition variables.
      */
     PRLock* helperLock;
-#ifdef DEBUG
-    PRThread* lockOwner;
-#endif
+    mozilla::DebugOnly<mozilla::Atomic<PRThread*>> lockOwner;
 
     /* Condvars for threads waiting/notifying each other. */
     PRCondVar* consumerWakeup;
     PRCondVar* producerWakeup;
     PRCondVar* pauseWakeup;
 
     PRCondVar* whichWakeup(CondVar which) {
         switch (which) {