Bug 1463462 - Make gray marking incremental r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 06 Dec 2018 16:27:22 -0500
changeset 450670 e3fc6ddd9a5316edac3737f92666e3cd0c08a44e
parent 450669 1aefa97ae11619e27931c3fbacb8ce777ed98720
child 450671 1544326ba29a387f1240415af38da7a33f5083ef
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 - Make gray marking incremental r=sfink
js/src/gc/GC.cpp
js/src/gc/GCRuntime.h
js/src/gc/Marking.cpp
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -924,16 +924,17 @@ GCRuntime::GCRuntime(JSRuntime* rt)
       safeToYield(true),
       sweepOnBackgroundThread(false),
       blocksToFreeAfterSweeping(
           (size_t)JSContext::TEMP_LIFO_ALLOC_PRIMARY_CHUNK_SIZE),
       sweepGroupIndex(0),
       sweepGroups(nullptr),
       currentSweepGroup(nullptr),
       sweepZone(nullptr),
+      hasMarkedGrayRoots(false),
       abortSweepAfterCurrentGroup(false),
       startedCompacting(false),
       relocatedArenasToRelease(nullptr),
 #ifdef JS_GC_ZEAL
       markingValidator(nullptr),
 #endif
       defaultTimeBudget_(TuningDefaults::DefaultTimeBudget),
       incrementalAllowed(true),
@@ -4958,17 +4959,17 @@ void GCRuntime::getNextSweepGroup() {
   }
 
   MOZ_ASSERT_IF(abortSweepAfterCurrentGroup, !isIncremental);
   if (!isIncremental) {
     ZoneComponentFinder::mergeGroups(currentSweepGroup);
   }
 
   for (Zone* zone = currentSweepGroup; zone; zone = zone->nextNodeInGroup()) {
-    MOZ_ASSERT(zone->isGCMarking());
+    MOZ_ASSERT(zone->isGCMarkingBlackOnly());
     MOZ_ASSERT(!zone->isQueuedForBackgroundSweep());
   }
 
   if (abortSweepAfterCurrentGroup) {
     for (SweepGroupZonesIter zone(rt); !zone.done(); zone.next()) {
       MOZ_ASSERT(!zone->gcNextGraphComponent);
       zone->setNeedsIncrementalBarrier(false);
       zone->changeGCState(Zone::MarkBlackOnly, Zone::NoGC);
@@ -4978,16 +4979,18 @@ void GCRuntime::getNextSweepGroup() {
 
     for (SweepGroupCompartmentsIter comp(rt); !comp.done(); comp.next()) {
       ResetGrayList(comp);
     }
 
     abortSweepAfterCurrentGroup = false;
     currentSweepGroup = nullptr;
   }
+
+  hasMarkedGrayRoots = false;
 }
 
 /*
  * Gray marking:
  *
  * At the end of collection, anything reachable from a gray root that has not
  * otherwise been marked black must be marked gray.
  *
@@ -5129,18 +5132,16 @@ void GCRuntime::markIncomingCrossCompart
         }
       }
     }
 
     if (unlinkList) {
       c->gcIncomingGrayPointers = nullptr;
     }
   }
-
-  drainMarkStack();
 }
 
 static bool RemoveFromGrayList(JSObject* wrapper) {
   AutoTouchingGrayThings tgt;
 
   if (!IsGrayListObject(wrapper)) {
     return false;
   }
@@ -5249,42 +5250,45 @@ static inline void MaybeCheckWeakMapMark
 
 #endif
 }
 
 IncrementalProgress GCRuntime::markGrayReferencesInCurrentGroup(
     FreeOp* fop, SliceBudget& budget) {
   MOZ_ASSERT(marker.markColor() == MarkColor::Black);
 
+  if (hasMarkedGrayRoots) {
+    return Finished;
+  }
+
   gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::SWEEP_MARK);
 
   // 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);
+  drainMarkStack();
 
   // 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);
 
   markGrayRoots<SweepGroupZonesIter>(gcstats::PhaseKind::SWEEP_MARK_GRAY);
 
-  // TODO: Make this incremental.
-  drainMarkStack();
-
-  return Finished;
+  hasMarkedGrayRoots = true;
+  return marker.markUntilBudgetExhausted(budget) ? Finished : NotFinished;
 }
 
 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);
@@ -5780,26 +5784,24 @@ void GCRuntime::beginSweepPhase(JS::gcre
 
   computeNonIncrementalMarkingForValidation(session);
 
   gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::SWEEP);
 
   sweepOnBackgroundThread = reason != JS::gcreason::DESTROY_RUNTIME &&
                             !gcTracer.traceEnabled() && CanUseExtraThreads();
 
+  hasMarkedGrayRoots = false;
+
   AssertNoWrappersInGrayList(rt);
   DropStringWrappers(rt);
 
   groupZonesForSweeping(reason);
 
   sweepActions->assertFinished();
-
-  // We must not yield after this point until we start sweeping the first sweep
-  // group.
-  safeToYield = false;
 }
 
 bool ArenaLists::foregroundFinalize(FreeOp* fop, AllocKind thingKind,
                                     SliceBudget& sliceBudget,
                                     SortedArenaList& sweepList) {
   if (!arenaListsToSweep(thingKind) && incrementalSweptArenas.ref().isEmpty()) {
     return true;
   }
@@ -6783,18 +6785,16 @@ GCRuntime::IncrementalResult GCRuntime::
 
       MOZ_ASSERT(!marker.shouldCheckCompartments());
 
       break;
     }
 
     case State::Sweep: {
       // Finish sweeping the current sweep group, then abort.
-      marker.reset();
-
       for (CompartmentsIter c(rt); !c.done(); c.next()) {
         c->gcState.scheduledForDestruction = false;
       }
 
       abortSweepAfterCurrentGroup = true;
       isCompacting = false;
 
       break;
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -888,16 +888,17 @@ class GCRuntime {
 
   MainThreadData<JS::Zone*> sweepGroups;
   MainThreadOrGCTaskData<JS::Zone*> currentSweepGroup;
   MainThreadData<UniquePtr<SweepAction<GCRuntime*, FreeOp*, SliceBudget&>>>
       sweepActions;
   MainThreadOrGCTaskData<JS::Zone*> sweepZone;
   MainThreadData<mozilla::Maybe<AtomsTable::SweepIterator>> maybeAtomsToSweep;
   MainThreadOrGCTaskData<JS::detail::WeakCacheBase*> sweepCache;
+  MainThreadData<bool> hasMarkedGrayRoots;
   MainThreadData<bool> abortSweepAfterCurrentGroup;
 
   friend class SweepGroupsIter;
   friend class WeakCacheSweepIterator;
 
   /*
    * Incremental compacting state.
    */
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -314,26 +314,37 @@ static inline bool ShouldMarkCrossCompar
   }
 
   if (color == MarkColor::Black) {
     // Check our sweep groups are correct: we should never have to
     // mark something in a zone that we have started sweeping.
     MOZ_ASSERT_IF(!dst.isMarkedBlack(), !dstZone->isGCSweeping());
 
     /*
-     * Having black->gray edges violates our promise to the cycle
-     * collector. This can happen if we're collecting a compartment and it
-     * has an edge to an uncollected compartment: it's possible that the
-     * source and destination of the cross-compartment edge should be gray,
-     * but the source was marked black by the write barrier.
+     * Having black->gray edges violates our promise to the cycle collector so
+     * we ensure that gray things we encounter when marking black end up getting
+     * marked black.
+     *
+     * This can happen for two reasons:
+     *
+     * 1) If we're collecting a compartment and it has an edge to an uncollected
+     * compartment it's possible that the source and destination of the
+     * cross-compartment edge should be gray, but the source was marked black by
+     * the write barrier.
+     *
+     * 2) If we yield during gray marking and the write barrier marks a gray
+     * thing black.
+     *
+     * We handle the first case before returning whereas the second case happens
+     * as part of normal marking.
      */
-    if (dst.isMarkedGray()) {
-      MOZ_ASSERT(!dstZone->isCollecting());
+    if (dst.isMarkedGray() && !dstZone->isGCMarking()) {
       UnmarkGrayGCThing(marker->runtime(),
                         JS::GCCellPtr(&dst, dst.getTraceKind()));
+      return false;
     }
 
     return dstZone->isGCMarking();
   } else {
     // Check our sweep groups are correct as above.
     MOZ_ASSERT_IF(!dst.isMarkedAny(), !dstZone->isGCSweeping());
 
     if (dstZone->isGCMarkingBlackOnly()) {
@@ -1604,17 +1615,16 @@ bool GCMarker::markUntilBudgetExhausted(
       }
     }
 
     if (hasGrayEntries()) {
       AutoSetMarkColor autoSetGray(*this, MarkColor::Gray);
       do {
         processMarkStackTop(budget);
         if (budget.isOverBudget()) {
-          MOZ_CRASH("Incremental gray marking NYI");
           return false;
         }
       } while (hasGrayEntries());
     }
 
     if (!hasDelayedChildren()) {
       break;
     }
@@ -2470,17 +2480,17 @@ void GCMarker::pushValueArray(JSObject* 
 
   if (!stack.push(obj, start, end)) {
     delayMarkingChildren(obj);
   }
 }
 
 void GCMarker::repush(JSObject* obj) {
   MOZ_ASSERT_IF(markColor() == MarkColor::Gray,
-                gc::TenuredCell::fromPointer(obj)->isMarkedGray());
+                gc::TenuredCell::fromPointer(obj)->isMarkedAny());
   MOZ_ASSERT_IF(markColor() == MarkColor::Black,
                 gc::TenuredCell::fromPointer(obj)->isMarkedBlack());
   pushTaggedPtr(obj);
 }
 
 void GCMarker::enterWeakMarkingMode() {
   MOZ_ASSERT(tag_ == TracerKindTag::Marking);
   if (linearWeakMarkingDisabled_) {
@@ -3497,30 +3507,63 @@ class UnmarkGrayTracer : public JS::Call
 
   void onChild(const JS::GCCellPtr& thing) override;
 
 #ifdef DEBUG
   TracerKind getTracerKind() const override { return TracerKind::UnmarkGray; }
 #endif
 };
 
+static bool
+IsCCTraceKindInternal(JS::TraceKind kind)
+{
+    switch (kind) {
+#define EXPAND_IS_CC_TRACE_KIND(name, _, addToCCKind)    \
+      case JS::TraceKind::name:                          \
+        return addToCCKind;
+JS_FOR_EACH_TRACEKIND(EXPAND_IS_CC_TRACE_KIND)
+      default:
+        MOZ_CRASH("Unexpected trace kind");
+    }
+}
+
 void UnmarkGrayTracer::onChild(const JS::GCCellPtr& thing) {
   Cell* cell = thing.asCell();
 
-  // Cells in the nursery cannot be gray, and therefore must necessarily point
-  // to only black edges.
-  if (!cell->isTenured()) {
+  // Cells in the nursery cannot be gray, and nor can certain kinds of tenured
+  // cells. These must necessarily point only to black edges.
+  if (!cell->isTenured() ||
+      !IsCCTraceKindInternal(cell->asTenured().getTraceKind())) {
 #ifdef DEBUG
+    MOZ_ASSERT(!cell->isMarkedGray());
     AssertNonGrayTracer nongray(runtime());
     TraceChildren(&nongray, cell, thing.kind());
 #endif
     return;
   }
 
   TenuredCell& tenured = cell->asTenured();
+
+  // If the cell is in a zone that we're currently marking gray, then it's
+  // possible that it is currently white but will end up gray. To handle this
+  // case, push any cells in zones that are currently being marked onto the
+  // mark stack and they will eventually get marked black.
+  Zone* zone = tenured.zone();
+  if (zone->needsIncrementalBarrier()) {
+    if (!cell->isMarkedBlack()) {
+      Cell* tmp = cell;
+      TraceManuallyBarrieredGenericPointerEdge(zone->barrierTracer(), &tmp,
+                                               "read barrier");
+      MOZ_ASSERT(tmp == cell);
+      unmarkedAny = true;
+    }
+    return;
+  }
+
+  MOZ_ASSERT(!zone->isGCMarkingBlackAndGray());
   if (!tenured.isMarkedGray()) {
     return;
   }
 
   tenured.markBlack();
   unmarkedAny = true;
 
   if (!stack.append(thing)) {
@@ -3551,16 +3594,17 @@ bool js::IsUnmarkGrayTracer(JSTracer* tr
   return trc->isCallbackTracer() &&
          trc->asCallbackTracer()->getTracerKind() ==
              JS::CallbackTracer::TracerKind::UnmarkGray;
 }
 #endif
 
 static bool UnmarkGrayGCThing(JSRuntime* rt, JS::GCCellPtr thing) {
   MOZ_ASSERT(thing);
+  MOZ_ASSERT(thing.asCell()->isMarkedGray());
 
   // Gray cell unmarking can occur at different points between recording and
   // replay, so disallow recorded events from occurring in the tracer.
   mozilla::recordreplay::AutoDisallowThreadEvents d;
 
   UnmarkGrayTracer unmarker(rt);
   gcstats::AutoPhase innerPhase(rt->gc.stats(),
                                 gcstats::PhaseKind::UNMARK_GRAY);