Bug 1463462 - Add a non-incremental gray marking phase before weakmap marking that marks from gray roots in the current sweep group r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 06 Dec 2018 16:27:22 -0500
changeset 450668 6ae14f44b4af53cb2ecf23eb5077ab90d1615ea9
parent 450667 3e7a4e085ead5b20172ee33e4809770dd124a8f7
child 450669 1aefa97ae11619e27931c3fbacb8ce777ed98720
push id110516
push userjcoppeard@mozilla.com
push dateFri, 14 Dec 2018 14:22:31 +0000
treeherdermozilla-inbound@fd4d12eb1b97 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1463462
milestone66.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 1463462 - Add a non-incremental gray marking phase before weakmap marking that marks from gray roots in the current sweep group r=sfink
js/src/gc/GC.cpp
js/src/gc/GCRuntime.h
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -4455,43 +4455,39 @@ void GCRuntime::markWeakReferences(gcsta
   marker.leaveWeakMarkingMode();
 }
 
 void GCRuntime::markWeakReferencesInCurrentGroup(gcstats::PhaseKind phase) {
   markWeakReferences<SweepGroupZonesIter>(phase);
 }
 
 template <class ZoneIterT>
-void GCRuntime::markGrayReferences(gcstats::PhaseKind phase) {
+void GCRuntime::markGrayRoots(gcstats::PhaseKind phase) {
   MOZ_ASSERT(marker.markColor() == MarkColor::Gray);
 
   gcstats::AutoPhase ap(stats(), phase);
   if (hasValidGrayRootsBuffer()) {
     for (ZoneIterT zone(rt); !zone.done(); zone.next()) {
       markBufferedGrayRoots(zone);
     }
   } else {
     MOZ_ASSERT(!isIncremental);
     if (JSTraceDataOp op = grayRootTracer.op) {
       (*op)(&marker, grayRootTracer.data);
     }
   }
-  drainMarkStack();
-}
-
-void GCRuntime::markGrayReferencesInCurrentGroup(gcstats::PhaseKind phase) {
-  markGrayReferences<SweepGroupZonesIter>(phase);
 }
 
 void GCRuntime::markAllWeakReferences(gcstats::PhaseKind phase) {
   markWeakReferences<GCZonesIter>(phase);
 }
 
 void GCRuntime::markAllGrayReferences(gcstats::PhaseKind phase) {
-  markGrayReferences<GCZonesIter>(phase);
+  markGrayRoots<GCZonesIter>(phase);
+  drainMarkStack();
 }
 
 #ifdef JS_GC_ZEAL
 
 struct GCChunkHasher {
   typedef gc::Chunk* Lookup;
 
   /*
@@ -5105,19 +5101,19 @@ void GCRuntime::markIncomingCrossCompart
   static const gcstats::PhaseKind statsPhases[] = {
       gcstats::PhaseKind::SWEEP_MARK_INCOMING_BLACK,
       gcstats::PhaseKind::SWEEP_MARK_INCOMING_GRAY};
   gcstats::AutoPhase ap1(stats(), statsPhases[unsigned(color)]);
 
   bool unlinkList = color == MarkColor::Gray;
 
   for (SweepGroupCompartmentsIter c(rt); !c.done(); c.next()) {
+    MOZ_ASSERT(c->zone()->isGCMarking());
     MOZ_ASSERT_IF(color == MarkColor::Gray,
                   c->zone()->isGCMarkingBlackAndGray());
-    MOZ_ASSERT_IF(color == MarkColor::Black, c->zone()->isGCMarkingBlackOnly());
     MOZ_ASSERT_IF(c->gcIncomingGrayPointers,
                   IsGrayListObject(c->gcIncomingGrayPointers));
 
     for (JSObject* src = c->gcIncomingGrayPointers; src;
          src = NextIncomingCrossCompartmentPointer(src, unlinkList)) {
       JSObject* dst = CrossCompartmentPointerReferent(src);
       MOZ_ASSERT(dst->compartment() == c);
 
@@ -5181,16 +5177,28 @@ static bool RemoveFromGrayList(JSObject*
 static void ResetGrayList(Compartment* comp) {
   JSObject* src = comp->gcIncomingGrayPointers;
   while (src) {
     src = NextIncomingCrossCompartmentPointer(src, true);
   }
   comp->gcIncomingGrayPointers = nullptr;
 }
 
+#ifdef DEBUG
+static bool HasIncomingCrossCompartmentPointers(JSRuntime* rt) {
+  for (SweepGroupCompartmentsIter c(rt); !c.done(); c.next()) {
+    if (c->gcIncomingGrayPointers) {
+      return true;
+    }
+  }
+
+  return false;
+}
+#endif
+
 void js::NotifyGCNukeWrapper(JSObject* obj) {
   /*
    * References to target of wrapper are being removed, we no longer have to
    * remember to mark it.
    */
   RemoveFromGrayList(obj);
 }
 
@@ -5237,44 +5245,60 @@ static inline void MaybeCheckWeakMapMark
     for (SweepGroupZonesIter zone(gc->rt); !zone.done(); zone.next()) {
       MOZ_RELEASE_ASSERT(WeakMapBase::checkMarkingForZone(zone));
     }
   }
 
 #endif
 }
 
-IncrementalProgress GCRuntime::endMarkingSweepGroup(FreeOp* fop,
-                                                    SliceBudget& budget) {
+IncrementalProgress GCRuntime::markGrayReferencesInCurrentGroup(
+    FreeOp* fop, SliceBudget& budget) {
   MOZ_ASSERT(marker.markColor() == MarkColor::Black);
 
   gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::SWEEP_MARK);
 
-  // Mark any incoming black pointers from previously swept compartments
-  // whose referents are not marked. This can occur when gray cells become
-  // black by the action of UnmarkGray.
+  // Mark any incoming gray pointers from previously swept compartments that
+  // have subsequently been marked black. This can occur when gray cells
+  // become black by the action of UnmarkGray.
   markIncomingCrossCompartmentPointers(MarkColor::Black);
-  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 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::MarkBlackOnly, Zone::MarkBlackAndGray);
   }
 
   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);
+  markGrayRoots<SweepGroupZonesIter>(gcstats::PhaseKind::SWEEP_MARK_GRAY);
+
+  // TODO: Make this incremental.
+  drainMarkStack();
+
+  return Finished;
+}
+
+IncrementalProgress GCRuntime::endMarkingSweepGroup(FreeOp* fop,
+                                                    SliceBudget& budget) {
+  MOZ_ASSERT(marker.markColor() == MarkColor::Black);
+  MOZ_ASSERT(!HasIncomingCrossCompartmentPointers(rt));
+
+  gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::SWEEP_MARK);
+
+  markWeakReferencesInCurrentGroup(gcstats::PhaseKind::SWEEP_MARK_WEAK);
+
+  AutoSetMarkColor setColorGray(marker, MarkColor::Gray);
+
+  // Mark transitively inside the current compartment group.
   markWeakReferencesInCurrentGroup(gcstats::PhaseKind::SWEEP_MARK_GRAY_WEAK);
 
   MOZ_ASSERT(marker.isDrained());
 
   // We must not yield after this point before we start sweeping the group.
   safeToYield = false;
 
   MaybeCheckWeakMapMarking(this);
@@ -6436,16 +6460,17 @@ ForEachAllocKind(AllocKinds kinds, Uniqu
 
 bool GCRuntime::initSweepActions() {
   using namespace sweepaction;
   using sweepaction::Call;
 
   sweepActions.ref() = RepeatForSweepGroup(
       rt,
       Sequence(
+          Call(&GCRuntime::markGrayReferencesInCurrentGroup),
           Call(&GCRuntime::endMarkingSweepGroup),
           Call(&GCRuntime::beginSweepingSweepGroup),
           MaybeYield(ZealMode::IncrementalMultipleSlices),
           MaybeYield(ZealMode::YieldBeforeSweepingAtoms),
           Call(&GCRuntime::sweepAtomsTable),
           MaybeYield(ZealMode::YieldBeforeSweepingCaches),
           Call(&GCRuntime::sweepWeakCaches),
           ForEachZoneInSweepGroup(
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -620,26 +620,27 @@ class GCRuntime {
   void markCompartments();
   IncrementalProgress markUntilBudgetExhausted(SliceBudget& sliceBudget,
                                                gcstats::PhaseKind phase);
   void drainMarkStack();
   template <class ZoneIterT>
   void markWeakReferences(gcstats::PhaseKind phase);
   void markWeakReferencesInCurrentGroup(gcstats::PhaseKind phase);
   template <class ZoneIterT>
-  void markGrayReferences(gcstats::PhaseKind phase);
+  void markGrayRoots(gcstats::PhaseKind phase);
   void markBufferedGrayRoots(JS::Zone* zone);
-  void markGrayReferencesInCurrentGroup(gcstats::PhaseKind phase);
   void markAllWeakReferences(gcstats::PhaseKind phase);
   void markAllGrayReferences(gcstats::PhaseKind phase);
 
   void beginSweepPhase(JS::gcreason::Reason reason, AutoGCSession& session);
   void groupZonesForSweeping(JS::gcreason::Reason reason);
   MOZ_MUST_USE bool findInterZoneEdges();
   void getNextSweepGroup();
+  IncrementalProgress markGrayReferencesInCurrentGroup(FreeOp* fop,
+                                                       SliceBudget& budget);
   IncrementalProgress endMarkingSweepGroup(FreeOp* fop, SliceBudget& budget);
   void markIncomingCrossCompartmentPointers(MarkColor color);
   IncrementalProgress beginSweepingSweepGroup(FreeOp* fop, SliceBudget& budget);
   void sweepDebuggerOnMainThread(FreeOp* fop);
   void sweepJitDataOnMainThread(FreeOp* fop);
   IncrementalProgress endSweepingSweepGroup(FreeOp* fop, SliceBudget& budget);
   IncrementalProgress performSweepActions(SliceBudget& sliceBudget);
   IncrementalProgress sweepTypeInformation(FreeOp* fop, SliceBudget& budget,