Bug 1384885 - Do gray unmarking of cross zone edges as soon as they are found r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 28 Jul 2017 11:06:40 +0100
changeset 420407 75902ad4c97a19b048160e3ef2470ad56ddfdcee
parent 420406 5f8790aae878be61fcb64ff03fcdf51ae2689166
child 420408 e0d2ec43d7b204be47b195c566b55eb8723e5cc6
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
bugs1384885
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 1384885 - Do gray unmarking of cross zone edges as soon as they are found r=sfink
js/public/TracingAPI.h
js/src/gc/GCRuntime.h
js/src/gc/GenerateStatsPhases.py
js/src/gc/Heap.h
js/src/gc/Marking.cpp
js/src/gc/Marking.h
js/src/gc/Statistics.cpp
js/src/jsgc.cpp
--- a/js/public/TracingAPI.h
+++ b/js/public/TracingAPI.h
@@ -218,17 +218,18 @@ class JS_PUBLIC_API(CallbackTracer) : pu
     };
 
 #ifdef DEBUG
     enum class TracerKind {
         DoNotCare,
         Moving,
         GrayBuffering,
         VerifyTraceProtoAndIface,
-        ClearEdges
+        ClearEdges,
+        UnmarkGray
     };
     virtual TracerKind getTracerKind() const { return TracerKind::DoNotCare; }
 #endif
 
     // In C++, overriding a method hides all methods in the base class with
     // that name, not just methods with that signature. Thus, the typed edge
     // methods have to have distinct names to allow us to override them
     // individually, which is freqently useful if, for example, we only want to
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -845,21 +845,16 @@ class GCRuntime
     JS::GCNurseryCollectionCallback setNurseryCollectionCallback(
         JS::GCNurseryCollectionCallback callback);
     JS::DoCycleCollectionCallback setDoCycleCollectionCallback(JS::DoCycleCollectionCallback callback);
     void callDoCycleCollectionCallback(JSContext* cx);
 
     void setFullCompartmentChecks(bool enable);
 
     JS::Zone* getCurrentSweepGroup() { return currentSweepGroup; }
-    void setFoundBlackGrayEdges(TenuredCell& target) {
-        AutoEnterOOMUnsafeRegion oomUnsafe;
-        if (!foundBlackGrayEdges.ref().append(&target))
-            oomUnsafe.crash("OOM|small: failed to insert into foundBlackGrayEdges");
-    }
 
     uint64_t gcNumber() const { return number; }
 
     uint64_t minorGCCount() const { return minorGCNumber; }
     void incMinorGcNumber() { ++minorGCNumber; ++number; }
 
     uint64_t majorGCCount() const { return majorGCNumber; }
     void incMajorGcNumber() { ++majorGCNumber; ++number; }
@@ -1247,19 +1242,16 @@ class GCRuntime
     ActiveThreadData<bool> lastMarkSlice;
 
     /* Whether any sweeping will take place in the separate GC helper thread. */
     ActiveThreadData<bool> sweepOnBackgroundThread;
 
     /* Whether observed type information is being released in the current GC. */
     ActiveThreadData<bool> releaseObservedTypes;
 
-    /* Whether any black->gray edges were found during marking. */
-    ActiveThreadData<BlackGrayEdgeVector> foundBlackGrayEdges;
-
     /* Singly linked list of zones to be swept in the background. */
     ActiveThreadOrGCTaskData<ZoneList> backgroundSweepZones;
 
     /*
      * Free LIFO blocks are transferred to this allocator before being freed on
      * the background GC thread after sweeping.
      */
     ActiveThreadOrGCTaskData<LifoAlloc> blocksToFreeAfterSweeping;
--- a/js/src/gc/GenerateStatsPhases.py
+++ b/js/src/gc/GenerateStatsPhases.py
@@ -66,16 +66,18 @@ MarkRootsPhaseKind = PhaseKind("MARK_ROO
     PhaseKind("MARK_STACK", "Mark C and JS stacks", 51),
     PhaseKind("MARK_RUNTIME_DATA", "Mark Runtime-wide Data", 52),
     PhaseKind("MARK_EMBEDDING", "Mark Embedding", 53),
     PhaseKind("MARK_COMPARTMENTS", "Mark Compartments", 54)
 ])
 
 JoinParallelTasksPhaseKind = PhaseKind("JOIN_PARALLEL_TASKS", "Join Parallel Tasks", 67)
 
+UnmarkGrayPhaseKind = PhaseKind("UNMARK_GRAY", "Unmark gray", 56)
+
 PhaseKindGraphRoots = [
     PhaseKind("MUTATOR", "Mutator Running", 0),
     PhaseKind("GC_BEGIN", "Begin Callback", 1),
     PhaseKind("EVICT_NURSERY_FOR_MAJOR_GC", "Evict Nursery For Major GC", 70, [
         MarkRootsPhaseKind,
     ]),
     PhaseKind("WAIT_BACKGROUND_THREAD", "Wait Background Thread", 2),
     PhaseKind("PREPARE", "Prepare For Collection", 69, [
@@ -84,23 +86,28 @@ PhaseKindGraphRoots = [
         PhaseKind("MARK_DISCARD_CODE", "Mark Discard Code", 3),
         PhaseKind("RELAZIFY_FUNCTIONS", "Relazify Functions", 4),
         PhaseKind("PURGE", "Purge", 5),
         PhaseKind("PURGE_SHAPE_TABLES", "Purge ShapeTables", 60),
         JoinParallelTasksPhaseKind
         ]),
     PhaseKind("MARK", "Mark", 6, [
         MarkRootsPhaseKind,
+        UnmarkGrayPhaseKind,
         PhaseKind("MARK_DELAYED", "Mark Delayed", 8)
         ]),
     PhaseKind("SWEEP", "Sweep", 9, [
         PhaseKind("SWEEP_MARK", "Mark During Sweeping", 10, [
-            PhaseKind("SWEEP_MARK_TYPES", "Mark Types During Sweeping", 11),
-            PhaseKind("SWEEP_MARK_INCOMING_BLACK", "Mark Incoming Black Pointers", 12),
-            PhaseKind("SWEEP_MARK_WEAK", "Mark Weak", 13),
+            UnmarkGrayPhaseKind,
+            PhaseKind("SWEEP_MARK_INCOMING_BLACK", "Mark Incoming Black Pointers", 12, [
+                UnmarkGrayPhaseKind,
+            ]),
+            PhaseKind("SWEEP_MARK_WEAK", "Mark Weak", 13, [
+                UnmarkGrayPhaseKind,
+            ]),
             PhaseKind("SWEEP_MARK_INCOMING_GRAY", "Mark Incoming Gray Pointers", 14),
             PhaseKind("SWEEP_MARK_GRAY", "Mark Gray", 15),
             PhaseKind("SWEEP_MARK_GRAY_WEAK", "Mark Gray and Weak", 16)
         ]),
         PhaseKind("FINALIZE_START", "Finalize Start Callbacks", 17, [
             PhaseKind("WEAK_ZONES_CALLBACK", "Per-Slice Weak Callback", 57),
             PhaseKind("WEAK_COMPARTMENT_CALLBACK", "Per-Compartment Weak Callback", 58)
         ]),
@@ -152,17 +159,17 @@ PhaseKindGraphRoots = [
     ]),
     PhaseKind("EVICT_NURSERY", "Minor GCs to Evict Nursery", 46, [
         MarkRootsPhaseKind,
     ]),
     PhaseKind("TRACE_HEAP", "Trace Heap", 47, [
         MarkRootsPhaseKind,
     ]),
     PhaseKind("BARRIER", "Barriers", 55, [
-        PhaseKind("UNMARK_GRAY", "Unmark gray", 56),
+        UnmarkGrayPhaseKind
     ])
 ]
 
 # Make a linear list of all unique phases by performing a depth first
 # search on the phase graph starting at the roots.  This will be used to
 # generate the PhaseKind enum.
 
 def findAllPhaseKinds():
--- a/js/src/gc/Heap.h
+++ b/js/src/gc/Heap.h
@@ -43,20 +43,16 @@ RuntimeFromActiveCooperatingThreadIsHeap
 #ifdef DEBUG
 
 // Barriers can't be triggered during backend Ion compilation, which may run on
 // a helper thread.
 extern bool
 CurrentThreadIsIonCompiling();
 #endif
 
-// The return value indicates if anything was unmarked.
-extern bool
-UnmarkGrayCellRecursively(gc::Cell* cell, JS::TraceKind kind);
-
 extern void
 TraceManuallyBarrieredGenericPointerEdge(JSTracer* trc, gc::Cell** thingp, const char* name);
 
 namespace gc {
 
 class Arena;
 class ArenaCellSet;
 class ArenaList;
@@ -1381,17 +1377,17 @@ TenuredCell::readBarrier(TenuredCell* th
         TraceManuallyBarrieredGenericPointerEdge(shadowZone->barrierTracer(), &tmp, "read barrier");
         MOZ_ASSERT(tmp == thing);
     }
 
     if (thing->isMarkedGray()) {
         // There shouldn't be anything marked grey unless we're on the active thread.
         MOZ_ASSERT(CurrentThreadCanAccessRuntime(thing->runtimeFromAnyThread()));
         if (!RuntimeFromActiveCooperatingThreadIsHeapMajorCollecting(shadowZone))
-            UnmarkGrayCellRecursively(thing, thing->getTraceKind());
+            JS::UnmarkGrayGCThingRecursively(JS::GCCellPtr(thing, thing->getTraceKind()));
     }
 }
 
 void
 AssertSafeToSkipBarrier(TenuredCell* thing);
 
 /* static */ MOZ_ALWAYS_INLINE void
 TenuredCell::writeBarrierPre(TenuredCell* thing)
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -232,17 +232,18 @@ js::CheckTracedThing(JSTracer* trc, T* t
                thing->getTraceKind());
 
     /*
      * Do not check IsMarkingTracer directly -- it should only be used in paths
      * where we cannot be the gray buffering tracer.
      */
     bool isGcMarkingTracer = trc->isMarkingTracer();
 
-    MOZ_ASSERT_IF(zone->requireGCTracer(), isGcMarkingTracer || IsBufferGrayRootsTracer(trc));
+    MOZ_ASSERT_IF(zone->requireGCTracer(),
+                  isGcMarkingTracer || IsBufferGrayRootsTracer(trc) || IsUnmarkGrayTracer(trc));
 
     if (isGcMarkingTracer) {
         GCMarker* gcMarker = GCMarker::fromTracer(trc);
         MOZ_ASSERT_IF(gcMarker->shouldCheckCompartments(),
                       zone->isCollecting() || zone->isAtomsZone());
 
         MOZ_ASSERT_IF(gcMarker->markColor() == MarkColor::Gray,
                       !zone->isGCMarkingBlack() || zone->isAtomsZone());
@@ -284,42 +285,47 @@ js::CheckTracedThing(JSTracer* trc, T th
 
 namespace js {
 #define IMPL_CHECK_TRACED_THING(_, type, __) \
     template void CheckTracedThing<type>(JSTracer*, type*);
 JS_FOR_EACH_TRACEKIND(IMPL_CHECK_TRACED_THING);
 #undef IMPL_CHECK_TRACED_THING
 } // namespace js
 
+static bool UnmarkGrayGCThing(JSRuntime* rt, JS::GCCellPtr thing);
+
 static bool
 ShouldTraceCrossCompartment(JSTracer* trc, JSObject* src, Cell* cell)
 {
     if (!trc->isMarkingTracer())
         return true;
 
     MarkColor color = GCMarker::fromTracer(trc)->markColor();
 
     if (!cell->isTenured()) {
         MOZ_ASSERT(color == MarkColor::Black);
         return false;
     }
     TenuredCell& tenured = cell->asTenured();
 
     JS::Zone* zone = tenured.zone();
+    if (!src->zone()->isGCMarking() && !zone->isGCMarking())
+        return false;
+
     if (color == MarkColor::Black) {
         /*
          * 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.
          */
         if (tenured.isMarkedGray()) {
             MOZ_ASSERT(!zone->isCollecting());
-            trc->runtime()->gc.setFoundBlackGrayEdges(tenured);
+            UnmarkGrayGCThing(trc->runtime(), JS::GCCellPtr(cell, cell->getTraceKind()));
         }
         return zone->isGCMarking();
     } else {
         if (zone->isGCMarkingBlack()) {
             /*
              * The destination compartment is being not being marked gray now,
              * but it will be later, so record the cell so it can be marked gray
              * at the appropriate time.
@@ -3343,16 +3349,20 @@ class UnmarkGrayTracer : public JS::Call
     // Whether we ran out of memory.
     bool oom;
 
   private:
     // Stack of cells to traverse.
     Vector<JS::GCCellPtr, 0, SystemAllocPolicy>& stack;
 
     void onChild(const JS::GCCellPtr& thing) override;
+
+#ifdef DEBUG
+    TracerKind getTracerKind() const override { return TracerKind::UnmarkGray; }
+#endif
 };
 
 void
 UnmarkGrayTracer::onChild(const JS::GCCellPtr& thing)
 {
     Cell* cell = thing.asCell();
 
     // Cells in the nursery cannot be gray, and therefore must necessarily point
@@ -3390,53 +3400,51 @@ UnmarkGrayTracer::unmark(JS::GCCellPtr c
          // If we run out of memory, we take a drastic measure: require that we
          // GC again before the next CC.
         stack.clear();
         runtime()->gc.setGrayBitsInvalid();
         return;
     }
 }
 
-template <typename T>
-static bool
-TypedUnmarkGrayCellRecursively(T* t)
+#ifdef DEBUG
+bool
+js::IsUnmarkGrayTracer(JSTracer* trc)
 {
-    MOZ_ASSERT(t);
-
-    JSRuntime* rt = t->runtimeFromActiveCooperatingThread();
-    MOZ_ASSERT(!JS::CurrentThreadIsHeapCollecting());
-    MOZ_ASSERT(!JS::CurrentThreadIsHeapCycleCollecting());
+    return trc->isCallbackTracer() &&
+           trc->asCallbackTracer()->getTracerKind() == JS::CallbackTracer::TracerKind::UnmarkGray;
+}
+#endif
+
+static bool
+UnmarkGrayGCThing(JSRuntime* rt, JS::GCCellPtr thing)
+{
+    MOZ_ASSERT(thing);
 
     UnmarkGrayTracer unmarker(rt);
-    gcstats::AutoPhase outerPhase(rt->gc.stats(), gcstats::PhaseKind::BARRIER);
     gcstats::AutoPhase innerPhase(rt->gc.stats(), gcstats::PhaseKind::UNMARK_GRAY);
-    unmarker.unmark(JS::GCCellPtr(t, MapTypeToTraceKind<T>::kind));
+    unmarker.unmark(thing);
     return unmarker.unmarkedAny;
 }
 
-struct UnmarkGrayCellRecursivelyFunctor {
-    template <typename T> bool operator()(T* t) { return TypedUnmarkGrayCellRecursively(t); }
-};
-
-bool
-js::UnmarkGrayCellRecursively(Cell* cell, JS::TraceKind kind)
+JS_FRIEND_API(bool)
+JS::UnmarkGrayGCThingRecursively(JS::GCCellPtr thing)
 {
-    return DispatchTraceKindTyped(UnmarkGrayCellRecursivelyFunctor(), cell, kind);
+    MOZ_ASSERT(!JS::CurrentThreadIsHeapCollecting());
+    MOZ_ASSERT(!JS::CurrentThreadIsHeapCycleCollecting());
+
+    JSRuntime* rt = thing.asCell()->runtimeFromActiveCooperatingThread();
+    gcstats::AutoPhase outerPhase(rt->gc.stats(), gcstats::PhaseKind::BARRIER);
+    return UnmarkGrayGCThing(rt, thing);
 }
 
 bool
 js::UnmarkGrayShapeRecursively(Shape* shape)
 {
-    return TypedUnmarkGrayCellRecursively(shape);
-}
-
-JS_FRIEND_API(bool)
-JS::UnmarkGrayGCThingRecursively(JS::GCCellPtr thing)
-{
-    return js::UnmarkGrayCellRecursively(thing.asCell(), thing.kind());
+    return JS::UnmarkGrayGCThingRecursively(JS::GCCellPtr(shape));
 }
 
 namespace js {
 namespace debug {
 
 MarkInfo
 GetMarkInfo(Cell* rawCell)
 {
--- a/js/src/gc/Marking.h
+++ b/js/src/gc/Marking.h
@@ -392,16 +392,19 @@ class GCMarker : public JSTracer
 #endif // DEBUG
 };
 
 #ifdef DEBUG
 // Return true if this trace is happening on behalf of gray buffering during
 // the marking phase of incremental GC.
 bool
 IsBufferGrayRootsTracer(JSTracer* trc);
+
+bool
+IsUnmarkGrayTracer(JSTracer* trc);
 #endif
 
 namespace gc {
 
 /*** Special Cases ***/
 
 void
 PushArena(GCMarker* gcmarker, Arena* arena);
--- a/js/src/gc/Statistics.cpp
+++ b/js/src/gc/Statistics.cpp
@@ -158,31 +158,29 @@ Statistics::lookupChildPhase(PhaseKind p
 {
     if (phaseKind == PhaseKind::IMPLICIT_SUSPENSION)
         return Phase::IMPLICIT_SUSPENSION;
     if (phaseKind == PhaseKind::EXPLICIT_SUSPENSION)
         return Phase::EXPLICIT_SUSPENSION;
 
     MOZ_ASSERT(phaseKind < PhaseKind::LIMIT);
 
-    // Most phases only correspond to a single expanded phase so check for that
-    // first.
-    Phase phase = phaseKinds[phaseKind].firstPhase;
-    if (phases[phase].nextInPhase == Phase::NONE) {
-        MOZ_ASSERT(phases[phase].parent == currentPhase());
-        return phase;
+    // Search all expanded phases that correspond to the required
+    // phase to find the one whose parent is the current expanded phase.
+    Phase phase;
+    for (phase = phaseKinds[phaseKind].firstPhase;
+         phase != Phase::NONE;
+         phase = phases[phase].nextInPhase)
+    {
+        if (phases[phase].parent == currentPhase())
+            break;
     }
 
-    // Otherwise search all expanded phases that correspond to the required
-    // phase to find the one whose parent is the current expanded phase.
-    Phase parent = currentPhase();
-    while (phases[phase].parent != parent) {
-        phase = phases[phase].nextInPhase;
-        MOZ_ASSERT(phase != Phase::NONE);
-    }
+    MOZ_RELEASE_ASSERT(phase != Phase::NONE,
+                       "Requested child phase not found under current phase");
 
     return phase;
 }
 
 inline decltype(mozilla::MakeEnumeratedRange(Phase::FIRST, Phase::LIMIT))
 AllPhases()
 {
     return mozilla::MakeEnumeratedRange(Phase::FIRST, Phase::LIMIT);
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -6853,48 +6853,16 @@ class AutoScheduleZonesForGC
     }
 
     ~AutoScheduleZonesForGC() {
         for (ZonesIter zone(rt_, WithAtoms); !zone.done(); zone.next())
             zone->unscheduleGC();
     }
 };
 
-/*
- * An invariant of our GC/CC interaction is that there must not ever be any
- * black to gray edges in the system. It is possible to violate this with
- * simple compartmental GC. For example, in GC[n], we collect in both
- * compartmentA and compartmentB, and mark both sides of the cross-compartment
- * edge gray. Later in GC[n+1], we only collect compartmentA, but this time
- * mark it black. Now we are violating the invariants and must fix it somehow.
- *
- * To prevent this situation, we explicitly detect the black->gray state when
- * marking cross-compartment edges -- see ShouldMarkCrossCompartment -- adding
- * each violating edges to foundBlackGrayEdges. After we leave the trace
- * session for each GC slice, we "ExposeToActiveJS" on each of these edges
- * (which we cannot do safely from the guts of the GC).
- */
-class AutoExposeLiveCrossZoneEdges
-{
-    BlackGrayEdgeVector* edges;
-
-  public:
-    explicit AutoExposeLiveCrossZoneEdges(BlackGrayEdgeVector* edgesPtr) : edges(edgesPtr) {
-        MOZ_ASSERT(edges->empty());
-    }
-    ~AutoExposeLiveCrossZoneEdges() {
-        for (auto& target : *edges) {
-            MOZ_ASSERT(target);
-            MOZ_ASSERT(!target->zone()->isCollecting());
-            UnmarkGrayCellRecursively(target, target->getTraceKind());
-        }
-        edges->clear();
-    }
-};
-
 } /* anonymous namespace */
 
 /*
  * Run one GC "cycle" (either a slice of incremental GC or an entire
  * non-incremental GC. We disable inlining to ensure that the bottom of the
  * stack with possible GC roots recorded in MarkRuntime excludes any pointers we
  * use during the marking implementation.
  *
@@ -6904,18 +6872,16 @@ class AutoExposeLiveCrossZoneEdges
 MOZ_NEVER_INLINE GCRuntime::IncrementalResult
 GCRuntime::gcCycle(bool nonincrementalByAPI, SliceBudget& budget, JS::gcreason::Reason reason)
 {
     // Note that the following is allowed to re-enter GC in the finalizer.
     AutoNotifyGCActivity notify(*this);
 
     gcstats::AutoGCSlice agc(stats(), scanZonesBeforeGC(), invocationKind, budget, reason);
 
-    AutoExposeLiveCrossZoneEdges aelcze(&foundBlackGrayEdges.ref());
-
     minorGC(reason, gcstats::PhaseKind::EVICT_NURSERY_FOR_MAJOR_GC);
 
     AutoTraceSession session(rt, JS::HeapState::MajorCollecting);
 
     majorGCTriggerReason = JS::gcreason::NO_REASON;
     interFrameGC = true;
 
     number++;