commit 00c99c7b32988d3131c6305f991a7947d71833b5
parent be45c3dff0d54d13191e9684f34b8ebb10a8aff6
Author: Jon Coppeard <jcoppeard@mozilla.com>
Date: Wed, 29 Oct 2025 09:19:04 +0000
Bug 1996636 - Simplify disabling barriers during sweeping r=sfink
Differential Revision: https://phabricator.services.mozilla.com/D270218
Diffstat:
5 files changed, 55 insertions(+), 35 deletions(-)
diff --git a/js/src/gc/GC.cpp b/js/src/gc/GC.cpp
@@ -3562,9 +3562,9 @@ void GCRuntime::checkGCStateNotInUse() {
MOZ_ASSERT(marker->isDrained());
}
MOZ_ASSERT(!hasDelayedMarking());
-
MOZ_ASSERT(!lastMarkSlice);
+ MOZ_ASSERT(!disableBarriersForSweeping);
MOZ_ASSERT(foregroundFinalizedArenas.ref().isNothing());
for (ZonesIter zone(this, WithAtoms); !zone.done(); zone.next()) {
@@ -3877,12 +3877,12 @@ GCRuntime::IncrementalResult GCRuntime::resetIncrementalGC(
return IncrementalResult::Reset;
}
-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()) {
+void GCRuntime::disableIncrementalBarriers() {
+ // Clear needsIncrementalBarrier so we don't do any write barriers during
+ // foreground finalization. This would otherwise happen when destroying
+ // HeapPtr<>s to GC things in zones which are still marking.
+
+ for (GCZonesIter zone(this); !zone.done(); zone.next()) {
if (zone->isGCMarking()) {
MOZ_ASSERT(zone->needsIncrementalBarrier());
zone->setNeedsIncrementalBarrier(false);
@@ -3891,8 +3891,8 @@ AutoDisableBarriers::AutoDisableBarriers(GCRuntime* gc) : gc(gc) {
}
}
-AutoDisableBarriers::~AutoDisableBarriers() {
- for (GCZonesIter zone(gc); !zone.done(); zone.next()) {
+void GCRuntime::enableIncrementalBarriers() {
+ for (GCZonesIter zone(this); !zone.done(); zone.next()) {
MOZ_ASSERT(!zone->needsIncrementalBarrier());
if (zone->isGCMarking()) {
zone->setNeedsIncrementalBarrier(true);
diff --git a/js/src/gc/GCInternals.h b/js/src/gc/GCInternals.h
@@ -106,18 +106,6 @@ class MOZ_RAII AutoEmptyNurseryAndPrepareForTracing : private AutoFinishGC,
AutoTraceSession(cx->runtime()) {}
};
-/*
- * Temporarily disable incremental barriers.
- */
-class AutoDisableBarriers {
- public:
- explicit AutoDisableBarriers(GCRuntime* gc);
- ~AutoDisableBarriers();
-
- private:
- GCRuntime* gc;
-};
-
// Set compartments' maybeAlive flags if anything is marked while this class is
// live. This is used while marking roots.
class AutoUpdateLiveCompartments {
diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h
@@ -46,6 +46,7 @@ using BlackGrayEdgeVector = Vector<TenuredCell*, 0, SystemAllocPolicy>;
using ZoneVector = Vector<JS::Zone*, 4, SystemAllocPolicy>;
class AutoCallGCCallbacks;
+class AutoUpdateBarriersForSweeping;
class AutoGCSession;
class AutoHeapSession;
class AutoTraceSession;
@@ -940,6 +941,8 @@ class GCRuntime {
void freeFromBackgroundThread(AutoLockHelperThreadState& lock);
void sweepBackgroundThings(ZoneList& zones);
void prepareForSweepSlice(JS::GCReason reason);
+ void disableIncrementalBarriers();
+ void enableIncrementalBarriers();
void assertBackgroundSweepingFinished();
#ifdef DEBUG
bool zoneInCurrentSweepGroup(Zone* zone) const;
@@ -1265,6 +1268,8 @@ class GCRuntime {
weakCachesToSweep;
MainThreadData<bool> abortSweepAfterCurrentGroup;
MainThreadOrGCTaskData<IncrementalProgress> sweepMarkResult;
+ MainThreadData<bool> disableBarriersForSweeping;
+ friend class AutoUpdateBarriersForSweeping;
/*
* During incremental foreground finalization, we may have a list of arenas of
diff --git a/js/src/gc/Sweeping.cpp b/js/src/gc/Sweeping.cpp
@@ -1676,6 +1676,13 @@ IncrementalProgress GCRuntime::beginSweepingSweepGroup(JS::GCContext* gcx,
AutoSetThreadIsSweeping threadIsSweeping;
+ // Disable incremental barriers for all zones while we are sweeping/finalizing
+ // zones in this sweep group. Set the |disableBarriersForSweeping| flag so we
+ // enable/disable the barriers on yield/resume.
+ MOZ_ASSERT(!disableBarriersForSweeping);
+ disableBarriersForSweeping = true;
+ disableIncrementalBarriers();
+
// This must happen before sweeping realm globals.
sweepDebuggerOnMainThread(gcx);
@@ -1835,6 +1842,12 @@ IncrementalProgress GCRuntime::endSweepingSweepGroup(JS::GCContext* gcx,
}
queueZonesAndStartBackgroundSweep(std::move(zones));
+ // Re-enable incremental barriers for all zones now we are we done sweeping
+ // zones in this group.
+ MOZ_ASSERT(disableBarriersForSweeping);
+ disableBarriersForSweeping = false;
+ enableIncrementalBarriers();
+
return Finished;
}
@@ -2483,6 +2496,29 @@ void GCRuntime::prepareForSweepSlice(JS::GCReason reason) {
rt->mainContextFromOwnThread()->traceWrapperGCRooters(marker().tracer());
}
+// Ensure barriers are disabled if required when entering a sweep slice and
+// re-enabled when yielding to the mutator. |disableBarriersForSweeping| is set
+// in beginSweepingSweepGroup and cleared in endSweepingSweepGroup.
+class js::gc::AutoUpdateBarriersForSweeping {
+ public:
+ explicit AutoUpdateBarriersForSweeping(GCRuntime* gc) : gc(gc) {
+ MOZ_ASSERT(gc->state() == State::Sweep);
+ if (gc->disableBarriersForSweeping) {
+ gc->disableIncrementalBarriers();
+ }
+ }
+
+ ~AutoUpdateBarriersForSweeping() {
+ MOZ_ASSERT(gc->state() == State::Sweep);
+ if (gc->disableBarriersForSweeping) {
+ gc->enableIncrementalBarriers();
+ }
+ }
+
+ private:
+ GCRuntime* gc;
+};
+
IncrementalProgress GCRuntime::performSweepActions(SliceBudget& budget) {
MOZ_ASSERT_IF(storeBuffer().isEnabled(),
!storeBuffer().mayHavePointersToDeadCells());
@@ -2494,9 +2530,6 @@ IncrementalProgress GCRuntime::performSweepActions(SliceBudget& budget) {
AutoSetThreadIsSweeping threadIsSweeping(gcx);
AutoPoisonFreedJitCode pjc(gcx);
- // 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.
//
@@ -2515,6 +2548,9 @@ IncrementalProgress GCRuntime::performSweepActions(SliceBudget& budget) {
}
}
+ // Don't trigger pre-barriers when sweeping or finalizing.
+ AutoUpdateBarriersForSweeping updateBarriers(this);
+
// Then continue running sweep actions.
SweepAction::Args args{this, gcx, budget};
diff --git a/js/src/gc/Zone.cpp b/js/src/gc/Zone.cpp
@@ -223,19 +223,10 @@ void Zone::setNeedsIncrementalBarrier(bool needs) {
void Zone::changeGCState(GCState prev, GCState next) {
MOZ_ASSERT(RuntimeHeapIsBusy());
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();
+ MOZ_ASSERT_IF(isGCMarkingOrVerifyingPreBarriers(), 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();
- }
+ needsIncrementalBarrier_ = isGCMarkingOrVerifyingPreBarriers();
}
template <class Pred>
