Bug 1509824 - Add RAII class AutoSetMarkColor to handle changing the mark color r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Mon, 26 Nov 2018 13:34:01 +0000
changeset 447958 3785e94a44d084a5698fb86bb1afe4218ee63e02
parent 447957 bec065574446df6a0a4ba3235809ae2e0515ea64
child 447959 d7336bdef207654c48c3e0c6c976f3ed65fe5ccd
push id110103
push userjcoppeard@mozilla.com
push dateMon, 26 Nov 2018 13:36:02 +0000
treeherdermozilla-inbound@d7336bdef207 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1509824
milestone65.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 1509824 - Add RAII class AutoSetMarkColor to handle changing the mark color r=sfink
js/src/gc/GC.cpp
js/src/gc/GCMarker.h
js/src/gc/Marking.cpp
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -5004,27 +5004,27 @@ js::gc::MarkingValidator::nonIncremental
         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()) {
             zone->changeGCState(Zone::Mark, Zone::MarkGray);
         }
-        gc->marker.setMarkColorGray();
+
+        AutoSetMarkColor setColorGray(gc->marker, MarkColor::Gray);
 
         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()) {
             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. */
     {
         AutoLockGC lock(runtime);
         for (auto chunk = gc->allNonEmptyChunks(lock); !chunk.done(); chunk.next()) {
             ChunkBitmap* bitmap = &chunk->bitmap;
             ChunkBitmap* entry = map.lookup(chunk)->value().get();
@@ -5630,32 +5630,32 @@ GCRuntime::endMarkingSweepGroup(FreeOp* 
 
     // Change state of current group to MarkGray to restrict marking to this
     // group.  Note that there may be pointers to the atoms zone, and
     // these will be marked through, as they are not marked with
     // TraceCrossCompartmentEdge.
     for (SweepGroupZonesIter zone(rt); !zone.done(); zone.next()) {
         zone->changeGCState(Zone::Mark, Zone::MarkGray);
     }
-    marker.setMarkColorGray();
+
+    AutoSetMarkColor setColorGray(marker, MarkColor::Gray);
 
     // Mark incoming gray pointers from previously swept compartments.
     markIncomingCrossCompartmentPointers(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 (SweepGroupZonesIter zone(rt); !zone.done(); zone.next()) {
         zone->changeGCState(Zone::MarkGray, Zone::Mark);
     }
     MOZ_ASSERT(marker.isDrained());
-    marker.setMarkColorBlack();
 
     // We must not yield after this point before we start sweeping the group.
     safeToYield = false;
 
     return Finished;
 }
 
 // Causes the given WeakCache to be swept when run.
--- a/js/src/gc/GCMarker.h
+++ b/js/src/gc/GCMarker.h
@@ -265,26 +265,19 @@ class GCMarker : public JSTracer
 
     /*
      * Care must be taken changing the mark color from gray to black. The cycle
      * collector depends on the invariant that there are no black to gray edges
      * in the GC heap. This invariant lets the CC not trace through black
      * objects. If this invariant is violated, the cycle collector may free
      * objects that are still reachable.
      */
-    void setMarkColorGray() {
-        MOZ_ASSERT(isDrained());
-        MOZ_ASSERT(color == gc::MarkColor::Black);
-        color = gc::MarkColor::Gray;
-    }
-    void setMarkColorBlack() {
-        MOZ_ASSERT(isDrained());
-        MOZ_ASSERT(color == gc::MarkColor::Gray);
-        color = gc::MarkColor::Black;
-    }
+    void setMarkColorGray();
+    void setMarkColorBlack();
+    void setMarkColor(gc::MarkColor newColor);
     gc::MarkColor markColor() const { return color; }
 
     void enterWeakMarkingMode();
     void leaveWeakMarkingMode();
     void abortLinearWeakMarking() {
         leaveWeakMarkingMode();
         linearWeakMarkingDisabled_ = true;
     }
@@ -394,16 +387,44 @@ class GCMarker : public JSTracer
     /*
      * If this is true, all marked objects must belong to a compartment being
      * GCed. This is used to look for compartment bugs.
      */
     MainThreadData<bool> strictCompartmentChecking;
 #endif // DEBUG
 };
 
+namespace gc {
+
+/*
+ * Temporarily change the mark color while this class is on the stack.
+ *
+ * During incremental sweeping this also transitions zones in the
+ * current sweep group into the Mark or MarkGray state as appropriate.
+ */
+class MOZ_RAII AutoSetMarkColor
+{
+    GCMarker& marker_;
+    MarkColor initialColor_;
+
+  public:
+    AutoSetMarkColor(GCMarker& marker, MarkColor newColor)
+      : marker_(marker),
+        initialColor_(marker.markColor())
+    {
+        marker_.setMarkColor(newColor);
+    }
+
+    ~AutoSetMarkColor() {
+        marker_.setMarkColor(initialColor_);
+    }
+};
+
+} /* namespace gc */
+
 } /* namespace js */
 
 // Exported for Tracer.cpp
 inline bool ThingIsPermanentAtomOrWellKnownSymbol(js::gc::Cell* thing) { return false; }
 bool ThingIsPermanentAtomOrWellKnownSymbol(JSString*);
 bool ThingIsPermanentAtomOrWellKnownSymbol(JSFlatString*);
 bool ThingIsPermanentAtomOrWellKnownSymbol(JSLinearString*);
 bool ThingIsPermanentAtomOrWellKnownSymbol(JSAtom*);
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -2555,16 +2555,48 @@ GCMarker::reset()
 #ifdef DEBUG
         markLaterArenas--;
 #endif
     }
     MOZ_ASSERT(isDrained());
     MOZ_ASSERT(!markLaterArenas);
 }
 
+void
+GCMarker::setMarkColor(gc::MarkColor newColor)
+{
+    if (color == newColor) {
+        return;
+    }
+    if (newColor == gc::MarkColor::Black) {
+        setMarkColorBlack();
+    } else {
+        setMarkColorGray();
+    }
+}
+
+void
+GCMarker::setMarkColorGray()
+{
+    MOZ_ASSERT(isDrained());
+    MOZ_ASSERT(color == gc::MarkColor::Black);
+    MOZ_ASSERT(runtime()->gc.state() == State::Sweep);
+
+    color = gc::MarkColor::Gray;
+}
+
+void
+GCMarker::setMarkColorBlack()
+{
+    MOZ_ASSERT(isDrained());
+    MOZ_ASSERT(color == gc::MarkColor::Gray);
+    MOZ_ASSERT(runtime()->gc.state() == State::Sweep);
+
+    color = gc::MarkColor::Black;
+}
 
 template <typename T>
 void
 GCMarker::pushTaggedPtr(T* ptr)
 {
     checkZone(ptr);
     if (!stack.push(ptr)) {
         delayMarkingChildren(ptr);