Backed out 3 changesets (bug 1597206) as requested by sfink

Backed out changeset 334b41001234 (bug 1597206)
Backed out changeset e372ae10adb5 (bug 1597206)
Backed out changeset c82b377fbbde (bug 1597206)

js/public/TracingAPI.h

@@ -69,6 +69,12 @@ class JS_PUBLIC_API JSTracer {
     // everything reachable by regular edges has been marked.
     Marking,
 
+    // Same as Marking, except we have now moved on to the "weak marking
+    // phase", in which every marked obj/script is immediately looked up to
+    // see if it is a weak map key (and therefore might require marking its
+    // weak map value).
+    WeakMarking,
+
     // A tracer that traverses the graph for the purposes of moving objects
     // from the nursery to the tenured area.
     Tenuring,
@@ -77,7 +83,12 @@ class JS_PUBLIC_API JSTracer {
     // Traversing children is the responsibility of the callback.
     Callback
   };
-  bool isMarkingTracer() const { return tag_ == TracerKindTag::Marking; }
+  bool isMarkingTracer() const {
+    return tag_ == TracerKindTag::Marking || tag_ == TracerKindTag::WeakMarking;
+  }
+  bool isWeakMarkingTracer() const {
+    return tag_ == TracerKindTag::WeakMarking;
+  }
   bool isTenuringTracer() const { return tag_ == TracerKindTag::Tenuring; }
   bool isCallbackTracer() const { return tag_ == TracerKindTag::Callback; }
   inline JS::CallbackTracer* asCallbackTracer();

js/src/gc/GC.cpp

@@ -4171,7 +4171,7 @@ void GCRuntime::markWeakReferences(gcstats::PhaseKind phase) {
 
   for (;;) {
     bool markedAny = false;
-    if (!marker.isWeakMarking()) {
+    if (!marker.isWeakMarkingTracer()) {
       for (ZoneIterT zone(this); !zone.done(); zone.next()) {
         markedAny |= WeakMapBase::markZoneIteratively(zone, &marker);
       }

js/src/gc/GCMarker.h

@@ -235,26 +235,6 @@ class MarkStackIter {
 
 } /* namespace gc */
 
-enum MarkingState : uint8_t {
-  // Have not yet started marking.
-  NotActive,
-
-  // Main marking mode. Weakmap marking will be populating the weakKeys tables
-  // but not consulting them. The state will transition to WeakMarking until it
-  // is done, then back to RegularMarking.
-  RegularMarking,
-
-  // Same as RegularMarking except now every marked obj/script is immediately
-  // looked up in the weakKeys table to see if it is a weakmap key, and
-  // therefore might require marking its value. Transitions back to
-  // RegularMarking when done.
-  WeakMarking,
-
-  // Same as RegularMarking, but we OOMed (or obeyed a directive in the test
-  // marking queue) and fell back to iterating until the next GC.
-  IterativeMarking
-};
-
 class GCMarker : public JSTracer {
  public:
   explicit GCMarker(JSRuntime* rt);
@@ -306,15 +286,9 @@ class GCMarker : public JSTracer {
 
   void enterWeakMarkingMode();
   void leaveWeakMarkingMode();
-
-  // Do not use linear-time weak marking for the rest of this collection.
-  // Currently, this will only be triggered by an OOM when updating needed data
-  // structures.
   void abortLinearWeakMarking() {
-    if (state == MarkingState::WeakMarking) {
-      leaveWeakMarkingMode();
-    }
-    state = MarkingState::IterativeMarking;
+    leaveWeakMarkingMode();
+    linearWeakMarkingDisabled_ = true;
   }
 
   void delayMarkingChildren(gc::Cell* cell);
@@ -356,8 +330,6 @@ class GCMarker : public JSTracer {
   template <typename T>
   void markImplicitEdges(T* oldThing);
 
-  bool isWeakMarking() const { return state == MarkingState::WeakMarking; }
-
  private:
 #ifdef DEBUG
   void checkZone(void* p);
@@ -464,12 +436,15 @@ class GCMarker : public JSTracer {
   /* Whether more work has been added to the delayed marking list. */
   MainThreadOrGCTaskData<bool> delayedMarkingWorkAdded;
 
+  /*
+   * If the weakKeys table OOMs, disable the linear algorithm and fall back
+   * to iterating until the next GC.
+   */
+  MainThreadOrGCTaskData<bool> linearWeakMarkingDisabled_;
+
   /* The count of marked objects during GC. */
   size_t markCount;
 
-  /* Track the state of marking. */
-  MainThreadOrGCTaskData<MarkingState> state;
-
 #ifdef DEBUG
   /* Count of arenas that are currently in the stack. */
   MainThreadOrGCTaskData<size_t> markLaterArenas;

js/src/gc/Marking.cpp

@@ -653,7 +653,7 @@ void GCMarker::markEphemeronValues(gc::Cell* markedCell,
 
 template <typename T>
 void GCMarker::markImplicitEdgesHelper(T markedThing) {
-  if (!isWeakMarking()) {
+  if (!isWeakMarkingTracer()) {
     return;
   }
 
@@ -1593,7 +1593,7 @@ GCMarker::MarkQueueProgress GCMarker::processMarkQueue() {
         return QueueYielded;
       } else if (js::StringEqualsLiteral(str, "enter-weak-marking-mode") ||
                  js::StringEqualsLiteral(str, "abort-weak-marking-mode")) {
-        if (state == MarkingState::RegularMarking) {
+        if (!isWeakMarkingTracer() && !linearWeakMarkingDisabled_) {
           // We can't enter weak marking mode at just any time, so instead
           // we'll stop processing the queue and continue on with the GC. Once
           // we enter weak marking mode, we can continue to the rest of the
@@ -2430,11 +2430,11 @@ GCMarker::GCMarker(JSRuntime* rt)
       color(MarkColor::Black),
       mainStackColor(MarkColor::Black),
       delayedMarkingList(nullptr),
-      delayedMarkingWorkAdded(false),
-      state(MarkingState::NotActive)
+      delayedMarkingWorkAdded(false)
 #ifdef DEBUG
       ,
       markLaterArenas(0),
+      started(false),
       strictCompartmentChecking(false),
       markQueue(rt),
       queuePos(0)
@@ -2448,9 +2448,12 @@ bool GCMarker::init(JSGCMode gcMode) {
 }
 
 void GCMarker::start() {
-  MOZ_ASSERT(state == MarkingState::NotActive);
-  state = MarkingState::RegularMarking;
+#ifdef DEBUG
+  MOZ_ASSERT(!started);
+  started = true;
+#endif
   color = MarkColor::Black;
+  linearWeakMarkingDisabled_ = false;
 
 #ifdef DEBUG
   queuePos = 0;
@@ -2462,11 +2465,15 @@ void GCMarker::start() {
 }
 
 void GCMarker::stop() {
+#ifdef DEBUG
   MOZ_ASSERT(isDrained());
+
+  MOZ_ASSERT(started);
+  started = false;
+
   MOZ_ASSERT(!delayedMarkingList);
   MOZ_ASSERT(markLaterArenas == 0);
-  MOZ_ASSERT(state != MarkingState::NotActive);
-  state = MarkingState::NotActive;
+#endif
 
   stack.clear();
   auxStack.clear();
@@ -2575,43 +2582,42 @@ void GCMarker::repush(JSObject* obj) {
 void GCMarker::enterWeakMarkingMode() {
   MOZ_ASSERT(runtime()->gc.nursery().isEmpty());
 
-  MOZ_ASSERT(isMarkingTracer());
-  if (state != MarkingState::RegularMarking) {
+  MOZ_ASSERT(tag_ == TracerKindTag::Marking);
+  if (linearWeakMarkingDisabled_) {
     return;
   }
 
-  if (weakMapAction() != ExpandWeakMaps) {
-    return;  // This marker does not do linear-time weak marking.
-  }
-
   // During weak marking mode, we maintain a table mapping weak keys to
   // entries in known-live weakmaps. Initialize it with the keys of marked
   // weakmaps -- or more precisely, the keys of marked weakmaps that are
   // mapped to not yet live values. (Once bug 1167452 implements incremental
   // weakmap marking, this initialization step will become unnecessary, as
   // the table will already hold all such keys.)
+  if (weakMapAction() == ExpandWeakMaps) {
+    tag_ = TracerKindTag::WeakMarking;
 
-  // Set state before doing anything else, so any new key that is marked
-  // during the following gcWeakKeys scan will itself be looked up in
-  // gcWeakKeys and marked according to ephemeron rules.
-  state = MarkingState::WeakMarking;
+    // If there was an 'enter-weak-marking-mode' token in the queue, then it
+    // and everything after it will still be in the queue so we can process
+    // them now.
+    while (processMarkQueue() == QueueYielded) {
+    };
 
-  // If there was an 'enter-weak-marking-mode' token in the queue, then it
-  // and everything after it will still be in the queue so we can process
-  // them now.
-  while (processMarkQueue() == QueueYielded) {
-  };
-
-  for (SweepGroupZonesIter zone(runtime()); !zone.done(); zone.next()) {
-    for (WeakMapBase* m : zone->gcWeakMapList()) {
-      if (m->mapColor) {
-        mozilla::Unused << m->markEntries(this);
+    for (SweepGroupZonesIter zone(runtime()); !zone.done(); zone.next()) {
+      for (WeakMapBase* m : zone->gcWeakMapList()) {
+        if (m->mapColor) {
+          mozilla::Unused << m->markEntries(this);
+        }
       }
     }
   }
 }
 
 void GCMarker::leaveWeakMarkingMode() {
+  MOZ_ASSERT_IF(
+      weakMapAction() == ExpandWeakMaps && !linearWeakMarkingDisabled_,
+      tag_ == TracerKindTag::WeakMarking);
+  tag_ = TracerKindTag::Marking;
+
   // Table is expensive to maintain when not in weak marking mode, so we'll
   // rebuild it upon entry rather than allow it to contain stale data.
   AutoEnterOOMUnsafeRegion oomUnsafe;
@@ -2620,14 +2626,6 @@ void GCMarker::leaveWeakMarkingMode() {
       oomUnsafe.crash("clearing weak keys in GCMarker::leaveWeakMarkingMode()");
     }
   }
-
-  MOZ_ASSERT_IF(weakMapAction() == ExpandWeakMaps,
-                state == MarkingState::WeakMarking ||
-                    state == MarkingState::IterativeMarking);
-
-  if (state != MarkingState::IterativeMarking) {
-    state = MarkingState::RegularMarking;
-  }
 }
 
 void GCMarker::delayMarkingChildren(Cell* cell) {
@@ -2779,7 +2777,7 @@ void gc::PushArena(GCMarker* gcmarker, Arena* arena) {
 
 #ifdef DEBUG
 void GCMarker::checkZone(void* p) {
-  MOZ_ASSERT(state != MarkingState::NotActive);
+  MOZ_ASSERT(started);
   DebugOnly<Cell*> cell = static_cast<Cell*>(p);
   MOZ_ASSERT_IF(cell->isTenured(),
                 cell->asTenured().zone()->isCollectingFromAnyThread());

js/src/gc/WeakMap-inl.h

@@ -225,11 +225,6 @@ bool WeakMap<K, V>::markEntries(GCMarker* marker) {
     if (markEntry(marker, e.front().mutableKey(), e.front().value())) {
       markedAny = true;
     }
-    if (marker->isWeakMarking()) {
-      // No need to populate the weak key table yet; it will be built from
-      // scratch during enterWeakMarkingMode.
-      continue;
-    }
 
     JSRuntime* rt = zone()->runtimeFromAnyThread();
     CellColor keyColor =