Bug 1119550 - Stop using volatile to "synchronize" majorGCRequested; r=jonco
authorTerrence Cole <terrence@mozilla.com>
Sat, 17 Jan 2015 12:15:25 -0800
changeset 224738 6ee3e8054b729bab7a14fe3bff1c5259b35cf3dd
parent 224737 dc4af2782bb88c1d81459dd2769f484080ec0b1c
child 224739 8e736f848d7c2e7aea4e35fb7ad538a4ceff700d
push id28143
push userryanvm@gmail.com
push dateWed, 21 Jan 2015 03:14:12 +0000
treeherdermozilla-central@540077a30866 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1119550
milestone38.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 1119550 - Stop using volatile to "synchronize" majorGCRequested; r=jonco
js/src/gc/GCRuntime.h
js/src/jsgc.cpp
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -2,16 +2,18 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef gc_GCRuntime_h
 #define gc_GCRuntime_h
 
+#include "mozilla/Atomics.h"
+
 #include "jsgc.h"
 
 #include "gc/Heap.h"
 #include "gc/Nursery.h"
 #include "gc/Statistics.h"
 #include "gc/StoreBuffer.h"
 #include "gc/Tracer.h"
 
@@ -478,17 +480,19 @@ class GCRuntime
     bool isIncrementalGc() { return isIncremental; }
     bool isFullGc() { return isFull; }
 
     bool shouldCleanUpEverything() { return cleanUpEverything; }
 
     bool areGrayBitsValid() { return grayBitsValid; }
     void setGrayBitsInvalid() { grayBitsValid = false; }
 
-    bool isGcNeeded() { return minorGCRequested || majorGCRequested; }
+    bool minorGCRequested() const { return minorGCTriggerReason != JS::gcreason::NO_REASON; }
+    bool majorGCRequested() const { return majorGCTriggerReason != JS::gcreason::NO_REASON; }
+    bool isGcNeeded() { return minorGCRequested() || majorGCRequested(); }
 
     double computeHeapGrowthFactor(size_t lastBytes);
     size_t computeTriggerBytes(double growthFactor, size_t lastBytes);
 
     JSGCMode gcMode() const { return mode; }
     void setGCMode(JSGCMode m) {
         mode = m;
         marker.setGCMode(mode);
@@ -694,20 +698,18 @@ class GCRuntime
     bool cleanUpEverything;
 
     /*
      * The gray bits can become invalid if UnmarkGray overflows the stack. A
      * full GC will reset this bit, since it fills in all the gray bits.
      */
     bool grayBitsValid;
 
-    volatile uintptr_t majorGCRequested;
-    JS::gcreason::Reason majorGCTriggerReason;
+    mozilla::Atomic<JS::gcreason::Reason, mozilla::Relaxed> majorGCTriggerReason;
 
-    bool minorGCRequested;
     JS::gcreason::Reason minorGCTriggerReason;
 
     /* Incremented at the start of every major GC. */
     uint64_t majorGCNumber;
 
     /* The major GC number at which to release observed type information. */
     uint64_t jitReleaseNumber;
 
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -1093,19 +1093,17 @@ GCRuntime::GCRuntime(JSRuntime *rt) :
     verifyPostData(nullptr),
     chunkAllocationSinceLastGC(false),
     nextFullGCTime(0),
     lastGCTime(PRMJ_Now()),
     mode(JSGC_MODE_INCREMENTAL),
     decommitThreshold(32 * 1024 * 1024),
     cleanUpEverything(false),
     grayBitsValid(false),
-    majorGCRequested(0),
     majorGCTriggerReason(JS::gcreason::NO_REASON),
-    minorGCRequested(false),
     minorGCTriggerReason(JS::gcreason::NO_REASON),
     majorGCNumber(0),
     jitReleaseNumber(0),
     number(0),
     startNumber(0),
     isFull(false),
 #ifdef DEBUG
     disableStrictProxyCheckingCount(0),
@@ -3139,32 +3137,30 @@ void
 js::MarkCompartmentActive(InterpreterFrame *fp)
 {
     fp->script()->compartment()->zone()->active = true;
 }
 
 void
 GCRuntime::requestMajorGC(JS::gcreason::Reason reason)
 {
-    if (majorGCRequested)
+    if (majorGCRequested())
         return;
 
-    majorGCRequested = true;
     majorGCTriggerReason = reason;
     rt->requestInterrupt(JSRuntime::RequestInterruptUrgent);
 }
 
 void
 GCRuntime::requestMinorGC(JS::gcreason::Reason reason)
 {
     MOZ_ASSERT(CurrentThreadCanAccessRuntime(rt));
-    if (minorGCRequested)
+    if (minorGCRequested())
         return;
 
-    minorGCRequested = true;
     minorGCTriggerReason = reason;
     rt->requestInterrupt(JSRuntime::RequestInterruptUrgent);
 }
 
 bool
 GCRuntime::triggerGC(JS::gcreason::Reason reason)
 {
     /*
@@ -6054,17 +6050,17 @@ GCRuntime::gcCycle(bool incremental, Sli
     /*
      * Marking can trigger many incidental post barriers, some of them for
      * objects which are not going to be live after the GC.
      */
     AutoDisableStoreBuffer adsb(this);
 
     AutoTraceSession session(rt, MajorCollecting);
 
-    majorGCRequested = false;
+    majorGCTriggerReason = JS::gcreason::NO_REASON;
     interFrameGC = true;
 
     number++;
     if (!isIncrementalGCInProgress())
         majorGCNumber++;
 
     // It's ok if threads other than the main thread have suppressGC set, as
     // they are operating on zones which will not be collected from here.
@@ -6436,17 +6432,17 @@ GCRuntime::onOutOfMallocMemory(const Aut
     // might let the OS scrape together enough pages to satisfy the failing
     // malloc request.
     decommitAllWithoutUnlocking(lock);
 }
 
 void
 GCRuntime::minorGCImpl(JS::gcreason::Reason reason, Nursery::TypeObjectList *pretenureTypes)
 {
-    minorGCRequested = false;
+    minorGCTriggerReason = JS::gcreason::NO_REASON;
     TraceLoggerThread *logger = TraceLoggerForMainThread(rt);
     AutoTraceLog logMinorGC(logger, TraceLogger_MinorGC);
     nursery.collect(rt, reason, pretenureTypes);
     MOZ_ASSERT_IF(!rt->mainThread.suppressGC, nursery.isEmpty());
 }
 
 // Alternate to the runtime-taking form that allows marking type objects as
 // needing pretenuring.
@@ -6485,24 +6481,24 @@ GCRuntime::enableGenerationalGC()
     }
 }
 
 bool
 GCRuntime::gcIfNeeded(JSContext *cx /* = nullptr */)
 {
     // This method returns whether a major GC was performed.
 
-    if (minorGCRequested) {
+    if (minorGCRequested()) {
         if (cx)
             minorGC(cx, minorGCTriggerReason);
         else
             minorGC(minorGCTriggerReason);
     }
 
-    if (majorGCRequested) {
+    if (majorGCRequested()) {
         if (!isIncrementalGCInProgress())
             startGC(GC_NORMAL, majorGCTriggerReason);
         else
             gcSlice(majorGCTriggerReason);
         return true;
     }
 
     return false;