Bug 1676212 - Don't disable incremental barrier during marking since we don't trigger these any more r=sfink

Now we have assertions that we don't trigger barriers when marking, we don't
need to disable them. We still need to disable them for sweeping as this does
trigger barriers.

Differential Revision: https://phabricator.services.mozilla.com/D96438

js/public/shadow/Zone.h

@@ -25,6 +25,7 @@ namespace shadow {
 struct Zone {
   enum GCState : uint8_t {
     NoGC,
+    Prepare,
     MarkBlackOnly,
     MarkBlackAndGray,
     Sweep,
@@ -64,6 +65,7 @@ struct Zone {
 
   GCState gcState() const { return gcState_; }
   bool wasGCStarted() const { return gcState_ != NoGC; }
+  bool isGCPreparing() const { return gcState_ == Prepare; }
   bool isGCMarkingBlackOnly() const { return gcState_ == MarkBlackOnly; }
   bool isGCMarkingBlackAndGray() const { return gcState_ == MarkBlackAndGray; }
   bool isGCSweeping() const { return gcState_ == Sweep; }

js/src/gc/GC.cpp

@@ -3998,7 +3998,7 @@ bool GCRuntime::prepareZonesForCollection(JS::GCReason reason,
     if (shouldCollect) {
       MOZ_ASSERT(zone->canCollect());
       any = true;
-      zone->changeGCState(Zone::NoGC, Zone::MarkBlackOnly);
+      zone->changeGCState(Zone::NoGC, Zone::Prepare);
     } else if (zone->canCollect()) {
       *isFullOut = false;
     }
@@ -4040,14 +4040,14 @@ bool GCRuntime::prepareZonesForCollection(JS::GCReason reason,
    * purpose.
    */
   MOZ_ASSERT_IF(reason == JS::GCReason::DELAYED_ATOMS_GC,
-                atomsZone->isGCMarking());
+                atomsZone->isGCPreparing());
 
   /* Check that at least one zone is scheduled for collection. */
   return any;
 }
 
 void GCRuntime::discardJITCodeForGC() {
-  js::CancelOffThreadIonCompile(rt, JS::Zone::MarkBlackOnly);
+  js::CancelOffThreadIonCompile(rt, JS::Zone::Prepare);
   for (GCZonesIter zone(this); !zone.done(); zone.next()) {
     gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::MARK_DISCARD_CODE);
     zone->discardJitCode(rt->defaultFreeOp(), Zone::DiscardBaselineCode,
@@ -4224,7 +4224,14 @@ bool GCRuntime::beginMarkPhase(JS::GCReason reason, AutoGCSession& session) {
   /*
    * Mark phase.
    */
+
   gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::MARK);
+
+  for (GCZonesIter zone(this); !zone.done(); zone.next()) {
+    // Incremental marking barriers are enabled at this point.
+    zone->changeGCState(Zone::Prepare, Zone::MarkBlackOnly);
+  }
+
   if (rt->isBeingDestroyed()) {
     checkNoRuntimeRoots(session);
   } else {
@@ -4629,7 +4636,6 @@ void GCRuntime::getNextSweepGroup() {
     // Abort collection of subsequent sweep groups.
     for (SweepGroupZonesIter zone(this); !zone.done(); zone.next()) {
       MOZ_ASSERT(!zone->gcNextGraphComponent);
-      zone->setNeedsIncrementalBarrier(false);
       zone->changeGCState(Zone::MarkBlackOnly, Zone::NoGC);
       zone->arenas.unmarkPreMarkedFreeCells();
       zone->arenas.mergeNewArenasInMarkPhase();
@@ -6227,6 +6233,9 @@ IncrementalProgress GCRuntime::performSweepActions(SliceBudget& budget) {
   gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::SWEEP);
   JSFreeOp fop(rt);
 
+  // Don't trigger pre-barriers when finalizing.
+  AutoDisableBarriers disableBarriers(this);
+
   // Drain the mark stack, possibly in a parallel task if we're in a part of
   // sweeping that allows it.
   //
@@ -6574,7 +6583,6 @@ GCRuntime::IncrementalResult GCRuntime::resetIncrementalGC(
       }
 
       for (GCZonesIter zone(this); !zone.done(); zone.next()) {
-        zone->setNeedsIncrementalBarrier(false);
         zone->changeGCState(Zone::MarkBlackOnly, Zone::NoGC);
         zone->clearGCSliceThresholds();
         zone->arenas.unmarkPreMarkedFreeCells();
@@ -6629,31 +6637,12 @@ GCRuntime::IncrementalResult GCRuntime::resetIncrementalGC(
   return IncrementalResult::ResetIncremental;
 }
 
-namespace {
-
-/*
- * Manage barriers: disable during GC slices, and enable during an incremental
- * GC for all zones that are marking.
- */
-class AutoUpdateBarriers {
- public:
-  explicit AutoUpdateBarriers(GCRuntime* gc);
-  ~AutoUpdateBarriers();
-
- private:
-  GCRuntime* gc;
-};
-
-} /* anonymous namespace */
-
-AutoUpdateBarriers::AutoUpdateBarriers(GCRuntime* gc) : gc(gc) {
+AutoDisableBarriers::AutoDisableBarriers(GCRuntime* gc) : gc(gc) {
+  /*
+   * Clear needsIncrementalBarrier early so we don't do any write barriers
+   * during sweeping.
+   */
   for (GCZonesIter zone(gc); !zone.done(); zone.next()) {
-    /*
-     * Clear needsIncrementalBarrier early so we don't do any write
-     * barriers during GC. We don't need to update the Ion barriers (which
-     * is expensive) because Ion code doesn't run during GC. If need be,
-     * we'll update the Ion barriers in ~AutoUpdateBarriers.
-     */
     if (zone->isGCMarking()) {
       MOZ_ASSERT(zone->needsIncrementalBarrier());
       zone->setNeedsIncrementalBarrier(false);
@@ -6662,9 +6651,8 @@ AutoUpdateBarriers::AutoUpdateBarriers(GCRuntime* gc) : gc(gc) {
   }
 }
 
-AutoUpdateBarriers::~AutoUpdateBarriers() {
-  /* We can't use GCZonesIter if this is the end of the last slice. */
-  for (ZonesIter zone(gc, WithAtoms); !zone.done(); zone.next()) {
+AutoDisableBarriers::~AutoDisableBarriers() {
+  for (GCZonesIter zone(gc); !zone.done(); zone.next()) {
     MOZ_ASSERT(!zone->needsIncrementalBarrier());
     if (zone->isGCMarking()) {
       zone->setNeedsIncrementalBarrier(true);
@@ -6687,7 +6675,6 @@ static bool ShouldSweepOnBackgroundThread(JS::GCReason reason) {
 void GCRuntime::incrementalSlice(SliceBudget& budget,
                                  const MaybeInvocationKind& gckind,
                                  JS::GCReason reason, AutoGCSession& session) {
-  AutoUpdateBarriers updateBarriers(this);
   AutoSetThreadIsPerformingGC performingGC;
 
   bool destroyingRuntime = (reason == JS::GCReason::DESTROY_RUNTIME);
@@ -8754,6 +8741,8 @@ const char* StateName(JS::Zone::GCState state) {
   switch (state) {
     case JS::Zone::NoGC:
       return "NoGC";
+    case JS::Zone::Prepare:
+      return "Prepare";
     case JS::Zone::MarkBlackOnly:
       return "MarkBlackOnly";
     case JS::Zone::MarkBlackAndGray:

js/src/gc/GCInternals.h

@@ -169,6 +169,18 @@ class MOZ_RAII AutoEmptyNurseryAndPrepareForTracing : private AutoFinishGC,
         AutoTraceSession(cx->runtime()) {}
 };
 
+/*
+ * Temporarily disable incremental barriers.
+ */
+class AutoDisableBarriers {
+ public:
+  explicit AutoDisableBarriers(GCRuntime* gc);
+  ~AutoDisableBarriers();
+
+ private:
+  GCRuntime* gc;
+};
+
 GCAbortReason IsIncrementalGCUnsafe(JSRuntime* rt);
 
 #ifdef JS_GC_ZEAL

js/src/gc/Zone.cpp

@@ -235,6 +235,25 @@ void Zone::setNeedsIncrementalBarrier(bool needs) {
   needsIncrementalBarrier_ = needs;
 }
 
+void Zone::changeGCState(GCState prev, GCState next) {
+  MOZ_ASSERT(RuntimeHeapIsBusy());
+  MOZ_ASSERT(canCollect());
+  MOZ_ASSERT(gcState() == prev);
+
+  // This can be called when barriers have been temporarily disabled by
+  // AutoDisableBarriers. In that case, don't update needsIncrementalBarrier_
+  // and barriers will be re-enabled by ~AutoDisableBarriers() if necessary.
+  bool barriersDisabled = isGCMarking() && !needsIncrementalBarrier();
+
+  gcState_ = next;
+
+  // Update the barriers state when we transition between marking and
+  // non-marking states, unless barriers have been disabled.
+  if (!barriersDisabled) {
+    needsIncrementalBarrier_ = isGCMarking();
+  }
+}
+
 void Zone::beginSweepTypes() { types.beginSweep(); }
 
 template <class Pred>

js/src/gc/Zone.h

@@ -437,12 +437,7 @@ class Zone : public js::ZoneAllocator, public js::gc::GraphNodeBase<JS::Zone> {
   // Whether this zone can currently be collected.
   bool canCollect();
 
-  void changeGCState(GCState prev, GCState next) {
-    MOZ_ASSERT(RuntimeHeapIsBusy());
-    MOZ_ASSERT(gcState() == prev);
-    MOZ_ASSERT(canCollect());
-    gcState_ = next;
-  }
+  void changeGCState(GCState prev, GCState next);
 
   bool isCollecting() const {
     MOZ_ASSERT(js::CurrentThreadCanAccessRuntime(runtimeFromMainThread()));