Bug 1219418 - Always access minEmptyChunkCount under the GC lock; r=jonco
authorTerrence Cole <terrence@mozilla.com>
Wed, 28 Oct 2015 13:42:20 -0700
changeset 270758 07485d615e337c907c5ef57b0dc2e7fa0e0bd251
parent 270757 ba5fd5d4ea22bddaf2d2317c00c008c2850d03c6
child 270759 b526e349c77fd781319ba2fe95f78233b5670767
push id67444
push usertcole@mozilla.com
push dateMon, 02 Nov 2015 15:11:16 +0000
treeherdermozilla-inbound@b526e349c77f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1219418
milestone45.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 1219418 - Always access minEmptyChunkCount under the GC lock; r=jonco
js/src/gc/GCRuntime.h
js/src/gc/Zone.cpp
js/src/gc/Zone.h
js/src/jsapi.cpp
js/src/jsgc.cpp
js/src/vm/Runtime.cpp
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -182,20 +182,20 @@ class GCSchedulingTunables
     bool isDynamicHeapGrowthEnabled() const { return dynamicHeapGrowthEnabled_; }
     uint64_t highFrequencyThresholdUsec() const { return highFrequencyThresholdUsec_; }
     uint64_t highFrequencyLowLimitBytes() const { return highFrequencyLowLimitBytes_; }
     uint64_t highFrequencyHighLimitBytes() const { return highFrequencyHighLimitBytes_; }
     double highFrequencyHeapGrowthMax() const { return highFrequencyHeapGrowthMax_; }
     double highFrequencyHeapGrowthMin() const { return highFrequencyHeapGrowthMin_; }
     double lowFrequencyHeapGrowth() const { return lowFrequencyHeapGrowth_; }
     bool isDynamicMarkSliceEnabled() const { return dynamicMarkSliceEnabled_; }
-    unsigned minEmptyChunkCount() const { return minEmptyChunkCount_; }
+    unsigned minEmptyChunkCount(const AutoLockGC&) const { return minEmptyChunkCount_; }
     unsigned maxEmptyChunkCount() const { return maxEmptyChunkCount_; }
 
-    void setParameter(JSGCParamKey key, uint32_t value);
+    void setParameter(JSGCParamKey key, uint32_t value, const AutoLockGC& lock);
 };
 
 /*
  * GC Scheduling Overview
  * ======================
  *
  * Scheduling GC's in SpiderMonkey/Firefox is tremendously complicated because
  * of the large number of subtle, cross-cutting, and widely dispersed factors
@@ -586,19 +586,19 @@ class GCRuntime
     void finish();
 
     inline int zeal();
     inline bool upcomingZealousGC();
     inline bool needZealousGC();
 
     bool addRoot(Value* vp, const char* name);
     void removeRoot(Value* vp);
-    void setMarkStackLimit(size_t limit);
+    void setMarkStackLimit(size_t limit, AutoLockGC& lock);
 
-    void setParameter(JSGCParamKey key, uint32_t value);
+    void setParameter(JSGCParamKey key, uint32_t value, AutoLockGC& lock);
     uint32_t getParameter(JSGCParamKey key, const AutoLockGC& lock);
 
     bool triggerGC(JS::gcreason::Reason reason);
     void maybeAllocTriggerZoneGC(Zone* zone, const AutoLockGC& lock);
     bool triggerZoneGC(Zone* zone, JS::gcreason::Reason reason);
     bool maybeGC(Zone* zone);
     void maybePeriodicFullGC();
     void minorGC(JS::gcreason::Reason reason) {
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -43,17 +43,18 @@ JS::Zone::Zone(JSRuntime* rt)
     gcPreserveCode_(false),
     jitUsingBarriers_(false),
     listNext_(NotOnList)
 {
     /* Ensure that there are no vtables to mess us up here. */
     MOZ_ASSERT(reinterpret_cast<JS::shadow::Zone*>(this) ==
                static_cast<JS::shadow::Zone*>(this));
 
-    threshold.updateAfterGC(8192, GC_NORMAL, rt->gc.tunables, rt->gc.schedulingState);
+    AutoLockGC lock(rt);
+    threshold.updateAfterGC(8192, GC_NORMAL, rt->gc.tunables, rt->gc.schedulingState, lock);
     setGCMaxMallocBytes(rt->gc.maxMallocBytesAllocated() * 0.9);
 }
 
 Zone::~Zone()
 {
     JSRuntime* rt = runtimeFromMainThread();
     if (this == rt->gc.systemZone)
         rt->gc.systemZone = nullptr;
--- a/js/src/gc/Zone.h
+++ b/js/src/gc/Zone.h
@@ -43,26 +43,28 @@ class ZoneHeapThreshold
         gcTriggerBytes_(0)
     {}
 
     double gcHeapGrowthFactor() const { return gcHeapGrowthFactor_; }
     size_t gcTriggerBytes() const { return gcTriggerBytes_; }
     double allocTrigger(bool highFrequencyGC) const;
 
     void updateAfterGC(size_t lastBytes, JSGCInvocationKind gckind,
-                       const GCSchedulingTunables& tunables, const GCSchedulingState& state);
+                       const GCSchedulingTunables& tunables, const GCSchedulingState& state,
+                       const AutoLockGC& lock);
     void updateForRemovedArena(const GCSchedulingTunables& tunables);
 
   private:
     static double computeZoneHeapGrowthFactorForHeapSize(size_t lastBytes,
                                                          const GCSchedulingTunables& tunables,
                                                          const GCSchedulingState& state);
     static size_t computeZoneTriggerBytes(double growthFactor, size_t lastBytes,
                                           JSGCInvocationKind gckind,
-                                          const GCSchedulingTunables& tunables);
+                                          const GCSchedulingTunables& tunables,
+                                          const AutoLockGC& lock);
 };
 
 // Maps a Cell* to a unique, 64bit id.
 using UniqueIdMap = HashMap<Cell*, uint64_t, PointerHasher<Cell*, 3>, SystemAllocPolicy>;
 
 extern uint64_t NextCellUniqueId(JSRuntime* rt);
 
 } // namespace gc
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -1434,17 +1434,18 @@ JS_UpdateWeakPointerAfterGCUnbarriered(J
 {
     if (IsAboutToBeFinalizedUnbarriered(objp))
         *objp = nullptr;
 }
 
 JS_PUBLIC_API(void)
 JS_SetGCParameter(JSRuntime* rt, JSGCParamKey key, uint32_t value)
 {
-    rt->gc.setParameter(key, value);
+    AutoLockGC lock(rt);
+    rt->gc.setParameter(key, value, lock);
 }
 
 JS_PUBLIC_API(uint32_t)
 JS_GetGCParameter(JSRuntime* rt, JSGCParamKey key)
 {
     AutoLockGC lock(rt);
     return rt->gc.getParameter(key, lock);
 }
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -734,32 +734,32 @@ GCRuntime::expireEmptyChunkPool(bool shr
     for (ChunkPool::Iter iter(emptyChunks(lock)); !iter.done();) {
         Chunk* chunk = iter.get();
         iter.next();
 
         MOZ_ASSERT(chunk->unused());
         MOZ_ASSERT(!fullChunks(lock).contains(chunk));
         MOZ_ASSERT(!availableChunks(lock).contains(chunk));
         if (freeChunkCount >= tunables.maxEmptyChunkCount() ||
-            (freeChunkCount >= tunables.minEmptyChunkCount() &&
+            (freeChunkCount >= tunables.minEmptyChunkCount(lock) &&
              (shrinkBuffers || chunk->info.age == MAX_EMPTY_CHUNK_AGE)))
         {
             emptyChunks(lock).remove(chunk);
             prepareToFreeChunk(chunk->info);
             expired.push(chunk);
         } else {
             /* Keep the chunk but increase its age. */
             ++freeChunkCount;
             ++chunk->info.age;
         }
     }
     MOZ_ASSERT(expired.verify());
     MOZ_ASSERT(emptyChunks(lock).verify());
     MOZ_ASSERT(emptyChunks(lock).count() <= tunables.maxEmptyChunkCount());
-    MOZ_ASSERT_IF(shrinkBuffers, emptyChunks(lock).count() <= tunables.minEmptyChunkCount());
+    MOZ_ASSERT_IF(shrinkBuffers, emptyChunks(lock).count() <= tunables.minEmptyChunkCount(lock));
     return expired;
 }
 
 static void
 FreeChunkPool(JSRuntime* rt, ChunkPool& pool)
 {
     for (ChunkPool::Iter iter(pool); !iter.done();) {
         Chunk* chunk = iter.get();
@@ -1016,17 +1016,17 @@ Chunk::updateChunkListAfterFree(JSRuntim
 
 inline bool
 GCRuntime::wantBackgroundAllocation(const AutoLockGC& lock) const
 {
     // To minimize memory waste, we do not want to run the background chunk
     // allocation if we already have some empty chunks or when the runtime has
     // a small heap size (and therefore likely has a small growth rate).
     return allocTask.enabled() &&
-           emptyChunks(lock).count() < tunables.minEmptyChunkCount() &&
+           emptyChunks(lock).count() < tunables.minEmptyChunkCount(lock) &&
            (fullChunks(lock).count() + availableChunks(lock).count()) >= 4;
 }
 
 void
 GCRuntime::startBackgroundAllocTaskIfIdle()
 {
     AutoLockHelperThreadState helperLock;
     if (allocTask.isRunning())
@@ -1283,19 +1283,24 @@ GCRuntime::init(uint32_t maxbytes, uint3
 
     if (!helperState.init())
         return false;
 
     /*
      * Separate gcMaxMallocBytes from gcMaxBytes but initialize to maxbytes
      * for default backward API compatibility.
      */
-    tunables.setParameter(JSGC_MAX_BYTES, maxbytes);
+    AutoLockGC lock(rt);
+    tunables.setParameter(JSGC_MAX_BYTES, maxbytes, lock);
     setMaxMallocBytes(maxbytes);
 
+    const char* size = getenv("JSGC_MARK_STACK_LIMIT");
+    if (size)
+        setMarkStackLimit(atoi(size), lock);
+
     jitReleaseNumber = majorGCNumber + JIT_SCRIPT_RELEASE_TYPES_PERIOD;
 
     if (!nursery.init(maxNurseryBytes))
         return false;
 
     if (!nursery.isEnabled()) {
         MOZ_ASSERT(nursery.nurserySize() == 0);
         ++rt->gc.generationalDisabled;
@@ -1388,53 +1393,53 @@ GCRuntime::finishRoots()
 {
     if (rootsHash.initialized())
         rootsHash.clear();
 
     FinishPersistentRootedChains(rt->mainThread.roots);
 }
 
 void
-GCRuntime::setParameter(JSGCParamKey key, uint32_t value)
+GCRuntime::setParameter(JSGCParamKey key, uint32_t value, AutoLockGC& lock)
 {
     switch (key) {
       case JSGC_MAX_MALLOC_BYTES:
         setMaxMallocBytes(value);
         for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next())
             zone->setGCMaxMallocBytes(maxMallocBytesAllocated() * 0.9);
         break;
       case JSGC_SLICE_TIME_BUDGET:
         defaultTimeBudget_ = value ? value : SliceBudget::UnlimitedTimeBudget;
         break;
       case JSGC_MARK_STACK_LIMIT:
-        setMarkStackLimit(value);
+        setMarkStackLimit(value, lock);
         break;
       case JSGC_DECOMMIT_THRESHOLD:
         decommitThreshold = value * 1024 * 1024;
         break;
       case JSGC_MODE:
         mode = JSGCMode(value);
         MOZ_ASSERT(mode == JSGC_MODE_GLOBAL ||
                    mode == JSGC_MODE_COMPARTMENT ||
                    mode == JSGC_MODE_INCREMENTAL);
         break;
       case JSGC_COMPACTING_ENABLED:
         compactingEnabled = value != 0;
         break;
       default:
-        tunables.setParameter(key, value);
+        tunables.setParameter(key, value, lock);
         for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) {
             zone->threshold.updateAfterGC(zone->usage.gcBytes(), GC_NORMAL, tunables,
-                                         schedulingState);
+                                          schedulingState, lock);
         }
     }
 }
 
 void
-GCSchedulingTunables::setParameter(JSGCParamKey key, uint32_t value)
+GCSchedulingTunables::setParameter(JSGCParamKey key, uint32_t value, const AutoLockGC& lock)
 {
     switch(key) {
       case JSGC_MAX_BYTES:
         gcMaxBytes_ = value;
         break;
       case JSGC_HIGH_FREQUENCY_TIME_LIMIT:
         highFrequencyThresholdUsec_ = value * PRMJ_USEC_PER_MSEC;
         break;
@@ -1531,31 +1536,32 @@ GCRuntime::getParameter(JSGCParamKey key
         return uint32_t(tunables.lowFrequencyHeapGrowth() * 100);
       case JSGC_DYNAMIC_HEAP_GROWTH:
         return tunables.isDynamicHeapGrowthEnabled();
       case JSGC_DYNAMIC_MARK_SLICE:
         return tunables.isDynamicMarkSliceEnabled();
       case JSGC_ALLOCATION_THRESHOLD:
         return tunables.gcZoneAllocThresholdBase() / 1024 / 1024;
       case JSGC_MIN_EMPTY_CHUNK_COUNT:
-        return tunables.minEmptyChunkCount();
+        return tunables.minEmptyChunkCount(lock);
       case JSGC_MAX_EMPTY_CHUNK_COUNT:
         return tunables.maxEmptyChunkCount();
       case JSGC_COMPACTING_ENABLED:
         return compactingEnabled;
       default:
         MOZ_ASSERT(key == JSGC_NUMBER);
         return uint32_t(number);
     }
 }
 
 void
-GCRuntime::setMarkStackLimit(size_t limit)
+GCRuntime::setMarkStackLimit(size_t limit, AutoLockGC& lock)
 {
     MOZ_ASSERT(!rt->isHeapBusy());
+    AutoUnlockGC unlock(lock);
     AutoStopVerifyingBarriers pauseVerification(rt, false);
     marker.setMaxCapacity(limit);
 }
 
 bool
 GCRuntime::addBlackRootsTracer(JSTraceDataOp traceOp, void* data)
 {
     AssertHeapIsIdle(rt);
@@ -1800,32 +1806,34 @@ ZoneHeapThreshold::computeZoneHeapGrowth
     MOZ_ASSERT(factor >= minRatio);
     MOZ_ASSERT(factor <= maxRatio);
     return factor;
 }
 
 /* static */ size_t
 ZoneHeapThreshold::computeZoneTriggerBytes(double growthFactor, size_t lastBytes,
                                            JSGCInvocationKind gckind,
-                                           const GCSchedulingTunables& tunables)
+                                           const GCSchedulingTunables& tunables,
+                                           const AutoLockGC& lock)
 {
     size_t base = gckind == GC_SHRINK
-                ? Max(lastBytes, tunables.minEmptyChunkCount() * ChunkSize)
+                ? Max(lastBytes, tunables.minEmptyChunkCount(lock) * ChunkSize)
                 : Max(lastBytes, tunables.gcZoneAllocThresholdBase());
     double trigger = double(base) * growthFactor;
     return size_t(Min(double(tunables.gcMaxBytes()), trigger));
 }
 
 void
 ZoneHeapThreshold::updateAfterGC(size_t lastBytes, JSGCInvocationKind gckind,
                                  const GCSchedulingTunables& tunables,
-                                 const GCSchedulingState& state)
+                                 const GCSchedulingState& state, const AutoLockGC& lock)
 {
     gcHeapGrowthFactor_ = computeZoneHeapGrowthFactorForHeapSize(lastBytes, tunables, state);
-    gcTriggerBytes_ = computeZoneTriggerBytes(gcHeapGrowthFactor_, lastBytes, gckind, tunables);
+    gcTriggerBytes_ = computeZoneTriggerBytes(gcHeapGrowthFactor_, lastBytes, gckind, tunables,
+                                              lock);
 }
 
 void
 ZoneHeapThreshold::updateForRemovedArena(const GCSchedulingTunables& tunables)
 {
     size_t amount = ArenaSize * gcHeapGrowthFactor_;
 
     MOZ_ASSERT(amount > 0);
@@ -5203,19 +5211,20 @@ GCRuntime::beginSweepingZoneGroup()
 }
 
 void
 GCRuntime::endSweepingZoneGroup()
 {
     /* Update the GC state for zones we have swept. */
     for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) {
         MOZ_ASSERT(zone->isGCSweeping());
+        AutoLockGC lock(rt);
         zone->setGCState(Zone::Finished);
         zone->threshold.updateAfterGC(zone->usage.gcBytes(), invocationKind, tunables,
-                                      schedulingState);
+                                      schedulingState, lock);
     }
 
     /* Start background thread to sweep zones if required. */
     ZoneList zones;
     for (GCZoneGroupIter zone(rt); !zone.done(); zone.next())
         zones.append(zone);
     if (sweepOnBackgroundThread)
         queueZonesForBackgroundSweep(zones);
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -290,20 +290,16 @@ JSRuntime::init(uint32_t maxbytes, uint3
     if (CanUseExtraThreads() && !EnsureHelperThreadsInitialized())
         return false;
 
     js::TlsPerThreadData.set(&mainThread);
 
     if (!gc.init(maxbytes, maxNurseryBytes))
         return false;
 
-    const char* size = getenv("JSGC_MARK_STACK_LIMIT");
-    if (size)
-        gc.setMarkStackLimit(atoi(size));
-
     ScopedJSDeletePtr<Zone> atomsZone(new_<Zone>(this));
     if (!atomsZone || !atomsZone->init(true))
         return false;
 
     JS::CompartmentOptions options;
     ScopedJSDeletePtr<JSCompartment> atomsCompartment(new_<JSCompartment>(atomsZone.get(), options));
     if (!atomsCompartment || !atomsCompartment->init(nullptr))
         return false;