Bug 1516409 - Separate out flags for gray and black delayed marking r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 02 Jan 2019 18:19:01 +0000
changeset 509406 8d20164aedbf0b55d8244bfeafeb732957447d55
parent 509403 c58927f9b3c77b4485089951caed0ce1e53c7c85
child 509407 c60f868017bcb1de9a3daa620cccd14b7fe9a728
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1516409
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 1516409 - Separate out flags for gray and black delayed marking r=sfink
js/src/gc/GCMarker.h
js/src/gc/Heap-inl.h
js/src/gc/Heap.h
js/src/gc/Marking.cpp
js/src/gc/Statistics.cpp
--- a/js/src/gc/GCMarker.h
+++ b/js/src/gc/GCMarker.h
@@ -288,23 +288,17 @@ class GCMarker : public JSTracer {
 
   void enterWeakMarkingMode();
   void leaveWeakMarkingMode();
   void abortLinearWeakMarking() {
     leaveWeakMarkingMode();
     linearWeakMarkingDisabled_ = true;
   }
 
-  void delayMarkingArena(gc::Arena* arena);
   void delayMarkingChildren(gc::Cell* cell);
-  void markDelayedChildren(gc::Arena* arena, gc::MarkColor color);
-  MOZ_MUST_USE bool markAllDelayedChildren(SliceBudget& budget);
-  bool processDelayedMarkingList(gc::MarkColor color, bool shouldYield,
-                                 SliceBudget& budget);
-  bool hasDelayedChildren() const { return !!delayedMarkingList; }
 
   bool isDrained() { return isMarkStackEmpty() && !delayedMarkingList; }
 
   MOZ_MUST_USE bool markUntilBudgetExhausted(SliceBudget& budget);
 
   void setGCMode(JSGCMode mode) { stack.setGCMode(mode); }
 
   size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
@@ -379,16 +373,21 @@ class GCMarker : public JSTracer {
       const gc::MarkStack::SavedValueArray& savedArray);
 
   void saveValueRanges();
   gc::MarkStack::SavedValueArray saveValueRange(
       const gc::MarkStack::ValueArray& array);
 
   inline void processMarkStackTop(SliceBudget& budget);
 
+  void markDelayedChildren(gc::Arena* arena, gc::MarkColor color);
+  MOZ_MUST_USE bool markAllDelayedChildren(SliceBudget& budget);
+  bool processDelayedMarkingList(gc::MarkColor color, SliceBudget& budget);
+  bool hasDelayedChildren() const { return !!delayedMarkingList; }
+  void rebuildDelayedMarkingList();
   void appendToDelayedMarkingList(gc::Arena** listTail, gc::Arena* arena);
 
   template <typename F>
   void forEachDelayedMarkingArena(F&& f);
 
   /* The mark stack. Pointers in this stack are "gray" in the GC sense. */
   gc::MarkStack stack;
 
--- a/js/src/gc/Heap-inl.h
+++ b/js/src/gc/Heap-inl.h
@@ -13,25 +13,27 @@
 #include "gc/Zone.h"
 
 inline void js::gc::Arena::init(JS::Zone* zoneArg, AllocKind kind,
                                 const AutoLockGC& lock) {
   MOZ_ASSERT(firstFreeSpan.isEmpty());
   MOZ_ASSERT(!zone);
   MOZ_ASSERT(!allocated());
   MOZ_ASSERT(!onDelayedMarkingList_);
-  MOZ_ASSERT(!hasDelayedMarking_);
+  MOZ_ASSERT(!hasDelayedBlackMarking_);
+  MOZ_ASSERT(!hasDelayedGrayMarking_);
   MOZ_ASSERT(!nextDelayedMarkingArena_);
 
   MOZ_MAKE_MEM_UNDEFINED(this, ArenaSize);
 
   zone = zoneArg;
   allocKind = size_t(kind);
   onDelayedMarkingList_ = 0;
-  hasDelayedMarking_ = 0;
+  hasDelayedBlackMarking_ = 0;
+  hasDelayedGrayMarking_ = 0;
   nextDelayedMarkingArena_ = 0;
   if (zone->isAtomsZone()) {
     zone->runtimeFromAnyThread()->gc.atomMarking.registerArena(this, lock);
   } else {
     bufferedCells() = &ArenaCellSet::Empty;
   }
 
   setAsFullyUnused();
--- a/js/src/gc/Heap.h
+++ b/js/src/gc/Heap.h
@@ -231,21 +231,25 @@ class Arena {
   size_t allocKind : 8;
 
  private:
   /*
    * When recursive marking uses too much stack we delay marking of
    * arenas and link them into a list for later processing. This
    * uses the following fields.
    */
+  static const size_t DELAYED_MARKING_FLAG_BITS = 3;
+  static const size_t DELAYED_MARKING_ARENA_BITS =
+    JS_BITS_PER_WORD - 8 - DELAYED_MARKING_FLAG_BITS;
   size_t onDelayedMarkingList_ : 1;
-  size_t hasDelayedMarking_ : 1;
-  size_t nextDelayedMarkingArena_ : JS_BITS_PER_WORD - 8 - 1 - 1;
+  size_t hasDelayedBlackMarking_ : 1;
+  size_t hasDelayedGrayMarking_ : 1;
+  size_t nextDelayedMarkingArena_ : DELAYED_MARKING_ARENA_BITS;
   static_assert(
-      ArenaShift >= 8 + 1 + 1,
+      DELAYED_MARKING_ARENA_BITS >= JS_BITS_PER_WORD - ArenaShift,
       "Arena::nextDelayedMarkingArena_ packing assumes that ArenaShift has "
       "enough bits to cover allocKind and delayed marking state.");
 
   union {
     /*
      * For arenas in zones other than the atoms zone, if non-null, points
      * to an ArenaCellSet that represents the set of cells in this arena
      * that are in the nursery's store buffer.
@@ -284,17 +288,18 @@ class Arena {
 
   // Initialize an arena to its unallocated state. For arenas that were
   // previously allocated for some zone, use release() instead.
   void setAsNotAllocated() {
     firstFreeSpan.initAsEmpty();
     zone = nullptr;
     allocKind = size_t(AllocKind::LIMIT);
     onDelayedMarkingList_ = 0;
-    hasDelayedMarking_ = 0;
+    hasDelayedBlackMarking_ = 0;
+    hasDelayedGrayMarking_ = 0;
     nextDelayedMarkingArena_ = 0;
     bufferedCells_ = nullptr;
   }
 
   // Return an allocated arena to its unallocated state.
   inline void release(const AutoLockGC& lock);
 
   uintptr_t address() const {
@@ -394,45 +399,56 @@ class Arena {
   Arena* getNextDelayedMarking() const {
     MOZ_ASSERT(onDelayedMarkingList_);
     return reinterpret_cast<Arena*>(nextDelayedMarkingArena_ << ArenaShift);
   }
 
   void setNextDelayedMarkingArena(Arena* arena) {
     MOZ_ASSERT(!(uintptr_t(arena) & ArenaMask));
     MOZ_ASSERT(!onDelayedMarkingList_);
-    MOZ_ASSERT(!hasDelayedMarking_);
+    MOZ_ASSERT(!hasDelayedBlackMarking_);
+    MOZ_ASSERT(!hasDelayedGrayMarking_);
     MOZ_ASSERT(!nextDelayedMarkingArena_);
     onDelayedMarkingList_ = 1;
-    hasDelayedMarking_ = 1;
     if (arena) {
       nextDelayedMarkingArena_ = arena->address() >> ArenaShift;
     }
   }
 
   void updateNextDelayedMarkingArena(Arena* arena) {
     MOZ_ASSERT(!(uintptr_t(arena) & ArenaMask));
     MOZ_ASSERT(onDelayedMarkingList_);
     nextDelayedMarkingArena_ = arena ? arena->address() >> ArenaShift : 0;
   }
 
-  bool hasDelayedMarking() const {
+  bool hasDelayedMarking(MarkColor color) const {
     MOZ_ASSERT(onDelayedMarkingList_);
-    return hasDelayedMarking_;
+    return color == MarkColor::Black ? hasDelayedBlackMarking_
+                                     : hasDelayedGrayMarking_;
   }
 
-  void setHasDelayedMarking(bool value) {
+  bool hasAnyDelayedMarking() const {
     MOZ_ASSERT(onDelayedMarkingList_);
-    hasDelayedMarking_ = value;
+    return hasDelayedBlackMarking_ || hasDelayedGrayMarking_;
+  }
+
+  void setHasDelayedMarking(MarkColor color, bool value) {
+    MOZ_ASSERT(onDelayedMarkingList_);
+    if (color == MarkColor::Black) {
+      hasDelayedBlackMarking_ = value;
+    } else {
+      hasDelayedGrayMarking_ = value;
+    }
   }
 
   void clearDelayedMarkingState() {
     MOZ_ASSERT(onDelayedMarkingList_);
     onDelayedMarkingList_ = 0;
-    hasDelayedMarking_ = 0;
+    hasDelayedBlackMarking_ = 0;
+    hasDelayedGrayMarking_ = 0;
     nextDelayedMarkingArena_ = 0;
   }
 
   inline ArenaCellSet*& bufferedCells();
   inline size_t& atomBitmapStart();
 
   template <typename T>
   size_t finalize(FreeOp* fop, AllocKind thingKind, size_t thingSize);
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -945,29 +945,33 @@ struct TypeParticipatesInCC {};
 #define EXPAND_PARTICIPATES_IN_CC(_, type, addToCCKind) \
   template <>                                           \
   struct TypeParticipatesInCC<type> {                   \
     static const bool value = addToCCKind;              \
   };
 JS_FOR_EACH_TRACEKIND(EXPAND_PARTICIPATES_IN_CC)
 #undef EXPAND_PARTICIPATES_IN_CC
 
+}  // namespace
+
+#ifdef DEBUG
+
 struct ParticipatesInCCFunctor {
   template <typename T>
   bool operator()() {
     return TypeParticipatesInCC<T>::value;
   }
 };
 
-}  // namespace
-
 static bool TraceKindParticipatesInCC(JS::TraceKind kind) {
   return DispatchTraceKindTyped(ParticipatesInCCFunctor(), kind);
 }
 
+#endif // DEBUG
+
 template <typename T>
 bool js::GCMarker::mark(T* thing) {
   if (IsInsideNursery(thing)) {
     return false;
   }
   AssertShouldMarkInZone(thing);
   TenuredCell* cell = TenuredCell::fromPointer(thing);
 
@@ -2538,82 +2542,69 @@ void GCMarker::leaveWeakMarkingMode() {
   for (GCZonesIter zone(runtime()); !zone.done(); zone.next()) {
     if (!zone->gcWeakKeys().clear()) {
       oomUnsafe.crash("clearing weak keys in GCMarker::leaveWeakMarkingMode()");
     }
   }
 }
 
 void GCMarker::delayMarkingChildren(Cell* cell) {
-  delayMarkingArena(cell->asTenured().arena());
-}
-
-void GCMarker::delayMarkingArena(Arena* arena) {
-  if (arena->onDelayedMarkingList()) {
-    // The arena is already on the delayed marking list, so just set a flag to
-    // ensure it gets processed again.
-    if (!arena->hasDelayedMarking()) {
-      arena->setHasDelayedMarking(true);
-      delayedMarkingWorkAdded = true;
-    }
-    return;
+  Arena* arena = cell->asTenured().arena();
+  if (!arena->onDelayedMarkingList()) {
+    arena->setNextDelayedMarkingArena(delayedMarkingList);
+    delayedMarkingList = arena;
+#ifdef DEBUG
+    markLaterArenas++;
+#endif
   }
-  arena->setNextDelayedMarkingArena(delayedMarkingList);
-  delayedMarkingList = arena;
-  delayedMarkingWorkAdded = true;
-#ifdef DEBUG
-  markLaterArenas++;
-#endif
+  if (!arena->hasDelayedMarking(color)) {
+    arena->setHasDelayedMarking(color, true);
+    delayedMarkingWorkAdded = true;
+  }
 }
 
 void GCMarker::markDelayedChildren(Arena* arena, MarkColor color) {
   JS::TraceKind kind = MapAllocToTraceKind(arena->getAllocKind());
   MOZ_ASSERT_IF(color == MarkColor::Gray, TraceKindParticipatesInCC(kind));
 
+  AutoSetMarkColor setColor(*this, color);
   for (ArenaCellIterUnderGC i(arena); !i.done(); i.next()) {
     TenuredCell* t = i.getCell();
     if ((color == MarkColor::Gray && t->isMarkedGray()) ||
         (color == MarkColor::Black && t->isMarkedBlack())) {
       js::TraceChildren(this, t, kind);
     }
   }
 }
 
-static inline bool ArenaCanHaveGrayThings(Arena* arena) {
-  JS::TraceKind kind = MapAllocToTraceKind(arena->getAllocKind());
-  return TraceKindParticipatesInCC(kind);
-}
-
 /*
  * Process arenas from |delayedMarkingList| by marking the unmarked children of
- * marked cells of color |color|. If |shouldYield|, return early if the |budget|
- * is exceeded.
+ * marked cells of color |color|. Return early if the |budget| is exceeded.
  *
  * This is called twice, first to mark gray children and then to mark black
  * children.
  */
-bool GCMarker::processDelayedMarkingList(MarkColor color, bool shouldYield,
-                                         SliceBudget& budget) {
+bool GCMarker::processDelayedMarkingList(MarkColor color, SliceBudget& budget) {
   // Marking delayed children may add more arenas to the list, including arenas
-  // we are currently or have previously processed. Handle this by setting a
-  // flag on arenas we think we've processed which is cleared if they are
-  // re-added. Iterate the list until the flag is set on all arenas.
+  // we are currently processing or have previously processed. Handle this by
+  // clearing a flag on each arena before marking its children. This flag will
+  // be set again if the arena is re-added. Iterate the list until no new arenas
+  // were added.
 
   do {
     delayedMarkingWorkAdded = false;
     for (Arena* arena = delayedMarkingList; arena;
          arena = arena->getNextDelayedMarking()) {
-      if (!arena->hasDelayedMarking() ||
-          (color == MarkColor::Gray && !ArenaCanHaveGrayThings(arena))) {
+      if (!arena->hasDelayedMarking(color)) {
         continue;
       }
-      arena->setHasDelayedMarking(false);
+      arena->setHasDelayedMarking(color, false);
       markDelayedChildren(arena, color);
       budget.step(150);
-      if (shouldYield && budget.isOverBudget()) {
+      if (budget.isOverBudget()) {
         return false;
       }
     }
   } while (delayedMarkingWorkAdded);
 
   return true;
 }
 
@@ -2626,64 +2617,53 @@ bool GCMarker::markAllDelayedChildren(Sl
                         gcstats::PhaseKind::MARK_DELAYED);
 
   // We have a list of arenas containing marked cells with unmarked children
   // where we ran out of stack space during marking.
   //
   // Both black and gray cells in these arenas may have unmarked children, and
   // we must mark gray children first as gray entries always sit before black
   // entries on the mark stack. Therefore the list is processed in two stages.
-  //
-  // In order to guarantee progress here, the fist pass (gray marking) is done
-  // non-incrementally. We can't remove anything from the list until the second
-  // pass so if we yield during the first pass we will have to restart and
-  // process all the arenas over again. If there are enough arenas we may never
-  // finish during our timeslice. Disallowing yield during the first pass
-  // ensures that the list will at least shrink by one arena every time.
 
   MOZ_ASSERT(delayedMarkingList);
 
   bool finished;
-  finished = processDelayedMarkingList(MarkColor::Gray, false, /* don't yield */
-                                       budget);
-  MOZ_ASSERT(finished);
-
-  forEachDelayedMarkingArena([&](Arena* arena) {
-    MOZ_ASSERT(!arena->hasDelayedMarking());
-    arena->setHasDelayedMarking(true);
-  });
-
-  finished = processDelayedMarkingList(MarkColor::Black,
-                                       true, /* yield if over budget */
-                                       budget);
-
-  // Rebuild the list, removing processed arenas.
+  finished = processDelayedMarkingList(MarkColor::Gray, budget);
+  rebuildDelayedMarkingList();
+  if (!finished) {
+    return false;
+  }
+
+  finished = processDelayedMarkingList(MarkColor::Black, budget);
+  rebuildDelayedMarkingList();
+
+  MOZ_ASSERT_IF(finished, !delayedMarkingList);
+  MOZ_ASSERT_IF(finished, !markLaterArenas);
+
+  return finished;
+}
+
+void GCMarker::rebuildDelayedMarkingList() {
+  // Rebuild the delayed marking list, removing arenas which do not need further
+  // marking.
+
   Arena* listTail = nullptr;
   forEachDelayedMarkingArena([&](Arena* arena) {
-    if (!arena->hasDelayedMarking()) {
+    if (!arena->hasAnyDelayedMarking()) {
       arena->clearDelayedMarkingState();
 #ifdef DEBUG
       MOZ_ASSERT(markLaterArenas);
       markLaterArenas--;
 #endif
       return;
     }
 
     appendToDelayedMarkingList(&listTail, arena);
   });
   appendToDelayedMarkingList(&listTail, nullptr);
-
-  if (!finished) {
-    return false;
-  }
-
-  MOZ_ASSERT(!delayedMarkingList);
-  MOZ_ASSERT(!markLaterArenas);
-
-  return true;
 }
 
 inline void GCMarker::appendToDelayedMarkingList(Arena** listTail,
                                                  Arena* arena) {
   if (*listTail) {
     (*listTail)->updateNextDelayedMarkingArena(arena);
   } else {
     delayedMarkingList = arena;
--- a/js/src/gc/Statistics.cpp
+++ b/js/src/gc/Statistics.cpp
@@ -175,18 +175,20 @@ Phase Statistics::lookupChildPhase(Phase
   Phase phase;
   for (phase = phaseKinds[phaseKind].firstPhase; phase != Phase::NONE;
        phase = phases[phase].nextWithPhaseKind) {
     if (phases[phase].parent == currentPhase()) {
       break;
     }
   }
 
-  MOZ_RELEASE_ASSERT(phase != Phase::NONE,
-                     "Requested child phase not found under current phase");
+  if (phase == Phase::NONE) {
+      MOZ_CRASH_UNSAFE_PRINTF("Child phase kind %u not found under current phase kind %u",
+                              unsigned(phaseKind), unsigned(currentPhaseKind()));
+  }
 
   return phase;
 }
 
 inline decltype(mozilla::MakeEnumeratedRange(Phase::FIRST, Phase::LIMIT))
 AllPhases() {
   return mozilla::MakeEnumeratedRange(Phase::FIRST, Phase::LIMIT);
 }