commit e1a772de0e69e8661d5c1294c0994f91aa069176
parent 6502ca57a26a7e0694f4b413453c595de54c03f5
Author: Jon Coppeard <jcoppeard@mozilla.com>
Date: Sat, 1 Nov 2025 11:58:10 +0000
Bug 1994871 - Part 2: Allow symbols and prop maps to be gray r=sfink
This patch does a few things to allow symbols and prop maps (which entrain
symbols) to be marked as gray:
1) Set the flags in TraceKind.h
2) Update the atom marking bitmaps to permit atoms to be marked as gray.
Currently the bitmaps for each zone are refined on collection to compute the
minimum of the previous overaproximation and the actual marking state. This
works when the only allowed states are black and while, but does not work when
one state is gray (see the truth table for js::gc::ColorBit).
To make this work we have to ensure both mark bits are set when a symbol is
marked black. This allows us to carry on using bitwise and to refine the
bitmaps.
3) Update gray unmarking to mark gray atoms black in the atom marking bitmap.
Also, because gray unmarking is handed off to the GC marking in zones that are
currently marking, we need to make GC marking mark atoms in uncollected zone
(because gray unmarking needs to work over all zones).
4) This fixes PropMapChildReadBarrier to unmark gray prop maps. These couldn't
be gray previously.
Differential Revision: https://phabricator.services.mozilla.com/D269955
Diffstat:
16 files changed, 413 insertions(+), 175 deletions(-)
diff --git a/js/public/TraceKind.h b/js/public/TraceKind.h
@@ -96,11 +96,11 @@ struct MapTypeToTraceKind {
D(Script, js::BaseScript, true, true) \
D(Shape, js::Shape, true, false) \
D(String, JSString, false, false) \
- D(Symbol, JS::Symbol, false, false) \
+ D(Symbol, JS::Symbol, true, false) \
D(BigInt, JS::BigInt, false, false) \
D(RegExpShared, js::RegExpShared, true, true) \
D(GetterSetter, js::GetterSetter, true, true) \
- D(PropMap, js::PropMap, false, false)
+ D(PropMap, js::PropMap, true, false)
// clang-format on
// Returns true if the JS::TraceKind is represented as a node in cycle collector
diff --git a/js/src/ds/Bitmap.h b/js/src/ds/Bitmap.h
@@ -9,6 +9,7 @@
#include "mozilla/Array.h"
#include "mozilla/Assertions.h"
+#include "mozilla/Atomics.h"
#include "mozilla/Attributes.h"
#include <algorithm>
@@ -80,6 +81,14 @@ class DenseBitmap {
target[i] |= data[wordStart + i];
}
}
+
+ template <typename F>
+ void forEachWord(size_t wordStart, size_t numWords, F&& func) {
+ MOZ_ASSERT(wordStart + numWords <= data.length());
+ for (size_t i = 0; i < numWords; i++) {
+ func(data[wordStart + i]);
+ }
+ }
};
class SparseBitmap {
@@ -117,8 +126,7 @@ class SparseBitmap {
return p ? p->value() : nullptr;
}
- MOZ_ALWAYS_INLINE const BitBlock* readonlyThreadsafeGetBlock(
- size_t blockId) const {
+ MOZ_ALWAYS_INLINE BitBlock* readonlyThreadsafeGetBlock(size_t blockId) const {
Data::Ptr p = data.readonlyThreadsafeLookup(blockId);
return p ? p->value() : nullptr;
}
@@ -147,6 +155,15 @@ class SparseBitmap {
return true;
}
+ void atomicSetExistingBit(size_t bit) {
+ size_t word = bit / JS_BITS_PER_WORD;
+ size_t blockWord = blockStartWord(word);
+ BitBlock* block = readonlyThreadsafeGetBlock(blockWord / WordsInBlock);
+ MOZ_ASSERT(block);
+ uintptr_t* ptr = &(*block)[word - blockWord];
+ __atomic_fetch_or(ptr, bitMask(bit), __ATOMIC_RELAXED);
+ }
+
bool getBit(size_t bit) const;
bool readonlyThreadsafeGetBit(size_t bit) const;
diff --git a/js/src/gc/AtomMarking-inl.h b/js/src/gc/AtomMarking-inl.h
@@ -23,7 +23,8 @@
namespace js {
namespace gc {
-inline size_t GetAtomBit(TenuredCell* thing) {
+/* static */
+inline size_t AtomMarkingRuntime::getAtomBit(TenuredCell* thing) {
MOZ_ASSERT(thing->zoneFromAnyThread()->isAtomsZone());
Arena* arena = thing->arena();
size_t arenaBit = (reinterpret_cast<uintptr_t>(thing) - arena->address()) /
@@ -32,13 +33,13 @@ inline size_t GetAtomBit(TenuredCell* thing) {
}
template <typename T, bool Fallible>
-MOZ_ALWAYS_INLINE bool AtomMarkingRuntime::inlinedMarkAtomInternal(
- JSContext* cx, T* thing) {
+MOZ_ALWAYS_INLINE bool AtomMarkingRuntime::inlinedMarkAtomInternal(Zone* zone,
+ T* thing) {
static_assert(std::is_same_v<T, JSAtom> || std::is_same_v<T, JS::Symbol>,
"Should only be called with JSAtom* or JS::Symbol* argument");
- MOZ_ASSERT(cx->zone());
- MOZ_ASSERT(!cx->zone()->isAtomsZone());
+ MOZ_ASSERT(zone);
+ MOZ_ASSERT(!zone->isAtomsZone());
MOZ_ASSERT(thing);
js::gc::TenuredCell* cell = &thing->asTenured();
@@ -54,8 +55,10 @@ MOZ_ALWAYS_INLINE bool AtomMarkingRuntime::inlinedMarkAtomInternal(
}
}
- size_t bit = GetAtomBit(cell);
- MOZ_ASSERT(bit / JS_BITS_PER_WORD < allocatedWords);
+ size_t bit = getAtomBit(cell);
+ size_t blackBit = bit + size_t(ColorBit::BlackBit);
+ size_t grayOrBlackBit = bit + size_t(ColorBit::GrayOrBlackBit);
+ MOZ_ASSERT(grayOrBlackBit / JS_BITS_PER_WORD < allocatedWords);
{
mozilla::Maybe<AutoEnterOOMUnsafeRegion> oomUnsafe;
@@ -63,7 +66,10 @@ MOZ_ALWAYS_INLINE bool AtomMarkingRuntime::inlinedMarkAtomInternal(
oomUnsafe.emplace();
}
- bool ok = cx->zone()->markedAtoms().setBit(bit);
+ bool ok = zone->markedAtoms().setBit(blackBit);
+ if constexpr (std::is_same_v<T, JS::Symbol>) {
+ ok = ok && zone->markedAtoms().setBit(grayOrBlackBit);
+ }
if (!ok) {
if constexpr (!Fallible) {
@@ -74,21 +80,37 @@ MOZ_ALWAYS_INLINE bool AtomMarkingRuntime::inlinedMarkAtomInternal(
}
}
- // Trigger a read barrier on the atom, in case there is an incremental
- // GC in progress. This is necessary if the atom is being marked
- // because a reference to it was obtained from another zone which is
- // not being collected by the incremental GC.
- ReadBarrier(thing);
-
// Children of the thing also need to be marked in the context's zone.
// We don't have a JSTracer for this so manually handle the cases in which
// an atom can reference other atoms.
- markChildren(cx, thing);
+ markChildren(zone, thing);
return true;
}
-inline bool GCRuntime::isSymbolReferencedByUncollectedZone(JS::Symbol* sym) {
+inline void AtomMarkingRuntime::maybeUnmarkGrayAtomically(Zone* zone,
+ JS::Symbol* symbol) {
+ MOZ_ASSERT(zone);
+ MOZ_ASSERT(!zone->isAtomsZone());
+ MOZ_ASSERT(symbol);
+ MOZ_ASSERT(symbol->zone()->isAtomsZone());
+
+ if (symbol->isPermanentAndMayBeShared()) {
+ return;
+ }
+
+ MOZ_ASSERT(atomIsMarked(zone, symbol));
+
+ size_t blackBit = getAtomBit(symbol) + size_t(ColorBit::BlackBit);
+ MOZ_ASSERT(blackBit / JS_BITS_PER_WORD < allocatedWords);
+
+ zone->markedAtoms().atomicSetExistingBit(blackBit);
+
+ MOZ_ASSERT(getAtomMarkColor(zone, symbol) == CellColor::Black);
+}
+
+inline bool GCRuntime::isSymbolReferencedByUncollectedZone(JS::Symbol* sym,
+ MarkColor color) {
MOZ_ASSERT(sym->zone()->isAtomsZone());
if (!atomsUsedByUncollectedZones.ref()) {
@@ -97,35 +119,41 @@ inline bool GCRuntime::isSymbolReferencedByUncollectedZone(JS::Symbol* sym) {
MOZ_ASSERT(atomsZone()->wasGCStarted());
- size_t bit = GetAtomBit(sym);
- MOZ_ASSERT(bit / JS_BITS_PER_WORD < atomMarking.allocatedWords);
+ size_t bit = AtomMarkingRuntime::getAtomBit(sym);
+ size_t blackBit = bit + size_t(ColorBit::BlackBit);
+ size_t grayOrBlackBit = bit + size_t(ColorBit::GrayOrBlackBit);
+ MOZ_ASSERT(grayOrBlackBit / JS_BITS_PER_WORD < atomMarking.allocatedWords);
const DenseBitmap& bitmap = *atomsUsedByUncollectedZones.ref();
- if (bit >= bitmap.count()) {
+ if (grayOrBlackBit >= bitmap.count()) {
return false; // Atom created during collection.
}
- return bitmap.getBit(bit);
+ if (bitmap.getBit(blackBit)) {
+ return true;
+ }
+
+ return color == MarkColor::Gray && bitmap.getBit(grayOrBlackBit);
}
-void AtomMarkingRuntime::markChildren(JSContext* cx, JSAtom*) {}
+void AtomMarkingRuntime::markChildren(Zone* zone, JSAtom*) {}
-void AtomMarkingRuntime::markChildren(JSContext* cx, JS::Symbol* symbol) {
+void AtomMarkingRuntime::markChildren(Zone* zone, JS::Symbol* symbol) {
if (JSAtom* description = symbol->description()) {
- markAtom(cx, description);
+ inlinedMarkAtom(zone, description);
}
}
template <typename T>
-MOZ_ALWAYS_INLINE void AtomMarkingRuntime::inlinedMarkAtom(JSContext* cx,
+MOZ_ALWAYS_INLINE void AtomMarkingRuntime::inlinedMarkAtom(Zone* zone,
T* thing) {
- MOZ_ALWAYS_TRUE((inlinedMarkAtomInternal<T, false>(cx, thing)));
+ MOZ_ALWAYS_TRUE((inlinedMarkAtomInternal<T, false>(zone, thing)));
}
template <typename T>
-MOZ_ALWAYS_INLINE bool AtomMarkingRuntime::inlinedMarkAtomFallible(
- JSContext* cx, T* thing) {
- return inlinedMarkAtomInternal<T, true>(cx, thing);
+MOZ_ALWAYS_INLINE bool AtomMarkingRuntime::inlinedMarkAtomFallible(Zone* zone,
+ T* thing) {
+ return inlinedMarkAtomInternal<T, true>(zone, thing);
}
} // namespace gc
diff --git a/js/src/gc/AtomMarking.cpp b/js/src/gc/AtomMarking.cpp
@@ -20,33 +20,42 @@ namespace gc {
// [SMDOC] GC Atom Marking
//
-// Things in the atoms zone (which includes atomized strings and other things,
-// all of which we will refer to as 'atoms' here) may be pointed to freely by
-// things in other zones. To avoid the need to perform garbage collections of
-// the entire runtime to collect atoms, we compute a separate atom mark bitmap
-// for each zone that is always an overapproximation of the atoms that zone is
-// using. When an atom is not in the mark bitmap for any zone, it can be
-// destroyed.
+// Things in the atoms zone (which includes atomized strings, symbols, and other
+// things, all of which we will refer to as 'atoms' here) may be pointed to
+// freely by things in other zones. To avoid the need to perform garbage
+// collections of the entire runtime to collect atoms, we compute a separate
+// atom mark bitmap for each zone that is always an overapproximation of the
+// atoms that zone is using. When an atom is not in the mark bitmap for any
+// zone, it can be destroyed.
+//
+// (These bitmaps can only be calculated exactly if we collect a single zone
+// since they are based on the marking state at the end of a GC which may have
+// marked multiple zones.)
//
// To minimize interference with the rest of the GC, atom marking and sweeping
// is done by manipulating the mark bitmaps in the chunks used for the atoms.
// When the atoms zone is being collected, the mark bitmaps for the chunk(s)
-// used by the atoms are updated normally during marking. After marking
-// finishes, the chunk mark bitmaps are translated to a more efficient atom mark
-// bitmap (see below) that is stored on the zones which the GC collected
-// (computeBitmapFromChunkMarkBits). Before sweeping begins, the chunk mark
-// bitmaps are updated with any atoms that might be referenced by zones which
-// weren't collected (markAtomsUsedByUncollectedZones). The GC sweeping will
-// then release all atoms which are not marked by any zone.
+// used by the atoms are updated normally during marking.
+//
+// After marking has finished and before sweeping begins, two things happen:
+//
+// 1) The atom marking bitmaps for collected zones are updated to remove atoms
+// that GC marking has found are not referenced by any collected zone (see
+// refineZoneBitmapsForCollectedZones). This improves our approximation.
+//
+// 2) The chunk mark bitmaps are updated with any atoms that might be
+// referenced by zones which weren't collected (see
+// markAtomsUsedByUncollectedZones).
+//
+// GC sweeping will then release all atoms which are not marked by any zone.
//
// The representation of atom mark bitmaps is as follows:
//
// Each arena in the atoms zone has an atomBitmapStart() value indicating the
// word index into the bitmap of the first thing in the arena. Each arena uses
// ArenaBitmapWords of data to store its bitmap, which uses the same
-// representation as chunk mark bitmaps: one bit is allocated per Cell, with
-// bits for space between things being unused when things are larger than a
-// single Cell.
+// representation as chunk mark bitmaps: at least two bits per cell (see
+// CellBytesPerMarkBit and MarkBitsPerCell).
size_t AtomMarkingRuntime::allocateIndex(GCRuntime* gc) {
// We need to find a range of bits from the atoms bitmap for this arena.
@@ -105,43 +114,102 @@ void AtomMarkingRuntime::mergePendingFreeArenaIndexes(GCRuntime* gc) {
pendingFreeArenaIndexes.ref().clear();
}
-void AtomMarkingRuntime::refineZoneBitmapsForCollectedZones(GCRuntime* gc) {
- size_t collectedZones = 0;
- for (ZonesIter zone(gc, SkipAtoms); !zone.done(); zone.next()) {
- if (zone->isCollecting()) {
- collectedZones++;
+// Return whether more than one zone is being collected.
+static bool MultipleNonAtomZonesAreBeingCollected(GCRuntime* gc) {
+ size_t count = 0;
+ for (GCZonesIter zone(gc); !zone.done(); zone.next()) {
+ if (!zone->isAtomsZone()) {
+ count++;
+ if (count == 2) {
+ return true;
+ }
}
}
- // If there is more than one zone to update, copy the chunk mark bits into a
- // bitmap and AND that into the atom marking bitmap for each zone.
+ return false;
+}
+
+void AtomMarkingRuntime::refineZoneBitmapsForCollectedZones(GCRuntime* gc) {
+ // If there is more than one zone to update, it's more efficient to copy the
+ // chunk mark bits from each arena into a single dense bitmap and then use
+ // that to refine the atom marking bitmap for each zone.
DenseBitmap marked;
- if (collectedZones > 1 && computeBitmapFromChunkMarkBits(gc, marked)) {
+ if (MultipleNonAtomZonesAreBeingCollected(gc) &&
+ computeBitmapFromChunkMarkBits(gc, marked)) {
for (GCZonesIter zone(gc); !zone.done(); zone.next()) {
- refineZoneBitmapForCollectedZone(zone, marked);
+ if (!zone->isAtomsZone()) {
+ refineZoneBitmapForCollectedZone(zone, marked);
+ }
}
return;
}
- // If there's only one zone (or on OOM), AND the mark bits for each arena into
- // the zones' atom marking bitmaps directly.
+ // If there's only one zone (or on OOM), refine the mark bits for each arena
+ // with the zones' atom marking bitmaps directly.
for (GCZonesIter zone(gc); !zone.done(); zone.next()) {
- if (zone->isAtomsZone()) {
- continue;
+ if (!zone->isAtomsZone()) {
+ for (auto thingKind : AllAllocKinds()) {
+ for (ArenaIterInGC aiter(gc->atomsZone(), thingKind); !aiter.done();
+ aiter.next()) {
+ refineZoneBitmapForCollectedZone(zone, aiter);
+ }
+ }
}
+ }
+}
- for (auto thingKind : AllAllocKinds()) {
- for (ArenaIterInGC aiter(gc->atomsZone(), thingKind); !aiter.done();
- aiter.next()) {
- Arena* arena = aiter.get();
- AtomicBitmapWord* chunkWords =
- arena->chunk()->markBits.arenaBits(arena);
- zone->markedAtoms().bitwiseAndRangeWith(arena->atomBitmapStart(),
- ArenaBitmapWords, chunkWords);
- }
+// Refining atom marking bitmaps:
+//
+// The atom marking bitmap for a zone records an overapproximation of the mark
+// color for each atom referenced by that zone. After collection we refine this
+// based on the actual final mark state. The final mark state is the maximum of
+// the mark colours of all references for each atom. Therefore we refine the
+// bitmap by setting it to the minimum of itself and the actual mark state for
+// each atom.
+//
+// To find the minimum we use bitwise AND. For trace kinds that can only be
+// marked black this works on its own. For kinds that can be marked gray we must
+// preprocess the mark bitmap so that both mark bits are set for black cells.
+
+// Mask of bit positions in a mark bitmap word that can be black bits, which is
+// every other bit.
+#if JS_BITS_PER_WORD == 32
+static constexpr uintptr_t BlackBitMask = 0x55555555;
+#else
+static constexpr uintptr_t BlackBitMask = 0x5555555555555555;
+#endif
+
+static void PropagateBlackBitsToGrayOrBlackBits(DenseBitmap& bitmap,
+ Arena* arena) {
+ // This only works if the gray bit and black bits are in the same word,
+ // which is true for symbols.
+
+ MOZ_ASSERT(
+ TraceKindCanBeMarkedGray(MapAllocToTraceKind(arena->getAllocKind())));
+ MOZ_ASSERT((arena->getThingSize() / CellBytesPerMarkBit) % 2 == 0);
+
+ bitmap.forEachWord(
+ arena->atomBitmapStart(), ArenaBitmapWords,
+ [](uintptr_t& word) { word |= (word & BlackBitMask) << 1; });
+}
+
+static void PropagateBlackBitsToGrayOrBlackBits(
+ uintptr_t (&words)[ArenaBitmapWords]) {
+ for (size_t i = 0; i < ArenaBitmapWords; i++) {
+ words[i] |= (words[i] & BlackBitMask) << 1;
+ }
+}
+
+#ifdef DEBUG
+static bool ArenaContainsGrayCells(Arena* arena) {
+ for (ArenaCellIter cell(arena); !cell.done(); cell.next()) {
+ if (cell->isMarkedGray()) {
+ return true;
}
}
+ return false;
}
+#endif
bool AtomMarkingRuntime::computeBitmapFromChunkMarkBits(GCRuntime* gc,
DenseBitmap& bitmap) {
@@ -159,6 +227,16 @@ bool AtomMarkingRuntime::computeBitmapFromChunkMarkBits(GCRuntime* gc,
AtomicBitmapWord* chunkWords = arena->chunk()->markBits.arenaBits(arena);
bitmap.copyBitsFrom(arena->atomBitmapStart(), ArenaBitmapWords,
chunkWords);
+
+ if (thingKind == AllocKind::JITCODE) {
+ // The atoms zone can contain JitCode used for compiled self-hosted JS,
+ // however these cells are never marked gray so we can skip this step.
+ MOZ_ASSERT(!ArenaContainsGrayCells(arena));
+ } else if (TraceKindCanBeMarkedGray(MapAllocToTraceKind(thingKind))) {
+ // Ensure both mark bits are set for black cells so we can compute the
+ // minimum of each mark color by bitwise AND.
+ PropagateBlackBitsToGrayOrBlackBits(bitmap, arena);
+ }
}
}
@@ -168,10 +246,7 @@ bool AtomMarkingRuntime::computeBitmapFromChunkMarkBits(GCRuntime* gc,
void AtomMarkingRuntime::refineZoneBitmapForCollectedZone(
Zone* zone, const DenseBitmap& bitmap) {
MOZ_ASSERT(zone->isCollectingFromAnyThread());
-
- if (zone->isAtomsZone()) {
- return;
- }
+ MOZ_ASSERT(!zone->isAtomsZone());
// Take the bitwise and between the two mark bitmaps to get the best new
// overapproximation we can. |bitmap| might include bits that are not in
@@ -179,6 +254,31 @@ void AtomMarkingRuntime::refineZoneBitmapForCollectedZone(
zone->markedAtoms().bitwiseAndWith(bitmap);
}
+void AtomMarkingRuntime::refineZoneBitmapForCollectedZone(Zone* zone,
+ Arena* arena) {
+ MOZ_ASSERT(zone->isCollectingFromAnyThread());
+ MOZ_ASSERT(!zone->isAtomsZone());
+
+ AtomicBitmapWord* chunkWords = arena->chunk()->markBits.arenaBits(arena);
+
+ AllocKind kind = arena->getAllocKind();
+ if (kind == AllocKind::JITCODE) {
+ // The atoms zone can contain JitCode used for compiled self-hosted JS,
+ // however these cells are never marked gray so we can skip this step.
+ MOZ_ASSERT(!ArenaContainsGrayCells(arena));
+ } else if (TraceKindCanBeMarkedGray(MapAllocToTraceKind(kind))) {
+ uintptr_t words[ArenaBitmapWords];
+ memcpy(words, chunkWords, sizeof(words));
+ PropagateBlackBitsToGrayOrBlackBits(words);
+ zone->markedAtoms().bitwiseAndRangeWith(arena->atomBitmapStart(),
+ ArenaBitmapWords, words);
+ return;
+ }
+
+ zone->markedAtoms().bitwiseAndRangeWith(arena->atomBitmapStart(),
+ ArenaBitmapWords, chunkWords);
+}
+
// Set any bits in the chunk mark bitmaps for atoms which are marked in bitmap.
template <typename Bitmap>
static void BitwiseOrIntoChunkMarkBits(Zone* atomsZone, Bitmap& bitmap) {
@@ -229,7 +329,13 @@ void AtomMarkingRuntime::markAtomsUsedByUncollectedZones(
template <typename T>
void AtomMarkingRuntime::markAtom(JSContext* cx, T* thing) {
- return inlinedMarkAtom(cx, thing);
+ // Trigger a read barrier on the atom, in case there is an incremental
+ // GC in progress. This is necessary if the atom is being marked
+ // because a reference to it was obtained from another zone which is
+ // not being collected by the incremental GC.
+ ReadBarrier(thing);
+
+ return inlinedMarkAtom(cx->zone(), thing);
}
template void AtomMarkingRuntime::markAtom(JSContext* cx, JSAtom* thing);
@@ -264,7 +370,7 @@ void AtomMarkingRuntime::markAtomValue(JSContext* cx, const Value& value) {
}
template <typename T>
-bool AtomMarkingRuntime::atomIsMarked(Zone* zone, T* thing) {
+CellColor AtomMarkingRuntime::getAtomMarkColor(Zone* zone, T* thing) {
static_assert(std::is_same_v<T, JSAtom> || std::is_same_v<T, JS::Symbol>,
"Should only be called with JSAtom* or JS::Symbol* argument");
@@ -273,47 +379,89 @@ bool AtomMarkingRuntime::atomIsMarked(Zone* zone, T* thing) {
MOZ_ASSERT(thing->zoneFromAnyThread()->isAtomsZone());
if (!zone->runtimeFromAnyThread()->permanentAtomsPopulated()) {
- return true;
+ return CellColor::Black;
}
if (thing->isPermanentAndMayBeShared()) {
- return true;
+ return CellColor::Black;
}
if constexpr (std::is_same_v<T, JSAtom>) {
if (thing->isPinned()) {
- return true;
+ return CellColor::Black;
+ }
+ }
+
+ size_t bit = getAtomBit(&thing->asTenured());
+
+ size_t blackBit = bit + size_t(ColorBit::BlackBit);
+ size_t grayOrBlackBit = bit + size_t(ColorBit::GrayOrBlackBit);
+
+ SparseBitmap& bitmap = zone->markedAtoms();
+
+ MOZ_ASSERT_IF((std::is_same_v<T, JSAtom>),
+ !bitmap.readonlyThreadsafeGetBit(grayOrBlackBit));
+ MOZ_ASSERT_IF((std::is_same_v<T, JS::Symbol>) &&
+ bitmap.readonlyThreadsafeGetBit(blackBit),
+ bitmap.readonlyThreadsafeGetBit(grayOrBlackBit));
+
+ if (bitmap.readonlyThreadsafeGetBit(blackBit)) {
+ return CellColor::Black;
+ }
+
+ if constexpr (std::is_same_v<T, JS::Symbol>) {
+ if (bitmap.readonlyThreadsafeGetBit(grayOrBlackBit)) {
+ return CellColor::Gray;
}
}
- size_t bit = GetAtomBit(&thing->asTenured());
- return zone->markedAtoms().readonlyThreadsafeGetBit(bit);
+ return CellColor::White;
}
-template bool AtomMarkingRuntime::atomIsMarked(Zone* zone, JSAtom* thing);
-template bool AtomMarkingRuntime::atomIsMarked(Zone* zone, JS::Symbol* thing);
+template CellColor AtomMarkingRuntime::getAtomMarkColor(Zone* zone,
+ JSAtom* thing);
+template CellColor AtomMarkingRuntime::getAtomMarkColor(Zone* zone,
+ JS::Symbol* thing);
+
+CellColor AtomMarkingRuntime::getAtomMarkColorForIndex(Zone* zone,
+ size_t bitIndex) {
+ MOZ_ASSERT(zone->runtimeFromAnyThread()->permanentAtomsPopulated());
+
+ size_t blackBit = bitIndex + size_t(ColorBit::BlackBit);
+ size_t grayOrBlackBit = bitIndex + size_t(ColorBit::GrayOrBlackBit);
+
+ SparseBitmap& bitmap = zone->markedAtoms();
+ bool blackBitSet = bitmap.readonlyThreadsafeGetBit(blackBit);
+ bool grayOrBlackBitSet = bitmap.readonlyThreadsafeGetBit(grayOrBlackBit);
+
+ if (blackBitSet) {
+ return CellColor::Black;
+ }
+
+ if (grayOrBlackBitSet) {
+ return CellColor::Gray;
+ }
+
+ return CellColor::White;
+}
#ifdef DEBUG
template <>
-bool AtomMarkingRuntime::atomIsMarked(Zone* zone, TenuredCell* thing) {
- if (!thing) {
- return true;
- }
+CellColor AtomMarkingRuntime::getAtomMarkColor(Zone* zone, TenuredCell* thing) {
+ MOZ_ASSERT(thing);
+ MOZ_ASSERT(thing->zoneFromAnyThread()->isAtomsZone());
if (thing->is<JSString>()) {
JSString* str = thing->as<JSString>();
- if (!str->isAtom()) {
- return true;
- }
- return atomIsMarked(zone, &str->asAtom());
+ return getAtomMarkColor(zone, &str->asAtom());
}
if (thing->is<JS::Symbol>()) {
- return atomIsMarked(zone, thing->as<JS::Symbol>());
+ return getAtomMarkColor(zone, thing->as<JS::Symbol>());
}
- return true;
+ MOZ_CRASH("Unexpected atom kind");
}
bool AtomMarkingRuntime::idIsMarked(Zone* zone, jsid id) {
diff --git a/js/src/gc/AtomMarking.h b/js/src/gc/AtomMarking.h
@@ -10,6 +10,7 @@
#include "mozilla/Atomics.h"
#include "NamespaceImports.h"
+#include "gc/Cell.h"
#include "js/Vector.h"
#include "threading/ProtectedData.h"
@@ -33,8 +34,8 @@ class AtomMarkingRuntime {
js::GCLockData<Vector<size_t, 0, SystemAllocPolicy>> pendingFreeArenaIndexes;
mozilla::Atomic<bool, mozilla::Relaxed> hasPendingFreeArenaIndexes;
- inline void markChildren(JSContext* cx, JSAtom*);
- inline void markChildren(JSContext* cx, JS::Symbol* symbol);
+ inline void markChildren(Zone* zone, JSAtom*);
+ inline void markChildren(Zone* zone, JS::Symbol* symbol);
public:
// The extent of all allocated and free words in atom mark bitmaps.
@@ -64,16 +65,24 @@ class AtomMarkingRuntime {
void markAtomsUsedByUncollectedZones(GCRuntime* gc,
UniquePtr<DenseBitmap> markedUnion);
+ // Get the index into the atom marking bitmaps for the first bit associated
+ // with an atom.
+ // This is public for testing access.
+ static size_t getAtomBit(TenuredCell* thing);
+
private:
// Fill |bitmap| with an atom marking bitmap based on the things that are
// currently marked in the chunks used by atoms zone arenas. This returns
// false on an allocation failure (but does not report an exception).
bool computeBitmapFromChunkMarkBits(GCRuntime* gc, DenseBitmap& bitmap);
- // Update the atom marking bitmap in |zone| according to another
- // overapproximation of the reachable atoms in |bitmap|.
+ // Update the overapproximation of the reachable atoms in |zone| in one go
+ // according to the marking state after GC.
void refineZoneBitmapForCollectedZone(Zone* zone, const DenseBitmap& bitmap);
+ // As above but called per-arena and using the mark bits directly.
+ void refineZoneBitmapForCollectedZone(Zone* zone, Arena* arena);
+
public:
// Mark an atom or id as being newly reachable by the context's zone.
template <typename T>
@@ -82,18 +91,32 @@ class AtomMarkingRuntime {
// Version of markAtom that's always inlined, for performance-sensitive
// callers.
template <typename T, bool Fallible>
- MOZ_ALWAYS_INLINE bool inlinedMarkAtomInternal(JSContext* cx, T* thing);
+ MOZ_ALWAYS_INLINE bool inlinedMarkAtomInternal(Zone* zone, T* thing);
template <typename T>
- MOZ_ALWAYS_INLINE void inlinedMarkAtom(JSContext* cx, T* thing);
+ MOZ_ALWAYS_INLINE void inlinedMarkAtom(Zone* zone, T* thing);
template <typename T>
- MOZ_ALWAYS_INLINE bool inlinedMarkAtomFallible(JSContext* cx, T* thing);
+ [[nodiscard]] MOZ_ALWAYS_INLINE bool inlinedMarkAtomFallible(Zone* zone,
+ T* thing);
void markId(JSContext* cx, jsid id);
void markAtomValue(JSContext* cx, const Value& value);
+ // Get the mark color of |thing| in the atom marking bitmap for |zone|.
+ template <typename T>
+ CellColor getAtomMarkColor(Zone* zone, T* thing);
+
// Return whether |thing/id| is in the atom marking bitmap for |zone|.
template <typename T>
- bool atomIsMarked(Zone* zone, T* thing);
+ bool atomIsMarked(Zone* zone, T* thing) {
+ return getAtomMarkColor(zone, thing) != CellColor::White;
+ }
+
+ // For testing purposes, get the mark color associated with |bitIndex| in the
+ // atom marking bitmap for |zone|.
+ CellColor getAtomMarkColorForIndex(Zone* zone, size_t bitIndex);
+
+ // Called during (possibly parallel) marking to unmark possibly-gray symbols.
+ void maybeUnmarkGrayAtomically(Zone* zone, JS::Symbol* symbol);
#ifdef DEBUG
bool idIsMarked(Zone* zone, jsid id);
diff --git a/js/src/gc/GCMarker.h b/js/src/gc/GCMarker.h
@@ -565,9 +565,9 @@ class GCMarker {
friend class JS::Zone;
#ifdef DEBUG
- void checkZone(void* p);
+ void checkZone(gc::Cell* cell);
#else
- void checkZone(void* p) {}
+ void checkZone(gc::Cell* cell) {}
#endif
template <uint32_t markingOptions>
diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h
@@ -687,7 +687,9 @@ class GCRuntime {
static void* refillFreeList(JS::Zone* zone, AllocKind thingKind);
void attemptLastDitchGC();
- bool isSymbolReferencedByUncollectedZone(JS::Symbol* sym);
+ // Return whether |sym| is marked at least |color| in the atom marking state
+ // for uncollected zones.
+ bool isSymbolReferencedByUncollectedZone(JS::Symbol* sym, MarkColor color);
// Test mark queue.
#ifdef DEBUG
diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp
@@ -344,14 +344,12 @@ void js::gc::AssertShouldMarkInZone(GCMarker* marker, T* thing) {
return;
}
- // Allow marking marking atoms if we're not collected the atoms zone, except
- // for symbols which may entrain other GC things if they're used as weakmap
- // keys.
- bool allowAtoms = !std::is_same_v<T, JS::Symbol>;
-
+ // This allows marking atoms even if we're not collecting the atoms zone. This
+ // is necessary to unmark gray atoms during an incremental GC. Failing to do
+ // this will break our invariant about not having black to gray edges.
Zone* zone = thing->zone();
MOZ_ASSERT(zone->shouldMarkInZone(marker->markColor()) ||
- (allowAtoms && zone->isAtomsZone()));
+ zone->isAtomsZone());
}
void js::gc::AssertRootMarkingPhase(JSTracer* trc) {
@@ -786,7 +784,7 @@ void GCMarker::markImplicitEdges(T* markedThing) {
}
Zone* zone = markedThing->asTenured().zone();
- MOZ_ASSERT(zone->isGCMarking());
+ MOZ_ASSERT(zone->isGCMarking() || zone->isAtomsZone());
MOZ_ASSERT(!zone->isGCSweeping());
auto& ephemeronTable = zone->gcEphemeronEdges();
@@ -1145,8 +1143,10 @@ inline void GCMarker::checkTraversedEdge(S source, T* target) {
// atom bitmap.
if (checkAtomMarking && !sourceZone->isAtomsZone() &&
targetZone->isAtomsZone()) {
- MOZ_ASSERT(target->runtimeFromAnyThread()->gc.atomMarking.atomIsMarked(
- sourceZone, reinterpret_cast<TenuredCell*>(target)));
+ GCRuntime* gc = &target->runtimeFromAnyThread()->gc;
+ TenuredCell* atom = &target->asTenured();
+ MOZ_ASSERT(gc->atomMarking.getAtomMarkColor(sourceZone, atom) >=
+ AsCellColor(markColor()));
}
// If we have access to a compartment pointer for both things, they must
@@ -1158,6 +1158,12 @@ inline void GCMarker::checkTraversedEdge(S source, T* target) {
template <uint32_t opts, typename S, typename T>
void js::GCMarker::markAndTraverseEdge(S* source, T* target) {
+ if constexpr (std::is_same_v<T, JS::Symbol>) {
+ // Unmark gray symbols in incremental GC.
+ GCRuntime* gc = &runtime()->gc;
+ MOZ_ASSERT(gc->atomMarking.atomIsMarked(source->zone(), target));
+ gc->atomMarking.maybeUnmarkGrayAtomically(source->zone(), target);
+ }
checkTraversedEdge(source, target);
markAndTraverse<opts>(target);
}
@@ -1205,10 +1211,8 @@ bool js::GCMarker::mark(T* thing) {
return false;
}
- // Don't mark symbols if we're not collecting the atoms zone.
if constexpr (std::is_same_v<T, JS::Symbol>) {
- if (IsOwnedByOtherRuntime(runtime(), thing) ||
- !thing->zone()->isGCMarkingOrVerifyingPreBarriers()) {
+ if (IsOwnedByOtherRuntime(runtime(), thing)) {
return false;
}
}
@@ -2600,11 +2604,13 @@ inline void GCRuntime::forEachDelayedMarkingArena(F&& f) {
}
#ifdef DEBUG
-void GCMarker::checkZone(void* p) {
+void GCMarker::checkZone(Cell* cell) {
MOZ_ASSERT(state != NotActive);
- DebugOnly<Cell*> cell = static_cast<Cell*>(p);
- MOZ_ASSERT_IF(cell->isTenured(),
- cell->asTenured().zone()->isCollectingFromAnyThread());
+ if (cell->isTenured()) {
+ Zone* zone = cell->asTenured().zone();
+ MOZ_ASSERT(zone->isGCMarkingOrVerifyingPreBarriers() ||
+ zone->isAtomsZone());
+ }
}
#endif
@@ -2871,6 +2877,9 @@ class js::gc::UnmarkGrayTracer final : public JS::CallbackTracer {
// marked.
GCMarker* marker;
+ // The source of edges traversed by onChild.
+ Zone* sourceZone;
+
// Stack of cells to traverse.
Vector<JS::GCCellPtr, 0, SystemAllocPolicy>& stack;
@@ -2892,7 +2901,7 @@ void UnmarkGrayTracer::onChild(JS::GCCellPtr thing, const char* name) {
}
TenuredCell& tenured = cell->asTenured();
- Zone* zone = tenured.zone();
+ Zone* zone = tenured.zoneFromAnyThread();
// If the cell is in a zone whose mark bits are being cleared, then it will
// end up white.
@@ -2900,30 +2909,33 @@ void UnmarkGrayTracer::onChild(JS::GCCellPtr thing, const char* name) {
return;
}
- // If the cell is in a zone that we're currently marking, then it's possible
- // that it is currently white but will end up gray. To handle this case,
- // trigger the barrier for any cells in zones that are currently being
- // marked. This will ensure they will eventually get marked black.
+ // If the cell is already marked black then there's nothing more to do.
+ if (tenured.isMarkedBlack()) {
+ return;
+ }
+
if (zone->isGCMarking()) {
- if (!cell->isMarkedBlack()) {
- TraceEdgeForBarrier(marker, &tenured, thing.kind());
- unmarkedAny = true;
+ // If the cell is in a zone that we're currently marking, then it's possible
+ // that it is currently white but will end up gray. To handle this case,
+ // trigger the barrier for any cells in zones that are currently being
+ // marked. This will ensure they will eventually get marked black.
+ TraceEdgeForBarrier(marker, &tenured, thing.kind());
+ } else if (tenured.isMarkedGray()) {
+ // TODO: It may be a small improvement to only use the atomic version during
+ // parallel marking.
+ tenured.markBlackAtomic();
+ if (!stack.append(thing)) {
+ oom = true;
}
- return;
}
- if (!tenured.isMarkedGray()) {
- return;
+ if (zone->isAtomsZone() && sourceZone) {
+ MOZ_ASSERT(tenured.is<JS::Symbol>());
+ runtime()->gc.atomMarking.maybeUnmarkGrayAtomically(
+ sourceZone, tenured.as<JS::Symbol>());
}
- // TODO: It may be a small improvement to only use the atomic version during
- // parallel marking.
- tenured.markBlackAtomic();
unmarkedAny = true;
-
- if (!stack.append(thing)) {
- oom = true;
- }
}
void UnmarkGrayTracer::unmark(JS::GCCellPtr cell) {
@@ -2933,10 +2945,13 @@ void UnmarkGrayTracer::unmark(JS::GCCellPtr cell) {
// invalid. However an early return here causes ExposeGCThingToActiveJS to
// fail because it asserts that something gets unmarked.
+ sourceZone = nullptr;
onChild(cell, "unmarking root");
while (!stack.empty() && !oom) {
- TraceChildren(this, stack.popCopy());
+ JS::GCCellPtr thing = stack.popCopy();
+ sourceZone = thing.asCell()->zone();
+ TraceChildren(this, thing);
}
if (oom) {
@@ -2979,10 +2994,6 @@ void js::gc::UnmarkGrayGCThingRecursively(TenuredCell* cell) {
JS::UnmarkGrayGCThingRecursively(JS::GCCellPtr(cell, cell->getTraceKind()));
}
-bool js::UnmarkGrayShapeRecursively(Shape* shape) {
- return JS::UnmarkGrayGCThingRecursively(JS::GCCellPtr(shape));
-}
-
#ifdef DEBUG
Cell* js::gc::UninlinedForwarded(const Cell* cell) { return Forwarded(cell); }
#endif
diff --git a/js/src/gc/Marking.h b/js/src/gc/Marking.h
@@ -94,9 +94,6 @@ bool UnmarkGrayGCThingUnchecked(GCMarker* marker, JS::GCCellPtr thing);
} /* namespace gc */
-// The return value indicates if anything was unmarked.
-bool UnmarkGrayShapeRecursively(Shape* shape);
-
namespace gc {
// Functions for checking and updating GC thing pointers that might have been
diff --git a/js/src/gc/Sweeping.cpp b/js/src/gc/Sweeping.cpp
@@ -2571,14 +2571,12 @@ bool GCRuntime::allCCVisibleZonesWereCollected() {
// The gray bits change from invalid to valid if we finished a full GC from
// the point of view of the cycle collector. We ignore the following:
//
- // - Helper thread zones, as these are not reachable from the main heap.
- // - The atoms zone, since strings and symbols are never marked gray.
// - Empty zones.
//
- // These exceptions ensure that when the CC requests a full GC the gray mark
- // state ends up valid even it we don't collect all of the zones.
+ // This exception ensures that when the CC requests a full GC the gray mark
+ // state ends up valid even if we don't collect all of the zones.
- for (ZonesIter zone(this, SkipAtoms); !zone.done(); zone.next()) {
+ for (ZonesIter zone(this, WithAtoms); !zone.done(); zone.next()) {
if (!zone->isCollecting() && !zone->arenas.arenaListsAreEmpty()) {
return false;
}
diff --git a/js/src/gc/Tracer.cpp b/js/src/gc/Tracer.cpp
@@ -92,11 +92,14 @@ void js::gc::TraceIncomingCCWs(JSTracer* trc,
// This function is used by the Cycle Collector (CC) to trace through -- or in
// CC parlance, traverse -- a Shape. The CC does not care about Shapes,
-// BaseShapes or PropMaps, only the JSObjects held live by them. Thus, we only
-// report non-Shape things.
+// BaseShapes or PropMaps themselves, only things held live by them that can
+// participate in cycles.
void gc::TraceCycleCollectorChildren(JS::CallbackTracer* trc, Shape* shape) {
shape->base()->traceChildren(trc);
- // Don't trace the PropMap because the CC doesn't care about PropertyKey.
+
+ // TODO: Trace symbols reachable from |shape|. Shapes can entrain symbols via
+ // their propmaps, and these can now participate in cycles. Not doing this
+ // means there are some cycles that the CC will not be able to collect.
}
/*** Traced Edge Printer ****************************************************/
diff --git a/js/src/gc/Verifier.cpp b/js/src/gc/Verifier.cpp
@@ -927,8 +927,9 @@ void HeapCheckTracerBase::dumpCellInfo(Cell* cell) {
}
fprintf(stderr, " %p", cell);
if (obj) {
- fprintf(stderr, " (compartment %p)", obj->compartment());
+ fprintf(stderr, " in compartment %p", obj->compartment());
}
+ fprintf(stderr, " in zone %p", cell->zone());
}
void HeapCheckTracerBase::dumpCellPath(const char* name) {
@@ -1200,8 +1201,13 @@ bool js::gc::CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key,
// Symbols key must be marked in the atom marking bitmap for the zone.
if (key->is<JS::Symbol>()) {
GCRuntime* gc = &mapRuntime->gc;
- if (!gc->atomMarking.atomIsMarked(zone, key->as<JS::Symbol>())) {
- fprintf(stderr, "Symbol key %p not marked in atom marking bitmap\n", key);
+ CellColor bitmapColor =
+ gc->atomMarking.getAtomMarkColor(zone, key->as<JS::Symbol>());
+ if (bitmapColor < keyColor) {
+ fprintf(stderr, "Atom marking bitmap is less marked than symbol key %p\n",
+ key);
+ fprintf(stderr, "(key %p is %s, bitmap is %s)\n", key,
+ CellColorName(keyColor), CellColorName(bitmapColor));
ok = false;
}
}
diff --git a/js/src/gc/WeakMap-inl.h b/js/src/gc/WeakMap-inl.h
@@ -183,7 +183,8 @@ bool WeakMap<K, V, AP>::markEntry(GCMarker* marker, gc::CellColor mapColor,
// zone, and if so mark it now. There's no need to set |marked| as this
// would have been marked later anyway.
auto* sym = static_cast<JS::Symbol*>(keyCell);
- if (marker->runtime()->gc.isSymbolReferencedByUncollectedZone(sym)) {
+ if (marker->runtime()->gc.isSymbolReferencedByUncollectedZone(
+ sym, marker->markColor())) {
TraceEdge(trc, &key, "WeakMap symbol key");
}
}
diff --git a/js/src/vm/JSAtomUtils.cpp b/js/src/vm/JSAtomUtils.cpp
@@ -411,7 +411,8 @@ AtomizeAndCopyCharsNonStaticValidLengthFromLookup(
return nullptr;
}
- if (MOZ_UNLIKELY(!cx->atomMarking().inlinedMarkAtomFallible(cx, atom))) {
+ if (MOZ_UNLIKELY(
+ !cx->atomMarking().inlinedMarkAtomFallible(cx->zone(), atom))) {
ReportOutOfMemory(cx);
return nullptr;
}
diff --git a/js/src/vm/JSContext-inl.h b/js/src/vm/JSContext-inl.h
@@ -95,13 +95,23 @@ class ContextChecks {
static_assert(std::is_same_v<T, JSAtom> || std::is_same_v<T, JS::Symbol>,
"Should only be called with JSAtom* or JS::Symbol* argument");
+ JS::AssertCellIsNotGray(thing);
+
#ifdef DEBUG
- // Atoms which move across zone boundaries need to be marked in the new
- // zone, see JS_MarkCrossZoneId.
- if (zone()) {
- if (!cx->runtime()->gc.atomMarking.atomIsMarked(zone(), thing)) {
+ // Atoms which move across zone boundaries need to be marked in the atom
+ // marking bitmap for the new zone, see JS_MarkCrossZoneId.
+ // Note that the atom marking state may not be up-to-date if incremental
+ // marking is taking place.
+ gc::GCRuntime* gc = &cx->runtime()->gc;
+ bool isGCMarking =
+ gc->state() >= gc::State::Mark && gc->state() <= gc::State::Sweep;
+ if (zone() && !isGCMarking) {
+ gc::GCRuntime* gc = &cx->runtime()->gc;
+ gc::CellColor color = gc->atomMarking.getAtomMarkColor(zone(), thing);
+ if (color != gc::CellColor::Black) {
MOZ_CRASH_UNSAFE_PRINTF(
- "*** Atom not marked for zone %p at argument %d", zone(), argIndex);
+ "*** Atom is marked %s for zone %p at argument %d",
+ gc::CellColorName(color), zone(), argIndex);
}
}
#endif
diff --git a/js/src/vm/PropMap.cpp b/js/src/vm/PropMap.cpp
@@ -135,13 +135,6 @@ DictionaryPropMap* SharedPropMap::toDictionaryMap(JSContext* cx,
static MOZ_ALWAYS_INLINE SharedPropMap* PropMapChildReadBarrier(
SharedPropMap* parent, SharedPropMap* child) {
JS::Zone* zone = child->zone();
- if (zone->needsIncrementalBarrier()) {
- // We need a read barrier for the map tree, since these are weak
- // pointers.
- ReadBarrier(child);
- return child;
- }
-
if (MOZ_UNLIKELY(zone->isGCSweeping() &&
IsAboutToBeFinalizedUnbarriered(child))) {
// The map we've found is unreachable and due to be finalized, so
@@ -151,9 +144,9 @@ static MOZ_ALWAYS_INLINE SharedPropMap* PropMapChildReadBarrier(
return nullptr;
}
- // We don't yield to the mutator when the zone is in this state so we don't
- // need to account for it here.
- MOZ_ASSERT(!zone->isGCCompacting());
+ // We need a read barrier for the map tree, since these are weak
+ // pointers.
+ ReadBarrier(child);
return child;
}
