Avoid TLS lookups when checking if zones need to be marked for GC.

This converts the fromPointer() calls to an object cast to (hopefully)
avoid the GC crashes. Logic is reversed from MustSkipMarking to
ShouldMark with appropriate renames.
This commit is contained in:
Moonchild 2023-06-27 18:39:24 +02:00 • committed by roytam1
commit 24b1a0f166
5 changed files with 42 additions and 44 deletions

View file

@ -308,7 +308,7 @@ ShouldMarkCrossCompartment(JSTracer* trc, JSObject* src, Cell* cell)
MOZ_ASSERT(!zone->isCollecting()); MOZ_ASSERT(!zone->isCollecting());
trc->runtime()->gc.setFoundBlackGrayEdges(tenured); trc->runtime()->gc.setFoundBlackGrayEdges(tenured);
} }
return zone->isGCMarking(); return zone->shouldMarkInZone();
} else { } else {
if (zone->isGCMarkingBlack()) { if (zone->isGCMarkingBlack()) {
/* /*
@ -331,26 +331,26 @@ ShouldMarkCrossCompartment(JSTracer* trc, JSObject* src, const Value& val)
} }
static void static void
AssertZoneIsMarking(Cell* thing) AssertShouldMarkInZone(Cell* thing)
{ {
MOZ_ASSERT(TenuredCell::fromPointer(thing)->zone()->isGCMarking()); MOZ_ASSERT(thing->asTenured().zone()->shouldMarkInZone());
} }
static void static void
AssertZoneIsMarking(JSString* str) AssertShouldMarkInZone(JSString* str)
{ {
#ifdef DEBUG #ifdef DEBUG
Zone* zone = TenuredCell::fromPointer(str)->zone(); Zone* zone = str->asTenured().zone();
MOZ_ASSERT(zone->isGCMarking() || zone->isAtomsZone()); MOZ_ASSERT(zone->shouldMarkInZone() || zone->isAtomsZone());
#endif #endif
} }
static void static void
AssertZoneIsMarking(JS::Symbol* sym) AssertShouldMarkInZone(JS::Symbol* sym)
{ {
#ifdef DEBUG #ifdef DEBUG
Zone* zone = TenuredCell::fromPointer(sym)->zone(); Zone* zone = sym->asTenured().zone();
MOZ_ASSERT(zone->isGCMarking() || zone->isAtomsZone()); MOZ_ASSERT(zone->shouldMarkInZone() || zone->isAtomsZone());
#endif #endif
} }
@ -759,35 +759,35 @@ GCMarker::markImplicitEdges(T* thing)
template <typename T> template <typename T>
static inline bool static inline bool
MustSkipMarking(GCMarker* gcmarker, T thing) ShouldMark(GCMarker* gcmarker, T thing)
{ {
// Don't trace things that are owned by another runtime. // Don't trace things that are owned by another runtime.
if (IsOwnedByOtherRuntime(gcmarker->runtime(), thing)) if (IsOwnedByOtherRuntime(gcmarker->runtime(), thing))
return true; return false;
// Don't mark things outside a zone if we are in a per-zone GC. // Don't mark things outside a zone if we are in a per-zone GC.
return !thing->zone()->isGCMarking(); return thing->zone()->shouldMarkInZone();
} }
template <> template <>
bool bool
MustSkipMarking<JSObject*>(GCMarker* gcmarker, JSObject* obj) ShouldMark<JSObject*>(GCMarker* gcmarker, JSObject* obj)
{ {
// Don't trace things that are owned by another runtime. // Don't trace things that are owned by another runtime.
if (IsOwnedByOtherRuntime(gcmarker->runtime(), obj)) if (IsOwnedByOtherRuntime(gcmarker->runtime(), obj))
return true; return false;
// We may mark a Nursery thing outside the context of the // We may mark a Nursery thing outside the context of the
// MinorCollectionTracer because of a pre-barrier. The pre-barrier is not // MinorCollectionTracer because of a pre-barrier. The pre-barrier is not
// needed in this case because we perform a minor collection before each // needed in this case because we perform a minor collection before each
// incremental slice. // incremental slice.
if (IsInsideNursery(obj)) if (IsInsideNursery(obj))
return true; return false;
// Don't mark things outside a zone if we are in a per-zone GC. It is // Don't mark things outside a zone if we are in a per-zone GC. It is
// faster to check our own arena, which we can do since we know that // faster to check our own arena, which we can do since we know that
// the object is tenured. // the object is tenured.
return !TenuredCell::fromPointer(obj)->zone()->isGCMarking(); return obj->asTenured().zone()->shouldMarkInZone();
} }
template <typename T> template <typename T>
@ -795,7 +795,7 @@ void
DoMarking(GCMarker* gcmarker, T* thing) DoMarking(GCMarker* gcmarker, T* thing)
{ {
// Do per-type marking precondition checks. // Do per-type marking precondition checks.
if (MustSkipMarking(gcmarker, thing)) if (!ShouldMark(gcmarker, thing))
return; return;
CheckTracedThing(gcmarker, thing); CheckTracedThing(gcmarker, thing);
@ -822,7 +822,7 @@ void
NoteWeakEdge(GCMarker* gcmarker, T** thingp) NoteWeakEdge(GCMarker* gcmarker, T** thingp)
{ {
// Do per-type marking precondition checks. // Do per-type marking precondition checks.
if (MustSkipMarking(gcmarker, *thingp)) if (!ShouldMark(gcmarker, *thingp))
return; return;
CheckTracedThing(gcmarker, *thingp); CheckTracedThing(gcmarker, *thingp);
@ -971,7 +971,7 @@ template <typename T>
bool bool
js::GCMarker::mark(T* thing) js::GCMarker::mark(T* thing)
{ {
AssertZoneIsMarking(thing); AssertShouldMarkInZone(thing);
MOZ_ASSERT(!IsInsideNursery(gc::TenuredCell::fromPointer(thing))); MOZ_ASSERT(!IsInsideNursery(gc::TenuredCell::fromPointer(thing)));
return gc::ParticipatesInCC<T>::value return gc::ParticipatesInCC<T>::value
? gc::TenuredCell::fromPointer(thing)->markIfUnmarked(markColor()) ? gc::TenuredCell::fromPointer(thing)->markIfUnmarked(markColor())
@ -1106,7 +1106,7 @@ JSString::traceBase(JSTracer* trc)
inline void inline void
js::GCMarker::eagerlyMarkChildren(JSLinearString* linearStr) js::GCMarker::eagerlyMarkChildren(JSLinearString* linearStr)
{ {
AssertZoneIsMarking(linearStr); AssertShouldMarkInZone(linearStr);
MOZ_ASSERT(linearStr->isMarked()); MOZ_ASSERT(linearStr->isMarked());
MOZ_ASSERT(linearStr->JSString::isLinear()); MOZ_ASSERT(linearStr->JSString::isLinear());
@ -1116,7 +1116,7 @@ js::GCMarker::eagerlyMarkChildren(JSLinearString* linearStr)
MOZ_ASSERT(linearStr->JSString::isLinear()); MOZ_ASSERT(linearStr->JSString::isLinear());
if (linearStr->isPermanentAtom()) if (linearStr->isPermanentAtom())
break; break;
AssertZoneIsMarking(linearStr); AssertShouldMarkInZone(linearStr);
if (!mark(static_cast<JSString*>(linearStr))) if (!mark(static_cast<JSString*>(linearStr)))
break; break;
} }
@ -1169,7 +1169,7 @@ js::GCMarker::eagerlyMarkChildren(JSRope* rope)
JS_DIAGNOSTICS_ASSERT(rope->getTraceKind() == JS::TraceKind::String); JS_DIAGNOSTICS_ASSERT(rope->getTraceKind() == JS::TraceKind::String);
JS_DIAGNOSTICS_ASSERT(rope->JSString::isRope()); JS_DIAGNOSTICS_ASSERT(rope->JSString::isRope());
AssertZoneIsMarking(rope); AssertShouldMarkInZone(rope);
MOZ_ASSERT(rope->isMarked()); MOZ_ASSERT(rope->isMarked());
JSRope* next = nullptr; JSRope* next = nullptr;
@ -1649,7 +1649,7 @@ GCMarker::processMarkStackTop(SliceBudget& budget)
case ObjectTag: { case ObjectTag: {
obj = reinterpret_cast<JSObject*>(addr); obj = reinterpret_cast<JSObject*>(addr);
AssertZoneIsMarking(obj); AssertShouldMarkInZone(obj);
goto scan_obj; goto scan_obj;
} }
@ -1712,7 +1712,7 @@ GCMarker::processMarkStackTop(SliceBudget& budget)
scan_obj: scan_obj:
{ {
AssertZoneIsMarking(obj); AssertShouldMarkInZone(obj);
budget.step(); budget.step();
if (budget.isOverBudget()) { if (budget.isOverBudget()) {

View file

@ -232,11 +232,9 @@ struct Zone : public JS::shadow::Zone,
return rt->isHeapMajorCollecting() && !rt->gc.isHeapCompacting() && gcState_ != NoGC; return rt->isHeapMajorCollecting() && !rt->gc.isHeapCompacting() && gcState_ != NoGC;
} }
bool isGCMarking() { bool shouldMarkInZone() const {
if (runtimeFromMainThread()->isHeapCollecting()) return needsIncrementalBarrier() ||
return gcState_ == Mark || gcState_ == MarkGray; (gcState_ == Mark || gcState_ == MarkGray);
else
return needsIncrementalBarrier();
} }
GCState gcState() const { return gcState_; } GCState gcState() const { return gcState_; }

View file

@ -3952,7 +3952,7 @@ struct AddOutgoingEdgeFunctor {
*/ */
if (needsEdge_) { if (needsEdge_) {
JS::Zone* zone = other.zone(); JS::Zone* zone = other.zone();
if (zone->isGCMarking()) if (zone->shouldMarkInZone())
finder_.addEdgeTo(zone); finder_.addEdgeTo(zone);
} }
} }
@ -3983,14 +3983,14 @@ Zone::findOutgoingEdges(ZoneComponentFinder& finder)
*/ */
JSRuntime* rt = runtimeFromMainThread(); JSRuntime* rt = runtimeFromMainThread();
Zone* atomsZone = rt->atomsCompartment(finder.lock)->zone(); Zone* atomsZone = rt->atomsCompartment(finder.lock)->zone();
if (atomsZone->isGCMarking()) if (atomsZone->shouldMarkInZone())
finder.addEdgeTo(atomsZone); finder.addEdgeTo(atomsZone);
for (CompartmentsInZoneIter comp(this); !comp.done(); comp.next()) for (CompartmentsInZoneIter comp(this); !comp.done(); comp.next())
comp->findOutgoingEdges(finder); comp->findOutgoingEdges(finder);
for (ZoneSet::Range r = gcZoneGroupEdges.all(); !r.empty(); r.popFront()) { for (ZoneSet::Range r = gcZoneGroupEdges.all(); !r.empty(); r.popFront()) {
if (r.front()->isGCMarking()) if (r.front()->shouldMarkInZone())
finder.addEdgeTo(r.front()); finder.addEdgeTo(r.front());
} }
@ -4032,7 +4032,7 @@ GCRuntime::findZoneGroups(AutoLockForExclusiveAccess& lock)
finder.useOneComponent(); finder.useOneComponent();
for (GCZonesIter zone(rt); !zone.done(); zone.next()) { for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
MOZ_ASSERT(zone->isGCMarking()); MOZ_ASSERT(zone->shouldMarkInZone());
finder.addNode(zone); finder.addNode(zone);
} }
zoneGroups = finder.getResultsList(); zoneGroups = finder.getResultsList();
@ -4045,7 +4045,7 @@ GCRuntime::findZoneGroups(AutoLockForExclusiveAccess& lock)
#ifdef DEBUG #ifdef DEBUG
for (Zone* head = currentZoneGroup; head; head = head->nextGroup()) { for (Zone* head = currentZoneGroup; head; head = head->nextGroup()) {
for (Zone* zone = head; zone; zone = zone->nextNodeInGroup()) for (Zone* zone = head; zone; zone = zone->nextNodeInGroup())
MOZ_ASSERT(zone->isGCMarking()); MOZ_ASSERT(zone->shouldMarkInZone());
} }
MOZ_ASSERT_IF(!isIncremental, !currentZoneGroup->nextGroup()); MOZ_ASSERT_IF(!isIncremental, !currentZoneGroup->nextGroup());
@ -4068,7 +4068,7 @@ GCRuntime::getNextZoneGroup()
} }
for (Zone* zone = currentZoneGroup; zone; zone = zone->nextNodeInGroup()) { for (Zone* zone = currentZoneGroup; zone; zone = zone->nextNodeInGroup()) {
MOZ_ASSERT(zone->isGCMarking()); MOZ_ASSERT(zone->shouldMarkInZone());
MOZ_ASSERT(!zone->isQueuedForBackgroundSweep()); MOZ_ASSERT(!zone->isQueuedForBackgroundSweep());
} }
@ -4079,7 +4079,7 @@ GCRuntime::getNextZoneGroup()
MOZ_ASSERT(!isIncremental); MOZ_ASSERT(!isIncremental);
for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) { for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) {
MOZ_ASSERT(!zone->gcNextGraphComponent); MOZ_ASSERT(!zone->gcNextGraphComponent);
MOZ_ASSERT(zone->isGCMarking()); MOZ_ASSERT(zone->shouldMarkInZone());
zone->setNeedsIncrementalBarrier(false, Zone::UpdateJit); zone->setNeedsIncrementalBarrier(false, Zone::UpdateJit);
zone->setGCState(Zone::NoGC); zone->setGCState(Zone::NoGC);
zone->gcGrayRoots.clearAndFree(); zone->gcGrayRoots.clearAndFree();
@ -4522,7 +4522,7 @@ GCRuntime::beginSweepingZoneGroup(AutoLockForExclusiveAccess& lock)
bool sweepingAtoms = false; bool sweepingAtoms = false;
for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) { for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) {
/* Set the GC state to sweeping. */ /* Set the GC state to sweeping. */
MOZ_ASSERT(zone->isGCMarking()); MOZ_ASSERT(zone->shouldMarkInZone());
zone->setGCState(Zone::Sweep); zone->setGCState(Zone::Sweep);
/* Purge the ArenaLists before sweeping. */ /* Purge the ArenaLists before sweeping. */
@ -5231,7 +5231,7 @@ GCRuntime::resetIncrementalGC(gc::AbortReason reason, AutoLockForExclusiveAccess
ResetGrayList(c); ResetGrayList(c);
for (GCZonesIter zone(rt); !zone.done(); zone.next()) { for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
MOZ_ASSERT(zone->isGCMarking()); MOZ_ASSERT(zone->shouldMarkInZone());
zone->setNeedsIncrementalBarrier(false, Zone::UpdateJit); zone->setNeedsIncrementalBarrier(false, Zone::UpdateJit);
zone->setGCState(Zone::NoGC); zone->setGCState(Zone::NoGC);
} }
@ -5355,7 +5355,7 @@ AutoGCSlice::AutoGCSlice(JSRuntime* rt)
* is expensive) because Ion code doesn't run during GC. If need be, * is expensive) because Ion code doesn't run during GC. If need be,
* we'll update the Ion barriers in ~AutoGCSlice. * we'll update the Ion barriers in ~AutoGCSlice.
*/ */
if (zone->isGCMarking()) { if (zone->shouldMarkInZone()) {
MOZ_ASSERT(zone->needsIncrementalBarrier()); MOZ_ASSERT(zone->needsIncrementalBarrier());
zone->setNeedsIncrementalBarrier(false, Zone::DontUpdateJit); zone->setNeedsIncrementalBarrier(false, Zone::DontUpdateJit);
} else { } else {
@ -5368,7 +5368,7 @@ AutoGCSlice::~AutoGCSlice()
{ {
/* We can't use GCZonesIter if this is the end of the last slice. */ /* We can't use GCZonesIter if this is the end of the last slice. */
for (ZonesIter zone(runtime, WithAtoms); !zone.done(); zone.next()) { for (ZonesIter zone(runtime, WithAtoms); !zone.done(); zone.next()) {
if (zone->isGCMarking()) { if (zone->shouldMarkInZone()) {
zone->setNeedsIncrementalBarrier(true, Zone::UpdateJit); zone->setNeedsIncrementalBarrier(true, Zone::UpdateJit);
zone->arenas.purge(); zone->arenas.purge();
} else { } else {

View file

@ -121,7 +121,7 @@ WeakMapBase::restoreMarkedWeakMaps(WeakMapSet& markedWeakMaps)
{ {
for (WeakMapSet::Range r = markedWeakMaps.all(); !r.empty(); r.popFront()) { for (WeakMapSet::Range r = markedWeakMaps.all(); !r.empty(); r.popFront()) {
WeakMapBase* map = r.front(); WeakMapBase* map = r.front();
MOZ_ASSERT(map->zone->isGCMarking()); MOZ_ASSERT(map->zone->shouldMarkInZone());
MOZ_ASSERT(!map->marked); MOZ_ASSERT(!map->marked);
map->marked = true; map->marked = true;
} }
@ -144,7 +144,7 @@ ObjectValueMap::findZoneEdges()
if (!delegate) if (!delegate)
continue; continue;
Zone* delegateZone = delegate->zone(); Zone* delegateZone = delegate->zone();
if (delegateZone == zone || !delegateZone->isGCMarking()) if (delegateZone == zone || !delegateZone->shouldMarkInZone())
continue; continue;
if (!delegateZone->gcZoneGroupEdges.put(key->zone())) if (!delegateZone->gcZoneGroupEdges.put(key->zone()))
return false; return false;

View file

@ -2989,7 +2989,7 @@ Debugger::markAllIteratively(GCMarker* trc)
* - it actually has hooks that might be called * - it actually has hooks that might be called
*/ */
GCPtrNativeObject& dbgobj = dbg->toJSObjectRef(); GCPtrNativeObject& dbgobj = dbg->toJSObjectRef();
if (!dbgobj->zone()->isGCMarking()) if (!dbgobj->zone()->shouldMarkInZone())
continue; continue;
bool dbgMarked = IsMarked(rt, &dbgobj); bool dbgMarked = IsMarked(rt, &dbgobj);
@ -3139,7 +3139,7 @@ Debugger::findZoneEdges(Zone* zone, js::gc::ZoneComponentFinder& finder)
*/ */
for (Debugger* dbg : zone->runtimeFromMainThread()->debuggerList) { for (Debugger* dbg : zone->runtimeFromMainThread()->debuggerList) {
Zone* w = dbg->object->zone(); Zone* w = dbg->object->zone();
if (w == zone || !w->isGCMarking()) if (w == zone || !w->shouldMarkInZone())
continue; continue;
if (dbg->debuggeeZones.has(zone) || if (dbg->debuggeeZones.has(zone) ||
dbg->scripts.hasKeyInZone(zone) || dbg->scripts.hasKeyInZone(zone) ||