Bug 1382720 - Move zone state assertion into state update method r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 21 Jul 2017 10:00:59 +0100
changeset 418906 d9d10df4a1354f5a9ce2f69a52c2064423489dd2
parent 418905 d1340f39e016f763a780f70703cb4561a20a8162
child 418907 07888792204bdac76dea9f16afd04613edc6633d
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1382720
milestone56.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 1382720 - Move zone state assertion into state update method r=sfink
js/src/gc/Zone.h
js/src/jsgc.cpp
--- a/js/src/gc/Zone.h
+++ b/js/src/gc/Zone.h
@@ -213,21 +213,22 @@ struct Zone : public JS::shadow::Zone,
 
     void setPreservingCode(bool preserving) { gcPreserveCode_ = preserving; }
     bool isPreservingCode() const { return gcPreserveCode_; }
 
     bool canCollect();
 
     void notifyObservingDebuggers();
 
-    void setGCState(GCState state) {
+    void changeGCState(GCState prev, GCState next) {
         MOZ_ASSERT(CurrentThreadIsHeapBusy());
-        MOZ_ASSERT_IF(state != NoGC, canCollect());
-        gcState_ = state;
-        if (state == Finished)
+        MOZ_ASSERT(gcState() == prev);
+        MOZ_ASSERT_IF(next != NoGC, canCollect());
+        gcState_ = next;
+        if (isGCFinished())
             notifyObservingDebuggers();
     }
 
     bool isCollecting() const {
         MOZ_ASSERT(CurrentThreadCanAccessRuntime(runtimeFromActiveCooperatingThread()));
         return isCollectingFromAnyThread();
     }
 
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -3871,17 +3871,17 @@ GCRuntime::prepareZonesForCollection(JS:
 
     int64_t currentTime = PRMJ_Now();
 
     for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) {
         /* Set up which zones will be collected. */
         if (ShouldCollectZone(zone, reason)) {
             if (!zone->isAtomsZone()) {
                 any = true;
-                zone->setGCState(Zone::Mark);
+                zone->changeGCState(Zone::NoGC, Zone::Mark);
             }
         } else {
             *isFullOut = false;
         }
 
         zone->setPreservingCode(false);
     }
 
@@ -3919,17 +3919,17 @@ GCRuntime::prepareZonesForCollection(JS:
      * Otherwise, we always schedule a GC in the atoms zone so that atoms which
      * the other collected zones are using are marked, and we can update the
      * set of atoms in use by the other collected zones at the end of the GC.
      */
     if (!TlsContext.get()->keepAtoms || rt->hasHelperThreadZones()) {
         Zone* atomsZone = rt->atomsCompartment(lock)->zone();
         if (atomsZone->isGCScheduled()) {
             MOZ_ASSERT(!atomsZone->isCollecting());
-            atomsZone->setGCState(Zone::Mark);
+            atomsZone->changeGCState(Zone::NoGC, Zone::Mark);
             any = true;
         }
     }
 
     /* Check that at least one zone is scheduled for collection. */
     return any;
 }
 
@@ -4385,30 +4385,26 @@ js::gc::MarkingValidator::nonIncremental
     gc->incrementalState = State::Sweep;
     {
         gcstats::AutoPhase ap1(gc->stats(), gcstats::PhaseKind::SWEEP);
         gcstats::AutoPhase ap2(gc->stats(), gcstats::PhaseKind::SWEEP_MARK);
 
         gc->markAllWeakReferences(gcstats::PhaseKind::SWEEP_MARK_WEAK);
 
         /* Update zone state for gray marking. */
-        for (GCZonesIter zone(runtime); !zone.done(); zone.next()) {
-            MOZ_ASSERT(zone->isGCMarkingBlack());
-            zone->setGCState(Zone::MarkGray);
-        }
+        for (GCZonesIter zone(runtime); !zone.done(); zone.next())
+            zone->changeGCState(Zone::Mark, Zone::MarkGray);
         gc->marker.setMarkColorGray();
 
         gc->markAllGrayReferences(gcstats::PhaseKind::SWEEP_MARK_GRAY);
         gc->markAllWeakReferences(gcstats::PhaseKind::SWEEP_MARK_GRAY_WEAK);
 
         /* Restore zone state. */
-        for (GCZonesIter zone(runtime); !zone.done(); zone.next()) {
-            MOZ_ASSERT(zone->isGCMarkingGray());
-            zone->setGCState(Zone::Mark);
-        }
+        for (GCZonesIter zone(runtime); !zone.done(); zone.next())
+            zone->changeGCState(Zone::MarkGray, Zone::Mark);
         MOZ_ASSERT(gc->marker.isDrained());
         gc->marker.setMarkColorBlack();
     }
 
     /* Take a copy of the non-incremental mark state and restore the original. */
     for (auto chunk = gc->allNonEmptyChunks(); !chunk.done(); chunk.next()) {
         ChunkBitmap* bitmap = &chunk->bitmap;
         ChunkBitmap* entry = map.lookup(chunk)->value();
@@ -4706,19 +4702,18 @@ GCRuntime::getNextSweepGroup()
 
     if (!isIncremental)
         ZoneComponentFinder::mergeGroups(currentSweepGroup);
 
     if (abortSweepAfterCurrentGroup) {
         MOZ_ASSERT(!isIncremental);
         for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
             MOZ_ASSERT(!zone->gcNextGraphComponent);
-            MOZ_ASSERT(zone->isGCMarking());
             zone->setNeedsIncrementalBarrier(false);
-            zone->setGCState(Zone::NoGC);
+            zone->changeGCState(Zone::Mark, Zone::NoGC);
             zone->gcGrayRoots().clearAndFree();
         }
 
         for (GCCompartmentGroupIter comp(rt); !comp.done(); comp.next())
             ResetGrayList(comp);
 
         abortSweepAfterCurrentGroup = false;
         currentSweepGroup = nullptr;
@@ -4979,34 +4974,30 @@ GCRuntime::endMarkingSweepGroup()
     markWeakReferencesInCurrentGroup(gcstats::PhaseKind::SWEEP_MARK_WEAK);
 
     /*
      * Change state of current group to MarkGray to restrict marking to this
      * group.  Note that there may be pointers to the atoms compartment, and
      * these will be marked through, as they are not marked with
      * MarkCrossCompartmentXXX.
      */
-    for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
-        MOZ_ASSERT(zone->isGCMarkingBlack());
-        zone->setGCState(Zone::MarkGray);
-    }
+    for (GCSweepGroupIter zone(rt); !zone.done(); zone.next())
+        zone->changeGCState(Zone::Mark, Zone::MarkGray);
     marker.setMarkColorGray();
 
     /* Mark incoming gray pointers from previously swept compartments. */
     MarkIncomingCrossCompartmentPointers(rt, MarkColor::Gray);
 
     /* Mark gray roots and mark transitively inside the current compartment group. */
     markGrayReferencesInCurrentGroup(gcstats::PhaseKind::SWEEP_MARK_GRAY);
     markWeakReferencesInCurrentGroup(gcstats::PhaseKind::SWEEP_MARK_GRAY_WEAK);
 
     /* Restore marking state. */
-    for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
-        MOZ_ASSERT(zone->isGCMarkingGray());
-        zone->setGCState(Zone::Mark);
-    }
+    for (GCSweepGroupIter zone(rt); !zone.done(); zone.next())
+        zone->changeGCState(Zone::MarkGray, Zone::Mark);
     MOZ_ASSERT(marker.isDrained());
     marker.setMarkColorBlack();
 }
 
 // Causes the given WeakCache to be swept when run.
 class ImmediateSweepWeakCacheTask : public GCParallelTask
 {
     JS::detail::WeakCacheBase& cache;
@@ -5301,18 +5292,17 @@ GCRuntime::beginSweepingSweepGroup()
 
     using namespace gcstats;
 
     AutoSCC scc(stats(), sweepGroupIndex);
 
     bool sweepingAtoms = false;
     for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
         /* Set the GC state to sweeping. */
-        MOZ_ASSERT(zone->isGCMarking());
-        zone->setGCState(Zone::Sweep);
+        zone->changeGCState(Zone::Mark, Zone::Sweep);
 
         /* Purge the ArenaLists before sweeping. */
         zone->arenas.purge();
 
         if (zone->isAtomsZone())
             sweepingAtoms = true;
 
 #ifdef DEBUG
@@ -5407,19 +5397,18 @@ GCRuntime::endSweepingSweepGroup()
     {
         gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::FINALIZE_END);
         FreeOp fop(rt);
         callFinalizeCallbacks(&fop, JSFINALIZE_GROUP_END);
     }
 
     /* Update the GC state for zones we have swept. */
     for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
-        MOZ_ASSERT(zone->isGCSweeping());
         AutoLockGC lock(rt);
-        zone->setGCState(Zone::Finished);
+        zone->changeGCState(Zone::Sweep, Zone::Finished);
         zone->threshold.updateAfterGC(zone->usage.gcBytes(), invocationKind, tunables,
                                       schedulingState, lock);
     }
 
     /* Start background thread to sweep zones if required. */
     ZoneList zones;
     for (GCSweepGroupIter zone(rt); !zone.done(); zone.next())
         zones.append(zone);
@@ -6081,37 +6070,36 @@ GCRuntime::compactPhase(JS::gcreason::Re
     ZoneList relocatedZones;
     Arena* relocatedArenas = nullptr;
     while (!zonesToMaybeCompact.ref().isEmpty()) {
 
         Zone* zone = zonesToMaybeCompact.ref().front();
         zonesToMaybeCompact.ref().removeFront();
 
         MOZ_ASSERT(zone->group()->nursery().isEmpty());
-        MOZ_ASSERT(zone->isGCFinished());
-        zone->setGCState(Zone::Compact);
+        zone->changeGCState(Zone::Finished, Zone::Compact);
 
         if (relocateArenas(zone, reason, relocatedArenas, sliceBudget)) {
             updateZonePointersToRelocatedCells(zone, lock);
             relocatedZones.append(zone);
         } else {
-            zone->setGCState(Zone::Finished);
+            zone->changeGCState(Zone::Compact, Zone::Finished);
         }
 
         if (sliceBudget.isOverBudget())
             break;
     }
 
     if (!relocatedZones.isEmpty()) {
         updateRuntimePointersToRelocatedCells(lock);
 
         do {
             Zone* zone = relocatedZones.front();
             relocatedZones.removeFront();
-            zone->setGCState(Zone::Finished);
+            zone->changeGCState(Zone::Compact, Zone::Finished);
         }
         while (!relocatedZones.isEmpty());
     }
 
     if (ShouldProtectRelocatedArenas(reason))
         protectAndHoldArenas(relocatedArenas);
     else
         releaseRelocatedArenas(relocatedArenas);
@@ -6143,18 +6131,17 @@ GCRuntime::finishCollection(JS::gcreason
     clearBufferedGrayRoots();
     MemProfiler::SweepTenured(rt);
 
     uint64_t currentTime = PRMJ_Now();
     schedulingState.updateHighFrequencyMode(lastGCTime, currentTime, tunables);
 
     for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) {
         if (zone->isCollecting()) {
-            MOZ_ASSERT(zone->isGCFinished());
-            zone->setGCState(Zone::NoGC);
+            zone->changeGCState(Zone::Finished, Zone::NoGC);
         }
 
         MOZ_ASSERT(!zone->isCollectingFromAnyThread());
         MOZ_ASSERT(!zone->wasGCStarted());
     }
 
     MOZ_ASSERT(zonesToMaybeCompact.ref().isEmpty());
 
@@ -6248,19 +6235,18 @@ GCRuntime::resetIncrementalGC(gc::AbortR
         marker.reset();
         marker.stop();
         clearBufferedGrayRoots();
 
         for (GCCompartmentsIter c(rt); !c.done(); c.next())
             ResetGrayList(c);
 
         for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
-            MOZ_ASSERT(zone->isGCMarking());
             zone->setNeedsIncrementalBarrier(false);
-            zone->setGCState(Zone::NoGC);
+            zone->changeGCState(Zone::Mark, Zone::NoGC);
         }
 
         blocksToFreeAfterSweeping.ref().freeAll();
 
         incrementalState = State::NotActive;
 
         MOZ_ASSERT(!marker.shouldCheckCompartments());