Bug 1730140 - Remove the gray root buffer and mark gray roots after the start of collection r=sfink,mccr8

This removes gray root buffering from the first marking slice and traces the
gray roots directly in a later slice. This relies on Heap<T> read barriers
being sufficient to ensure correctness.

This is conservative in that it makes no effort to skip tracing roots added
after the start of GC. It also doesn't trace roots removed after the start of
GC, but this is OK because barriers ensure marking of any observed values.

The gray root tracing callback will be called once per sweep group, which means
we will trace all zone holders and xpconnect gray roots for every group rather
than just once. This should not be a problem in practice as we expect the
number of zones and hence zone groups to decrease with fission.

On the plus side we no longer have to do a virtual dispatch per root traced
(for the buffering tracer), allocate memory for the buffer, or trace each root
twice.

Note that this doesn't make the gray root marking itself incremental yet.

Differential Revision: https://phabricator.services.mozilla.com/D125188
This commit is contained in:
Jon Coppeard 2021-09-14 07:49:39 +00:00
parent 0500b123b6
commit fe43168478
12 changed files with 33 additions and 230 deletions

View File

@ -15,6 +15,7 @@
#include "mozilla/Vector.h"
#include "js/GCAnnotations.h"
#include "js/shadow/Zone.h"
#include "js/TypeDecls.h"
#include "js/UniquePtr.h"
#include "js/Utility.h"
@ -1197,7 +1198,10 @@ extern JS_PUBLIC_API void SetHostCleanupFinalizationRegistryCallback(
*/
extern JS_PUBLIC_API void ClearKeptObjects(JSContext* cx);
extern JS_PUBLIC_API bool ZoneIsCollecting(Zone* zone);
inline JS_PUBLIC_API bool ZoneIsGrayMarking(Zone* zone) {
return shadow::Zone::from(zone)->isGCMarkingBlackAndGray();
}
extern JS_PUBLIC_API bool AtomsZoneIsCollecting(JSRuntime* runtime);
extern JS_PUBLIC_API bool IsAtomsZone(Zone* zone);

View File

@ -860,7 +860,7 @@ void GCRuntime::updateRuntimePointersToRelocatedCells(AutoGCSession& session) {
DebugAPI::traceAllForMovingGC(&trc);
DebugAPI::traceCrossCompartmentEdges(&trc);
// Mark all gray roots. We call the trace callback to get the current set.
// Mark all gray roots.
traceEmbeddingGrayRoots(&trc);
Compartment::traceIncomingCrossCompartmentEdgesForZoneGC(
&trc, Compartment::GrayEdges);

View File

@ -1144,7 +1144,6 @@ GCRuntime::GCRuntime(JSRuntime* rt)
perZoneGCEnabled(TuningDefaults::PerZoneGCEnabled),
numActiveZoneIters(0),
cleanUpEverything(false),
grayBufferState(GCRuntime::GrayBufferState::Unused),
grayBitsValid(false),
majorGCTriggerReason(JS::GCReason::NO_REASON),
fullGCForAtomsRequested_(false),
@ -3717,17 +3716,6 @@ void GCRuntime::endPreparePhase(JS::GCReason reason) {
gcstats::PhaseKind::UNMARK_WEAKMAPS,
helperLock);
/*
* Buffer gray roots for incremental collections. This is linear in the
* number of roots which can be in the tens of thousands. Do this in
* parallel with the rest of this block.
*/
Maybe<AutoRunParallelTask> bufferGrayRootsTask;
if (isIncremental) {
bufferGrayRootsTask.emplace(this, &GCRuntime::bufferGrayRoots,
gcstats::PhaseKind::BUFFER_GRAY_ROOTS,
helperLock);
}
AutoUnlockHelperThreadState unlock(helperLock);
// Discard JIT code. For incremental collections, the sweep phase will
@ -3961,16 +3949,10 @@ void GCRuntime::markGrayRoots(gcstats::PhaseKind phase) {
MOZ_ASSERT(marker.markColor() == MarkColor::Gray);
gcstats::AutoPhase ap(stats(), phase);
if (hasValidGrayRootsBuffer()) {
for (ZoneIterT zone(this); !zone.done(); zone.next()) {
markBufferedGrayRoots(zone);
}
} else {
MOZ_ASSERT(!isIncremental);
traceEmbeddingGrayRoots(&marker);
Compartment::traceIncomingCrossCompartmentEdgesForZoneGC(
&marker, Compartment::GrayEdges);
}
traceEmbeddingGrayRoots(&marker);
Compartment::traceIncomingCrossCompartmentEdgesForZoneGC(
&marker, Compartment::GrayEdges);
}
IncrementalProgress GCRuntime::markAllWeakReferences() {
@ -4184,7 +4166,6 @@ void GCRuntime::getNextSweepGroup() {
zone->changeGCState(Zone::MarkBlackOnly, Zone::NoGC);
zone->arenas.unmarkPreMarkedFreeCells();
zone->arenas.mergeNewArenasInMarkPhase();
zone->gcGrayRoots().Clear();
zone->clearGCSliceThresholds();
}
@ -5763,8 +5744,6 @@ void GCRuntime::finishCollection() {
MOZ_ASSERT(marker.isDrained());
marker.stop();
clearBufferedGrayRoots();
maybeStopPretenuring();
{
@ -5971,7 +5950,6 @@ GCRuntime::IncrementalResult GCRuntime::resetIncrementalGC(
case State::Mark: {
// Cancel any ongoing marking.
marker.reset();
clearBufferedGrayRoots();
for (GCCompartmentsIter c(rt); !c.done(); c.next()) {
ResetGrayList(c);
@ -6182,16 +6160,7 @@ void GCRuntime::incrementalSlice(SliceBudget& budget,
}
endPreparePhase(reason);
beginMarkPhase(session);
// If we needed delayed marking for gray roots, then collect until done.
if (isIncremental && !hasValidGrayRootsBuffer()) {
budget = SliceBudget::unlimited();
isIncremental = false;
stats().nonincremental(GCAbortReason::GrayRootBufferingFailed);
}
incrementalState = State::Mark;
if (useZeal && hasZealMode(ZealMode::YieldBeforeMarking) &&

View File

@ -726,7 +726,6 @@ class GCRuntime {
[[nodiscard]] bool beginPreparePhase(JS::GCReason reason,
AutoGCSession& session);
bool prepareZonesForCollection(JS::GCReason reason, bool* isFullOut);
void bufferGrayRoots();
void unmarkWeakMaps();
void endPreparePhase(JS::GCReason reason);
void beginMarkPhase(AutoGCSession& session);
@ -749,27 +748,6 @@ class GCRuntime {
void maybeDoCycleCollection();
void findDeadCompartments();
// Gray marking must be done after all black marking is complete. However,
// we do not have write barriers on XPConnect roots. Therefore, XPConnect
// roots must be accumulated in the first slice of incremental GC. We
// accumulate these roots in each zone's gcGrayRoots vector and then mark
// them later, after black marking is complete for each compartment. This
// accumulation can fail, but in that case we switch to non-incremental GC.
enum class GrayBufferState { Unused, Okay, Failed };
bool hasValidGrayRootsBuffer() const {
return grayBufferState == GrayBufferState::Okay;
}
// Clear each zone's gray buffers, but do not change the current state.
void resetBufferedGrayRoots();
// Reset the gray buffering state to Unused.
void clearBufferedGrayRoots() {
grayBufferState = GrayBufferState::Unused;
resetBufferedGrayRoots();
}
friend class BackgroundMarkTask;
IncrementalProgress markUntilBudgetExhausted(
SliceBudget& sliceBudget,
@ -987,8 +965,6 @@ class GCRuntime {
/* During shutdown, the GC needs to clean up every possible object. */
MainThreadData<bool> cleanUpEverything;
MainThreadOrGCTaskData<GrayBufferState> grayBufferState;
/*
* The gray bits can become invalid if UnmarkGray overflows the stack. A
* full GC will reset this bit, since it fills in all the gray bits.

View File

@ -946,6 +946,14 @@ void DoMarking(GCMarker* gcmarker, T* thing) {
return;
}
// Don't mark gray in zones which we are still marking black, except for the
// atoms zone.
Zone* zone = thing->asTenured().zone();
if (gcmarker->markColor() == MarkColor::Gray &&
zone->isGCMarkingBlackOnly() && !zone->isAtomsZone()) {
return;
}
CheckTracedThing(gcmarker, thing);
AutoClearTracingSource acts(gcmarker);
gcmarker->markAndTraverse(thing);
@ -1175,7 +1183,7 @@ struct TraceKindCanBeGray {};
#define EXPAND_TRACEKIND_DEF(_, type, canBeGray, _1) \
template <> \
struct TraceKindCanBeGray<type> { \
static const bool value = canBeGray; \
static constexpr bool value = canBeGray; \
};
JS_FOR_EACH_TRACEKIND(EXPAND_TRACEKIND_DEF)
#undef EXPAND_TRACEKIND_DEF
@ -2715,9 +2723,6 @@ void GCMarker::checkZone(void* p) {
size_t GCMarker::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
size_t size = stack.sizeOfExcludingThis(mallocSizeOf);
size += auxStack.sizeOfExcludingThis(mallocSizeOf);
for (ZonesIter zone(runtime(), WithAtoms); !zone.done(); zone.next()) {
size += zone->gcGrayRoots().SizeOfExcludingThis(mallocSizeOf);
}
return size;
}

View File

@ -468,154 +468,6 @@ void js::gc::GCRuntime::checkNoRuntimeRoots(AutoGCSession& session) {
#endif // DEBUG
}
// Append traced things to a buffer on the zone for use later in the GC.
// See the comment in GCRuntime.h above grayBufferState for details.
class BufferGrayRootsTracer final : public GenericTracer {
// Set to false if we OOM while buffering gray roots.
bool bufferingGrayRootsFailed;
JSObject* onObjectEdge(JSObject* obj) override { return bufferRoot(obj); }
JSString* onStringEdge(JSString* string) override {
return bufferRoot(string);
}
js::BaseScript* onScriptEdge(js::BaseScript* script) override {
return bufferRoot(script);
}
JS::Symbol* onSymbolEdge(JS::Symbol* symbol) override {
return bufferRoot(symbol);
}
JS::BigInt* onBigIntEdge(JS::BigInt* bi) override { return bufferRoot(bi); }
js::Shape* onShapeEdge(js::Shape* shape) override {
unsupportedEdge();
return nullptr;
}
js::BaseShape* onBaseShapeEdge(js::BaseShape* base) override {
unsupportedEdge();
return nullptr;
}
js::GetterSetter* onGetterSetterEdge(js::GetterSetter* gs) override {
unsupportedEdge();
return nullptr;
}
js::PropMap* onPropMapEdge(js::PropMap* map) override {
unsupportedEdge();
return nullptr;
}
js::jit::JitCode* onJitCodeEdge(js::jit::JitCode* code) override {
unsupportedEdge();
return nullptr;
}
js::Scope* onScopeEdge(js::Scope* scope) override {
unsupportedEdge();
return nullptr;
}
js::RegExpShared* onRegExpSharedEdge(js::RegExpShared* shared) override {
unsupportedEdge();
return nullptr;
}
void unsupportedEdge() { MOZ_CRASH("Unsupported gray root edge kind"); }
template <typename T>
inline T* bufferRoot(T* thing);
public:
explicit BufferGrayRootsTracer(JSRuntime* rt)
: GenericTracer(rt, JS::TracerKind::GrayBuffering),
bufferingGrayRootsFailed(false) {}
bool failed() const { return bufferingGrayRootsFailed; }
void setFailed() { bufferingGrayRootsFailed = true; }
};
void js::gc::GCRuntime::bufferGrayRoots() {
// Precondition: the state has been reset to "unused" after the last GC
// and the zone's buffers have been cleared.
MOZ_ASSERT(grayBufferState == GrayBufferState::Unused);
for (GCZonesIter zone(this); !zone.done(); zone.next()) {
MOZ_ASSERT(zone->gcGrayRoots().IsEmpty());
}
BufferGrayRootsTracer grayBufferer(rt);
traceEmbeddingGrayRoots(&grayBufferer);
Compartment::traceIncomingCrossCompartmentEdgesForZoneGC(
&grayBufferer, Compartment::GrayEdges);
// Propagate the failure flag from the marker to the runtime.
if (grayBufferer.failed()) {
grayBufferState = GrayBufferState::Failed;
resetBufferedGrayRoots();
} else {
grayBufferState = GrayBufferState::Okay;
}
}
template <typename T>
inline T* BufferGrayRootsTracer::bufferRoot(T* thing) {
MOZ_ASSERT(JS::RuntimeHeapIsBusy());
MOZ_ASSERT(thing);
// Check if |thing| is corrupt by calling a method that touches the heap.
MOZ_ASSERT(thing->getTraceKind() != JS::TraceKind(0xff));
TenuredCell* tenured = &thing->asTenured();
// This is run from a helper thread while the mutator is paused so we have
// to use *FromAnyThread methods here.
Zone* zone = tenured->zoneFromAnyThread();
if (zone->isCollectingFromAnyThread()) {
// See the comment on SetMaybeAliveFlag to see why we only do this for
// objects and scripts. We rely on gray root buffering for this to work,
// but we only need to worry about uncollected dead compartments during
// incremental GCs (when we do gray root buffering).
SetMaybeAliveFlag(thing);
if (!zone->gcGrayRoots().Append(tenured)) {
bufferingGrayRootsFailed = true;
}
}
return thing;
}
void GCRuntime::markBufferedGrayRoots(JS::Zone* zone) {
MOZ_ASSERT(grayBufferState == GrayBufferState::Okay);
MOZ_ASSERT(zone->isGCMarkingBlackAndGray() || zone->isGCCompacting());
auto& roots = zone->gcGrayRoots();
if (roots.IsEmpty()) {
return;
}
for (auto iter = roots.Iter(); !iter.Done(); iter.Next()) {
Cell* cell = iter.Get();
// Bug 1203273: Check for bad pointers on OSX and output diagnostics.
#if defined(XP_DARWIN) && defined(MOZ_DIAGNOSTIC_ASSERT_ENABLED)
auto addr = uintptr_t(cell);
if (addr < ChunkSize || addr % CellAlignBytes != 0) {
MOZ_CRASH_UNSAFE_PRINTF(
"Bad GC thing pointer in gray root buffer: %p at address %p", cell,
&iter.Get());
}
#else
MOZ_ASSERT(IsCellPointerValid(cell));
#endif
TraceManuallyBarrieredGenericPointerEdge(&marker, &cell,
"buffered gray root");
}
}
void GCRuntime::resetBufferedGrayRoots() {
MOZ_ASSERT(
grayBufferState != GrayBufferState::Okay,
"Do not clear the gray buffers unless we are Failed or becoming Unused");
for (GCZonesIter zone(this); !zone.done(); zone.next()) {
zone->gcGrayRoots().Clear();
}
}
JS_PUBLIC_API void JS::AddPersistentRoot(
JS::RootingContext* cx, RootKind kind,
PersistentRooted<JS::detail::RootListEntry*>* root) {

View File

@ -162,7 +162,6 @@ JS::Zone::Zone(JSRuntime* rt, Kind kind)
gcWeakMapList_(this),
compartments_(),
crossZoneStringWrappers_(this),
gcGrayRoots_(this),
weakCaches_(this),
gcEphemeronEdges_(this, SystemAllocPolicy(),
rt->randomHashCodeScrambler()),

View File

@ -257,12 +257,6 @@ class Zone : public js::ZoneAllocator, public js::gc::GraphNodeBase<JS::Zone> {
// All cross-zone string wrappers in the zone.
js::MainThreadOrGCTaskData<js::StringWrapperMap> crossZoneStringWrappers_;
// This zone's gray roots.
using GrayRootVector =
mozilla::SegmentedVector<js::gc::Cell*, 1024 * sizeof(js::gc::Cell*),
js::SystemAllocPolicy>;
js::ZoneOrGCTaskData<GrayRootVector> gcGrayRoots_;
// List of non-ephemeron weak containers to sweep during
// beginSweepingSweepGroup.
js::ZoneOrGCTaskData<mozilla::LinkedList<detail::WeakCacheBase>> weakCaches_;
@ -521,8 +515,6 @@ class Zone : public js::ZoneAllocator, public js::gc::GraphNodeBase<JS::Zone> {
void sweepAllCrossCompartmentWrappers();
static void fixupAllCrossCompartmentWrappersAfterMovingGC(JSTracer* trc);
GrayRootVector& gcGrayRoots() { return gcGrayRoots_.ref(); }
mozilla::LinkedList<detail::WeakCacheBase>& weakCaches() {
return weakCaches_.ref();
}

View File

@ -1350,10 +1350,6 @@ JS_PUBLIC_API void JS::ClearKeptObjects(JSContext* cx) {
}
}
JS_PUBLIC_API bool JS::ZoneIsCollecting(JS::Zone* zone) {
return zone->wasGCStarted();
}
JS_PUBLIC_API bool JS::AtomsZoneIsCollecting(JSRuntime* runtime) {
return runtime->activeGCInAtomsZone();
}

View File

@ -20,6 +20,16 @@
class JSJitInfo;
/*
* Set a callback used to trace gray roots.
*
* The callback is called after the first slice of GC so the embedding must
* implement appropriate barriers on its gray roots to ensure correctness.
*
* This callback may be called multiple times for different sets of zones. Use
* JS::ZoneIsGrayMarking() to determine whether roots from a particular zone are
* required.
*/
extern JS_PUBLIC_API void JS_SetGrayGCRootsTracer(JSContext* cx,
JSTraceDataOp traceOp,
void* data);

View File

@ -491,8 +491,8 @@ inline void JSHolderMap::ForEach(F&& f, WhichHolders aWhich) {
ForEach(mAnyZoneJSHolders, f, nullptr);
for (auto i = mPerZoneJSHolders.modIter(); !i.done(); i.next()) {
if (aWhich == HoldersInCollectingZones &&
!JS::ZoneIsCollecting(i.get().key())) {
if (aWhich == HoldersInGrayMarkingZones &&
!JS::ZoneIsGrayMarking(i.get().key())) {
continue;
}
@ -972,7 +972,7 @@ void CycleCollectedJSRuntime::TraceGrayJS(JSTracer* aTracer, void* aData) {
// Mark these roots as gray so the CC can walk them later.
JSHolderMap::WhichHolders which = JSHolderMap::HoldersInCollectingZones;
JSHolderMap::WhichHolders which = JSHolderMap::HoldersInGrayMarkingZones;
if (JS::AtomsZoneIsCollecting(self->Runtime())) {
// Any holder may point into the atoms zone.
which = JSHolderMap::AllHolders;

View File

@ -88,7 +88,7 @@ class IncrementalFinalizeRunnable;
// SegmentedVector to speed up iteration.
class JSHolderMap {
public:
enum WhichHolders { AllHolders, HoldersInCollectingZones };
enum WhichHolders { AllHolders, HoldersInGrayMarkingZones };
JSHolderMap();