From e01a571e52294c67ac4822e73990ea78ddf0139d Mon Sep 17 00:00:00 2001 From: win7-7 Date: Tue, 13 Jan 2026 06:40:39 +0200 Subject: [PATCH] 903519 - Disable nursery strings in a Zone if too many get tenured among with other fixes, won't fix startup crash assert though. 903519 - Disable nursery strings in a Zone if too many get tenured among with other fixes, won't fix startup crash assert though. Also small fix for expanding macro correctly. --- js/src/gc/Allocator.cpp | 6 ++- js/src/gc/AtomMarking.cpp | 26 ++++++++++--- js/src/gc/AtomMarking.h | 6 +-- js/src/gc/Marking.cpp | 1 + js/src/gc/Nursery.cpp | 43 +++++++++++++++------ js/src/gc/Zone.cpp | 2 + js/src/gc/Zone.h | 9 +++++ js/src/jit/CompileWrappers.cpp | 4 +- js/src/jit/JitCompartment.h | 7 ++++ js/src/jsatom.cpp | 70 +++++++++++++++++++++++++++++++++- js/src/jsgc.cpp | 40 ++++++++++++------- js/src/vm/HelperThreads.cpp | 17 +++++---- js/src/vm/HelperThreads.h | 7 ++++ js/src/vm/String.cpp | 3 +- 14 files changed, 195 insertions(+), 46 deletions(-) diff --git a/js/src/gc/Allocator.cpp b/js/src/gc/Allocator.cpp index 4ebeb7fe72..be9924db02 100644 --- a/js/src/gc/Allocator.cpp +++ b/js/src/gc/Allocator.cpp @@ -177,7 +177,11 @@ js::AllocateString(JSContext* cx, InitialHeap heap) if (!rt->gc.checkAllocatorState(cx, kind)) return nullptr; - if (cx->nursery().isEnabled() && heap != TenuredHeap && cx->nursery().canAllocateStrings()) { + if (cx->nursery().isEnabled() && + heap != TenuredHeap && + cx->nursery().canAllocateStrings() && + cx->zone()->allocNurseryStrings) + { auto str = static_cast(rt->gc.tryNewNurseryString(cx, size, kind)); if (str) return str; diff --git a/js/src/gc/AtomMarking.cpp b/js/src/gc/AtomMarking.cpp index d688257b54..6000ee641d 100644 --- a/js/src/gc/AtomMarking.cpp +++ b/js/src/gc/AtomMarking.cpp @@ -95,8 +95,9 @@ AtomMarkingRuntime::computeBitmapFromChunkMarkBits(JSRuntime* runtime, DenseBitm } void -AtomMarkingRuntime::updateZoneBitmap(Zone* zone, const DenseBitmap& bitmap) +AtomMarkingRuntime::refineZoneBitmapForCollectedZone(Zone* zone, const DenseBitmap& bitmap) { + MOZ_ASSERT(zone->isCollectingFromAnyThread()); if (zone->isAtomsZone()) return; @@ -109,7 +110,7 @@ AtomMarkingRuntime::updateZoneBitmap(Zone* zone, const DenseBitmap& bitmap) // Set any bits in the chunk mark bitmaps for atoms which are marked in bitmap. template static void -AddBitmapToChunkMarkBits(JSRuntime* runtime, Bitmap& bitmap) +BitwiseOrIntoChunkMarkBits(JSRuntime* runtime, Bitmap& bitmap) { // Make sure that by copying the mark bits for one arena in word sizes we // do not affect the mark bits for other arenas. @@ -127,7 +128,7 @@ AddBitmapToChunkMarkBits(JSRuntime* runtime, Bitmap& bitmap) } void -AtomMarkingRuntime::updateChunkMarkBits(JSRuntime* runtime) +AtomMarkingRuntime::markAtomsUsedByUncollectedZones(JSRuntime* runtime) { MOZ_ASSERT(runtime->currentThreadHasExclusiveAccess()); @@ -143,11 +144,11 @@ AtomMarkingRuntime::updateChunkMarkBits(JSRuntime* runtime) if (!zone->isCollectingFromAnyThread()) zone->markedAtoms().bitwiseOrInto(markedUnion); } - AddBitmapToChunkMarkBits(runtime, markedUnion); + BitwiseOrIntoChunkMarkBits(runtime, markedUnion); } else { for (ZonesIter zone(runtime, SkipAtoms); !zone.done(); zone.next()) { if (!zone->isCollectingFromAnyThread()) - AddBitmapToChunkMarkBits(runtime, zone->markedAtoms()); + BitwiseOrIntoChunkMarkBits(runtime, zone->markedAtoms()); } } } @@ -211,6 +212,21 @@ AtomMarkingRuntime::atomIsMarked(Zone* zone, T* thing) MOZ_ASSERT(thing); MOZ_ASSERT(!IsInsideNursery(thing)); + + if (!thing->zoneFromAnyThread()->isAtomsZone()) { + js::gc::AllocKind kind = thing->asTenured().getAllocKind(); + + fprintf(stderr, "WRONG ZONE: ptr=%p kind=%d ", (void*)thing, (int)kind); + + if (kind == js::gc::AllocKind::SYMBOL || (int)kind == 29) { + JS::Symbol* sym = reinterpret_cast(thing); + fprintf(stderr, "SYMBOL code=%u", (unsigned int)sym->code()); + } + + fprintf(stderr, "\n"); + fflush(stderr); + } + MOZ_ASSERT(thing->zoneFromAnyThread()->isAtomsZone()); if (!zone->runtimeFromAnyThread()->permanentAtoms) diff --git a/js/src/gc/AtomMarking.h b/js/src/gc/AtomMarking.h index 44b3ce6024..e06ea3e66e 100644 --- a/js/src/gc/AtomMarking.h +++ b/js/src/gc/AtomMarking.h @@ -53,11 +53,11 @@ class AtomMarkingRuntime // Update the atom marking bitmap in |zone| according to another // overapproximation of the reachable atoms in |bitmap|. - void updateZoneBitmap(Zone* zone, const DenseBitmap& bitmap); + void refineZoneBitmapForCollectedZone(Zone* zone, const DenseBitmap& bitmap); // Set any bits in the chunk mark bitmaps for atoms which are marked in any - // zone in the runtime. - void updateChunkMarkBits(JSRuntime* runtime); + // uncollected zone in the runtime. + void markAtomsUsedByUncollectedZones(JSRuntime* runtime); // Mark an atom or id as being newly reachable by the context's zone. template void markAtom(JSContext* cx, T* thing); diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index 9a602e3be9..03a50b89e8 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -2960,6 +2960,7 @@ js::TenuringTracer::moveToTenured(JSString* src) AllocKind dstKind = src->getAllocKind(); Zone* zone = src->zone(); + zone->tenuredStrings++; TenuredCell* t = zone->arenas.allocateFromFreeList(dstKind, Arena::thingSize(dstKind)); if (!t) { diff --git a/js/src/gc/Nursery.cpp b/js/src/gc/Nursery.cpp index 30475955c1..4b0f8c0702 100644 --- a/js/src/gc/Nursery.cpp +++ b/js/src/gc/Nursery.cpp @@ -703,21 +703,40 @@ js::Nursery::collect(JSRuntime* rt, JS::gcreason::Reason reason) bool validPromotionRate; const float promotionRate = calcPromotionRate(&validPromotionRate); uint32_t pretenureCount = 0; - if (validPromotionRate) { - if (promotionRate > 0.8 || IsFullStoreBufferReason(reason)) { - JSContext* cx = TlsContext.get(); - for (auto& entry : tenureCounts.entries) { - if (entry.count >= 3000) { - ObjectGroup* group = entry.group; - if (group->canPreTenure() && group->zone()->group()->canEnterWithoutYielding(cx)) { - AutoCompartment ac(cx, group); - group->setShouldPreTenure(cx); - pretenureCount++; - } + bool shouldPretenure = (validPromotionRate && promotionRate > 0.6) || + IsFullStoreBufferReason(reason); + + if (shouldPretenure) { + JSContext* cx = TlsContext.get(); + for (auto& entry : tenureCounts.entries) { + if (entry.count >= 3000) { + ObjectGroup* group = entry.group; + if (group->canPreTenure() && group->zone()->group()->canEnterWithoutYielding(cx)) { + AutoCompartment ac(cx, group); + group->setShouldPreTenure(cx); + pretenureCount++; } } } } + for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) { + if (shouldPretenure && zone->allocNurseryStrings && zone->tenuredStrings >= 30 * 1000) { + JSRuntime::AutoProhibitActiveContextChange apacc(rt); + CancelOffThreadIonCompile(zone); + bool preserving = zone->isPreservingCode(); + zone->setPreservingCode(false); + zone->discardJitCode(rt->defaultFreeOp()); + zone->setPreservingCode(preserving); + for (CompartmentsInZoneIter c(zone); !c.done(); c.next()) { + if (jit::JitCompartment* jitComp = c->jitCompartment()) { + jitComp->discardStubs(); + jitComp->stringsCanBeInNursery = false; + } + } + zone->allocNurseryStrings = false; + } + zone->tenuredStrings = 0; + } endProfile(ProfileKey::Pretenure); // We ignore gcMaxBytes when allocating for minor collection. However, if we @@ -1048,7 +1067,7 @@ js::Nursery::setStartPosition() void js::Nursery::maybeResizeNursery(JS::gcreason::Reason reason) { - static const double GrowThreshold = 0.05; + static const double GrowThreshold = 0.03; static const double ShrinkThreshold = 0.01; unsigned newMaxNurseryChunks; diff --git a/js/src/gc/Zone.cpp b/js/src/gc/Zone.cpp index 16dbb48543..39fe00dca9 100644 --- a/js/src/gc/Zone.cpp +++ b/js/src/gc/Zone.cpp @@ -42,6 +42,8 @@ JS::Zone::Zone(JSRuntime* rt) functionToStringCache_(group), usage(&rt->gc.usage), gcDelayBytes(0), + tenuredStrings(group, 0), + allocNurseryStrings(group, true), propertyTree_(group, this), baseShapes_(group, this), initialShapes_(group, this), diff --git a/js/src/gc/Zone.h b/js/src/gc/Zone.h index 5b166dedc2..d00c062dc8 100644 --- a/js/src/gc/Zone.h +++ b/js/src/gc/Zone.h @@ -451,6 +451,10 @@ struct Zone : public JS::shadow::Zone, // the current GC. js::UnprotectedData gcDelayBytes; + js::ZoneGroupData tenuredStrings; + js::ZoneGroupData allocNurseryStrings; + + private: // Shared Shape property tree. js::PropertyTree propertyTree; @@ -653,6 +657,7 @@ class ZoneGroupsIter next(); } + bool done() const { return it == end; } void next() { @@ -732,6 +737,10 @@ class ZonesIter if (!atomsZone && !done()) next(); } + + bool atAtomsZone(JSRuntime* rt) const { + return !!atomsZone; + } bool done() const { return !atomsZone && group.done(); } diff --git a/js/src/jit/CompileWrappers.cpp b/js/src/jit/CompileWrappers.cpp index d7b89d7fa7..068327d9dd 100644 --- a/js/src/jit/CompileWrappers.cpp +++ b/js/src/jit/CompileWrappers.cpp @@ -245,7 +245,9 @@ CompileZone::addressOfStringNurseryCurrentEnd() bool CompileZone::canNurseryAllocateStrings() { - return nurseryExists() && zone()->group()->nursery().canAllocateStrings(); + return nurseryExists() && + zone()->group()->nursery().canAllocateStrings() && + zone()->allocNurseryStrings; } bool diff --git a/js/src/jit/JitCompartment.h b/js/src/jit/JitCompartment.h index eb45171d34..cc85141943 100644 --- a/js/src/jit/JitCompartment.h +++ b/js/src/jit/JitCompartment.h @@ -630,6 +630,13 @@ class JitCompartment JitCode* stringConcatStubNoBarrier() const { return stringConcatStub_; } + + void discardStubs() { + stringConcatStub_ = nullptr; + regExpMatcherStub_ = nullptr; + regExpSearcherStub_ = nullptr; + regExpTesterStub_ = nullptr; + } JitCode* regExpMatcherStubNoBarrier() const { return regExpMatcherStub_; diff --git a/js/src/jsatom.cpp b/js/src/jsatom.cpp index 522d0f44f8..47f15e4487 100644 --- a/js/src/jsatom.cpp +++ b/js/src/jsatom.cpp @@ -276,6 +276,30 @@ AtomIsPinned(JSContext* cx, JSAtom* atom) return p->isPinned(); } +#ifdef DEBUG + +bool +AtomIsPinnedInRuntime(JSRuntime* rt, JSAtom* atom) +{ + Maybe lock; + if (!rt->currentThreadHasExclusiveAccess()) + lock.emplace(rt); + + AtomHasher::Lookup lookup(atom); + + AtomSet::Ptr p = LookupAtomState(rt, lookup); + MOZ_ASSERT(p); + + return p->isPinned(); +} + +#endif // DEBUG + +template +MOZ_ALWAYS_INLINE +static JSAtom* +AtomizeAndCopyCharsInner(JSContext* cx, const CharT* tbchars, size_t length, PinningBehavior pin, + const AtomHasher::Lookup& lookup); /* |tbchars| must not point into an inline or short string. */ template MOZ_ALWAYS_INLINE @@ -287,6 +311,24 @@ AtomizeAndCopyChars(ExclusiveContext* cx, const CharT* tbchars, size_t length, P AtomHasher::Lookup lookup(tbchars, length); + // Try the per-Zone cache first. If we find the atom there we can avoid the + // atoms lock, the markAtom call, and the multiple HashSet lookups below. + // We don't use the per-Zone cache if we want a pinned atom: handling that + // is more complicated and pinning atoms is relatively uncommon. + Zone* zone = cx->zone(); + Maybe zonePtr; + if (MOZ_LIKELY(zone && pin == DoNotPinAtom)) { + zonePtr.emplace(zone->atomCache().lookupForAdd(lookup)); + if (zonePtr.ref()) { + // The cache is purged on GC so if we're in the middle of an + // incremental GC we should have barriered the atom when we put + // it in the cache. + JSAtom* atom = zonePtr.ref()->asPtrUnbarriered(); + MOZ_ASSERT(AtomIsMarked(zone, atom)); + return atom; + } + } + // Note: when this function is called while the permanent atoms table is // being initialized (in initializeAtoms()), |permanentAtoms| is not yet // initialized so this lookup is always skipped. Only once @@ -298,6 +340,29 @@ AtomizeAndCopyChars(ExclusiveContext* cx, const CharT* tbchars, size_t length, P return pp->asPtr(cx); } + // Validate the length before taking the exclusive access lock, as throwing + // an exception here may reenter this code. + if (MOZ_UNLIKELY(!JSString::validateLength(cx, length))) + return nullptr; + + JSAtom* atom = AtomizeAndCopyCharsInner(cx, tbchars, length, pin, lookup); + if (!atom) + return nullptr; + + cx->atomMarking().inlinedMarkAtom(cx, atom); + + if (zonePtr) + mozilla::Unused << zone->atomCache().add(*zonePtr, AtomStateEntry(atom, false)); + + return atom; +} + +template +MOZ_ALWAYS_INLINE +static JSAtom* +AtomizeAndCopyCharsInner(JSContext* cx, const CharT* tbchars, size_t length, PinningBehavior pin, + const AtomHasher::Lookup& lookup) +{ AutoLockForExclusiveAccess lock(cx); AtomSet& atoms = cx->atoms(lock); @@ -308,7 +373,9 @@ AtomizeAndCopyChars(ExclusiveContext* cx, const CharT* tbchars, size_t length, P return atom; } - AutoCompartment ac(cx, cx->atomsCompartment(lock), &lock); + JSAtom* atom; + { + AutoAtomsCompartment ac(cx, lock); JSFlatString* flat = NewStringCopyN(cx, tbchars, length); if (!flat) { @@ -329,7 +396,6 @@ AtomizeAndCopyChars(ExclusiveContext* cx, const CharT* tbchars, size_t length, P ReportOutOfMemory(cx); /* SystemAllocPolicy does not report OOM. */ return nullptr; } - return atom; } diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 7834d9b434..d6d9e091c6 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -946,7 +946,8 @@ const char* gc::ZealModeHelpText = " 14: (Compact) Perform a shrinking collection every N allocations\n" " 15: (CheckHeapAfterGC) Walk the heap to check its integrity after every GC\n" " 16: (CheckNursery) Check nursery integrity on minor GC\n" - " 17: (IncrementalSweepThenFinish) Incremental GC in two slices: 1) start sweeping 2) finish collection\n"; + " 17: (IncrementalSweepThenFinish) Incremental GC in two slices: 1) start sweeping 2) finish collection\n" + " 18: (CheckGrayMarking) Check gray marking invariants after every GC\n"; // The set of zeal modes that control incremental slices. These modes are // mutually exclusive. @@ -5014,11 +5015,17 @@ class SweepWeakCacheTask : public GCSweepTask } }; -#define MAKE_GC_SWEEP_TASK(name) \ - class name : public GCSweepTask { \ - public: \ - void run(); \ - explicit name (JSRuntime* rt) : GCSweepTask(rt) {} \ +static void +UpdateAtomsBitmap(JSRuntime* runtime) +{ + DenseBitmap marked; + if (runtime->gc.atomMarking.computeBitmapFromChunkMarkBits(runtime, marked)) { + for (GCZonesIter zone(runtime); !zone.done(); zone.next()) + runtime->gc.atomMarking.refineZoneBitmapForCollectedZone(zone, marked); + } else { + // Ignore OOM in computeBitmapFromChunkMarkBits. The + // refineZoneBitmapForCollectedZone call can only remove atoms from the + // zone bitmap, so it is conservative to just not call it. } MAKE_GC_SWEEP_TASK(SweepAtomsTask); MAKE_GC_SWEEP_TASK(SweepCCWrappersTask); @@ -5029,10 +5036,11 @@ MAKE_GC_SWEEP_TASK(SweepRegExpsTask); MAKE_GC_SWEEP_TASK(SweepMiscTask); #undef MAKE_GC_SWEEP_TASK -/* virtual */ void -SweepAtomsTask::run() -{ - runtime->sweepAtoms(); + runtime->gc.atomMarking.markAtomsUsedByUncollectedZones(runtime); + + // For convenience sweep these tables non-incrementally as part of bitmap + // sweeping; they are likely to be much smaller than the main atoms table. + runtime->unsafeSymbolRegistry().sweep(); for (CompartmentsIter comp(runtime, SkipAtoms); !comp.done(); comp.next()) comp->sweepVarNames(); } @@ -5340,6 +5348,14 @@ GCRuntime::beginSweepingSweepGroup(FreeOp* fop, SliceBudget& budget) } } } + + // Updating the atom marking bitmaps. This marks atoms referenced by + // uncollected zones so cannot be done in parallel with the other sweeping + // work below. + if (sweepingAtoms) { + AutoPhase ap(stats(), PHASE_UPDATE_ATOMS_BITMAP); + UpdateAtomsBitmap(rt); + } if (sweepingAtoms) { AutoLockHelperThreadState helperLock; @@ -5349,10 +5365,6 @@ GCRuntime::beginSweepingSweepGroup(FreeOp* fop, SliceBudget& budget) { AutoLockHelperThreadState lock; - Maybe updateAtomsBitmap; - if (sweepingAtoms) - updateAtomsBitmap.emplace(rt, UpdateAtomsBitmap, PHASE_UPDATE_ATOMS_BITMAP, lock); - AutoPhase ap(stats(), PHASE_SWEEP_COMPARTMENTS); AutoSCC scc(stats(), sweepGroupIndex); diff --git a/js/src/vm/HelperThreads.cpp b/js/src/vm/HelperThreads.cpp index 5ea4f941ea..a53e7776f5 100644 --- a/js/src/vm/HelperThreads.cpp +++ b/js/src/vm/HelperThreads.cpp @@ -137,8 +137,9 @@ GetSelectorRuntime(CompilationSelector selector) { struct Matcher { - JSRuntime* match(JSScript* script) { return script->runtimeFromMainThread(); } - JSRuntime* match(JSCompartment* comp) { return comp->runtimeFromMainThread(); } + JSRuntime* match(JSScript* script) { return script->runtimeFromActiveCooperatingThread(); } + JSRuntime* match(JSCompartment* comp) { return comp->runtimeFromActiveCooperatingThread(); } + JSRuntime* match(Zone* zone) { return zone->runtimeFromActiveCooperatingThread(); } JSRuntime* match(ZonesInState zbs) { return zbs.runtime; } JSRuntime* match(JSRuntime* runtime) { return runtime; } JSRuntime* match(AllCompilations all) { return nullptr; } @@ -154,8 +155,9 @@ JitDataStructuresExist(CompilationSelector selector) { bool match(JSScript* script) { return !!script->compartment()->jitCompartment(); } bool match(JSCompartment* comp) { return !!comp->jitCompartment(); } - bool match(ZonesInState zbs) { return !!zbs.runtime->jitRuntime(); } - bool match(JSRuntime* runtime) { return !!runtime->jitRuntime(); } + bool match(Zone* zone) { return !!zone->jitZone(); } + bool match(ZonesInState zbs) { return zbs.runtime->hasJitRuntime(); } + bool match(JSRuntime* runtime) { return runtime->hasJitRuntime(); } bool match(AllCompilations all) { return true; } }; @@ -169,9 +171,10 @@ CompiledScriptMatches(CompilationSelector selector, JSScript* target) { JSScript* target_; - bool match(JSScript* script) { return script == target_; } - bool match(JSCompartment* comp) { return comp == target_->compartment(); } - bool match(JSRuntime* runtime) { return runtime == target_->runtimeFromAnyThread(); } + bool match(JSScript* script) { return script == builder_->script(); } + bool match(JSCompartment* comp) { return comp == builder_->script()->compartment(); } + bool match(Zone* zone) { return zone == builder_->script()->zone(); } + bool match(JSRuntime* runtime) { return runtime == builder_->script()->runtimeFromAnyThread(); } bool match(AllCompilations all) { return true; } bool match(ZonesInState zbs) { return zbs.runtime == target_->runtimeFromAnyThread() && diff --git a/js/src/vm/HelperThreads.h b/js/src/vm/HelperThreads.h index e08257bab4..cc6000af46 100644 --- a/js/src/vm/HelperThreads.h +++ b/js/src/vm/HelperThreads.h @@ -444,6 +444,7 @@ struct ZonesInState { JSRuntime* runtime; JS::Zone::GCState state; }; using CompilationSelector = mozilla::Variant; @@ -466,6 +467,12 @@ CancelOffThreadIonCompile(JSCompartment* comp) CancelOffThreadIonCompile(CompilationSelector(comp), true); } +inline void +CancelOffThreadIonCompile(Zone* zone) +{ + CancelOffThreadIonCompile(CompilationSelector(zone), true); +} + inline void CancelOffThreadIonCompile(JSRuntime* runtime, JS::Zone::GCState state) { diff --git a/js/src/vm/String.cpp b/js/src/vm/String.cpp index f5c717bfc0..37fc4a893c 100644 --- a/js/src/vm/String.cpp +++ b/js/src/vm/String.cpp @@ -510,7 +510,8 @@ JSRope::flattenInternal(ExclusiveContext* maybecx) Nursery& nursery = zone()->group()->nursery(); if (!nursery.registerMallocedBuffer(wholeChars)) { js_free(wholeChars); - ReportOutOfMemory(maybecx); + if (maybecx) + ReportOutOfMemory(maybecx); return nullptr; } }