From d27f358dcf5cc5ed869e8a9e555f9c83a74cd631 Mon Sep 17 00:00:00 2001 From: roytam1 Date: Mon, 25 May 2026 23:42:22 +0800 Subject: [PATCH] Revert "ported from UXP: Issue #3092 - Fix unsafe GC multithreading changes (f0cba412)" This reverts commit a208479a1b22a69c836888301bb730f0fa20558c. --- js/src/gc/GCRuntime.h | 6 +- js/src/gc/IdleGC.cpp | 68 ++++++------ js/src/jsapi.h | 2 +- js/src/jsgc.cpp | 236 ++++++++++++++++++++++++++++++++++-------- 4 files changed, 232 insertions(+), 80 deletions(-) diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h index c86f95de5..abb2dbf6d 100644 --- a/js/src/gc/GCRuntime.h +++ b/js/src/gc/GCRuntime.h @@ -686,6 +686,10 @@ class GCRuntime void requestMinorGC(JS::gcreason::Reason reason); + // Zone relocation for compacting GC (can be called from helper threads) + MOZ_MUST_USE bool relocateArenas(Zone* zone, JS::gcreason::Reason reason, + Arena*& relocatedListOut, SliceBudget& sliceBudget); + #ifdef DEBUG bool onBackgroundThread() { return helperState.onBackgroundThread(); } #endif // DEBUG @@ -981,8 +985,6 @@ class GCRuntime void endCompactPhase(JS::gcreason::Reason reason); void sweepTypesAfterCompacting(Zone* zone); void sweepZoneAfterCompacting(Zone* zone); - MOZ_MUST_USE bool relocateArenas(Zone* zone, JS::gcreason::Reason reason, - Arena*& relocatedListOut, SliceBudget& sliceBudget); void updateTypeDescrObjects(MovingTracer* trc, Zone* zone); void updateCellPointers(MovingTracer* trc, Zone* zone, AllocKinds kinds, size_t bgTaskCount); void updateAllCellPointers(MovingTracer* trc, Zone* zone); diff --git a/js/src/gc/IdleGC.cpp b/js/src/gc/IdleGC.cpp index 2e6481960..ac5c0d305 100644 --- a/js/src/gc/IdleGC.cpp +++ b/js/src/gc/IdleGC.cpp @@ -5,36 +5,36 @@ /** * IDLE-TIME GARBAGE COLLECTION SYSTEM - * + * * Overview * -------- * This system implements idle-time-only garbage collection, deferring GC work * to periods when the JavaScript engine is not actively processing code. * This improves perceived responsiveness by avoiding GC pauses during critical * execution windows. - * + * * Architecture * ----------- - * + * * The system consists of several key components: - * + * * 1. IdleGCManager (gc/IdleGC.h, gc/IdleGC.cpp) * - Tracks when the JS engine is executing vs. idle - * - Maintains timestamp of last execution activity + * - Maintains timestamp of last execution activity * - Checks if sufficient idle time has passed * - Configurable idle threshold (default: 100ms) * - Can determine which GC reasons should bypass idle checks - * + * * 2. GCRuntime Integration (gc/GCRuntime.h) * - Contains an IdleGCManager instance * - Provides public methods: notifyJSExecutionStart/End() * - Modified checkIfGCAllowedInCurrentState() to check idle status - * + * * 3. Activity Tracking (vm/Runtime.cpp) * - triggerActivityCallback() now notifies IdleGCManager * - Called when JS enters/exits request (execution boundary) * - Updated in jsapi.cpp's StartRequest/StopRequest functions - * + * * 4. Public API (jsapi.h, jsapi.cpp) * - JS_SetIdleGCEnabled() / JS_IsIdleGCEnabled() * - JS_SetIdleGCThreshold() / JS_GetIdleGCThreshold() @@ -43,18 +43,18 @@ * * Behavior * -------- - * + * * Normal GC Trigger (Idle GC Enabled): - * + * * 1. JS code executes → notifyJSExecutionStart() called * 2. JS code finishes → notifyJSExecutionEnd() called, timestamp recorded * 3. GC needed → checkIfGCAllowedInCurrentState() checks idle status * 4a. If idle >= threshold → GC proceeds normally * 4b. If still executing/idle < threshold → GC is deferred * 5. Once idle threshold is met, next GC request proceeds - * + * * Critical GC Triggers (Always Proceed): - * + * * The following GC reasons bypass idle checking: * - OUT_OF_MEMORY: Memory pressure conditions * - ALLOC_TRIGGER: Allocation threshold exceeded @@ -65,63 +65,63 @@ * - EVICT_NURSERY: Nursery eviction * - SHUTDOWN_CC, DESTROY_RUNTIME, LAST_DITCH: Shutdown GCs * - DESTROY_ZONE, COMPARTMENT_REVOKED: Zone/compartment destruction - * + * * Configuration * ------------- - * + * * Idle GC Enabled (default: true): * - Enables idle-time-only GC mode * - Can be toggled dynamically via JS_SetIdleGCEnabled() - * + * * Idle Threshold (default: 100ms): * - Minimum idle time before GC is permitted * - Configurable via JS_SetIdleGCThreshold(ms) * - Typical values: 50-200ms depending on application - * + * * Integration Examples * -------------------- - * + * * Browser Integration: - * + * * // When browser loads configuration * JS_SetIdleGCEnabled(cx, true); * JS_SetIdleGCThreshold(cx, 100); // 100ms idle threshold - * + * * // Monitor idle GC effectiveness (optional) * uint64_t idleTime = JS_GetIdleTimeSinceLastExecution(cx); * if (idleTime > JS_GetIdleGCThreshold(cx)) { * // System is idle, GC would be allowed if triggered * } - * + * * Disabling for Specific Scenarios: - * + * * // During initialization when nothing is "idle" yet * JS_SetIdleGCEnabled(cx, false); * // ... do initial setup ... * JS_SetIdleGCEnabled(cx, true); // Re-enable for normal operation - * + * * Performance Considerations * -------------------------- - * + * * Benefits: * - Reduced jank during active JS execution * - GC pauses moved to idle periods where users won't notice * - Especially effective for interactive applications * - Improves Time-to-Interactive and First Input Delay metrics - * + * * Tradeoffs: * - May accumulate more garbage before collection * - Requires predictable idle periods (not suitable for all workloads) * - Critical memory pressure GCs still proceed immediately - * + * * Tuning: * - Lower threshold (50ms) = more frequent GC, less memory overhead * - Higher threshold (200ms) = less GC overhead, more memory usage * - Optimal value depends on application's execution pattern - * + * * Testing * ------- - * + * * Unit Tests: * // Test idle detection * JS_SetIdleGCThreshold(cx, 100); @@ -132,30 +132,30 @@ * cx->runtime()->gc.notifyJSExecutionEnd(); * // ... wait 150ms ... * MOZ_ASSERT(cx->runtime()->gc.idleGCMgr().isIdleEnough()); - * + * * Integration Tests: * - Verify GC is deferred during active execution * - Verify GC proceeds after idle period * - Verify critical GC reasons bypass idle check * - Measure latency improvements - * + * * Implementation Notes * -------------------- - * + * * Thread Safety: * - IdleGCManager uses mozilla::Atomic for thread-safe state * - TimeStamp operations are atomic * - No additional locking needed beyond existing GC locks - * + * * Compatibility: * - Works with both incremental and non-incremental GC * - Compatible with generational GC * - Works with zone GC and full GC * - Respects existing GC suppression mechanisms - * + * * Future Enhancements * ------------------- - * + * * Potential improvements: * - Adaptive idle threshold based on historical GC times * - Per-zone idle configuration @@ -163,7 +163,7 @@ * - Metrics/telemetry for idle GC effectiveness * - Machine learning-based prediction of idle periods * - Cooperative GC scheduling with other subsystems - * + * */ #include "gc/IdleGC.h" diff --git a/js/src/jsapi.h b/js/src/jsapi.h index ba0a9ff84..e68bea917 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -1786,7 +1786,7 @@ JS_SetIdleGCThreshold(JSContext* cx, uint64_t milliseconds); extern JS_PUBLIC_API(uint64_t) JS_GetIdleGCThreshold(JSContext* cx); -/** +/** * Get the current idle time since the last JS execution. */ extern JS_PUBLIC_API(uint64_t) diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 84853125c..729cc48db 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -4530,10 +4530,8 @@ PrepareWeakCacheTasks(JSRuntime* rt) WeakCacheTaskVector out; for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) { for (JS::WeakCache* cache : zone->weakCaches_) { - if (!out.emplaceBack(rt, *cache)) { - SweepWeakCachesFromMainThread(rt); - return WeakCacheTaskVector(); - } + if (!out.emplaceBack(rt, *cache)) + return out; } } return out; @@ -4637,14 +4635,67 @@ GCRuntime::beginSweepingZoneGroup(AutoLockForExclusiveAccess& lock) // Cancel any active or pending off thread compilations. js::CancelOffThreadIonCompile(rt, JS::Zone::Sweep); - for (GCCompartmentGroupIter c(rt); !c.done(); c.next()) { - c->sweepGlobalObject(&fop); - c->sweepDebugEnvironments(); - c->sweepJitCompartment(&fop); - c->sweepTemplateObjects(); - } + // Parallelize compartment and zone sweeping operations + struct CompartmentZoneCleanupTask : public GCParallelTaskHelper + { + JSRuntime* rt; + JSCompartment* comp; + Zone* zone; + + explicit CompartmentZoneCleanupTask(JSRuntime* r, JSCompartment* c, Zone* z) + : rt(r), comp(c), zone(z) + {} + + CompartmentZoneCleanupTask(CompartmentZoneCleanupTask&& other) + : GCParallelTaskHelper(mozilla::Move(other)), + rt(other.rt), + comp(other.comp), + zone(other.zone) + {} + + void run() { + FreeOp fop(rt); + if (comp) { + comp->sweepGlobalObject(&fop); + comp->sweepDebugEnvironments(); + comp->sweepJitCompartment(&fop); + comp->sweepTemplateObjects(); + } + if (zone) + zone->sweepWeakMaps(); + } + }; + + typedef Vector CleanupTaskVector; + CleanupTaskVector tasks; + + size_t taskCount = 0; + for (GCCompartmentGroupIter c(rt); !c.done(); c.next()) + taskCount++; for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) - zone->sweepWeakMaps(); + taskCount++; + + if (taskCount > 1 && tasks.reserve(taskCount)) { + for (GCCompartmentGroupIter c(rt); !c.done(); c.next()) + tasks.infallibleEmplaceBack(rt, c, nullptr); + for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) + tasks.infallibleEmplaceBack(rt, nullptr, zone); + + AutoLockHelperThreadState helperLock; + for (auto& task : tasks) + startTask(task, gcstats::PHASE_SWEEP_MISC, helperLock); + for (auto& task : tasks) + joinTask(task, gcstats::PHASE_SWEEP_MISC, helperLock); + } else { + for (GCCompartmentGroupIter c(rt); !c.done(); c.next()) { + c->sweepGlobalObject(&fop); + c->sweepDebugEnvironments(); + c->sweepJitCompartment(&fop); + c->sweepTemplateObjects(); + } + for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) + zone->sweepWeakMaps(); + } // Bug 1071218: the following two methods have not yet been // refactored to work on a single zone-group at once. @@ -4657,29 +4708,57 @@ GCRuntime::beginSweepingZoneGroup(AutoLockForExclusiveAccess& lock) jit::JitRuntime::SweepJitcodeGlobalTable(rt); } + // Parallelize per-zone JIT cleanup and metadata sweeping + struct ZoneCleanupTask : public GCParallelTaskHelper { - gcstats::AutoPhase apdc(stats, gcstats::PHASE_SWEEP_DISCARD_CODE); - for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) + JSRuntime* rt; + Zone* zone; + bool releaseObservedTypes; + + explicit ZoneCleanupTask(JSRuntime* r, Zone* z, bool releaseTypes) + : rt(r), zone(z), releaseObservedTypes(releaseTypes) + {} + + ZoneCleanupTask(ZoneCleanupTask&& other) + : GCParallelTaskHelper(mozilla::Move(other)), + rt(other.rt), + zone(other.zone), + releaseObservedTypes(other.releaseObservedTypes) + {} + + void run() { + FreeOp fop(rt); zone->discardJitCode(&fop); - } - - { - gcstats::AutoPhase ap1(stats, gcstats::PHASE_SWEEP_TYPES); - gcstats::AutoPhase ap2(stats, gcstats::PHASE_SWEEP_TYPES_BEGIN); - for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) zone->beginSweepTypes(&fop, releaseObservedTypes && !zone->isPreservingCode()); - } - - { - gcstats::AutoPhase ap(stats, gcstats::PHASE_SWEEP_BREAKPOINT); - for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) zone->sweepBreakpoints(&fop); - } + zone->sweepUniqueIds(&fop); + } + }; { - gcstats::AutoPhase ap(stats, gcstats::PHASE_SWEEP_BREAKPOINT); + typedef Vector ZoneCleanupTaskVector; + ZoneCleanupTaskVector tasks; + size_t zoneCount = 0; for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) - zone->sweepUniqueIds(&fop); + zoneCount++; + + if (zoneCount > 1 && tasks.reserve(zoneCount)) { + for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) + tasks.infallibleEmplaceBack(rt, zone, releaseObservedTypes); + + AutoLockHelperThreadState helperLock; + for (auto& task : tasks) + startTask(task, gcstats::PHASE_SWEEP_DISCARD_CODE, helperLock); + for (auto& task : tasks) + joinTask(task, gcstats::PHASE_SWEEP_DISCARD_CODE, helperLock); + } else { + for (GCZoneGroupIter zone(rt); !zone.done(); zone.next()) { + zone->discardJitCode(&fop); + zone->beginSweepTypes(&fop, releaseObservedTypes && !zone->isPreservingCode()); + zone->sweepBreakpoints(&fop); + zone->sweepUniqueIds(&fop); + } + } } } @@ -5031,7 +5110,7 @@ GCRuntime::performSweepActions(SliceBudget& budget, AutoLockForExclusiveAccess& // Reset phase index. sweepPhaseIndex = 0; - + endSweepingZoneGroup(); getNextZoneGroup(); if (!currentZoneGroup) @@ -5105,6 +5184,43 @@ GCRuntime::endSweepPhase(bool destroyingRuntime, AutoLockForExclusiveAccess& loc AssertNoWrappersInGrayList(rt); } +// Zone relocation task for parallel compaction +struct ZoneCompactionTask : public GCParallelTaskHelper +{ + JSRuntime* runtime; + Zone* zone; + JS::gcreason::Reason reason; + Arena* relocatedArenas; + bool relocateSucceeded; + + explicit ZoneCompactionTask(JSRuntime* rt, Zone* z, JS::gcreason::Reason r) + : runtime(rt), zone(z), reason(r), relocatedArenas(nullptr), relocateSucceeded(false) + {} + + ZoneCompactionTask(ZoneCompactionTask&& other) + : GCParallelTaskHelper(mozilla::Move(other)), + runtime(other.runtime), + zone(other.zone), + reason(other.reason), + relocatedArenas(nullptr), + relocateSucceeded(false) + {} + + void run() { + AutoSuppressProfilerSampling suppressSampling(runtime); + zone->setGCState(Zone::Compact); + SliceBudget unlimited = SliceBudget::unlimited(); + relocateSucceeded = runtime->gc.relocateArenas(zone, reason, relocatedArenas, unlimited); + zone->setGCState(Zone::Finished); + } + + Arena* takeRelocatedArenas() { + Arena* result = relocatedArenas; + relocatedArenas = nullptr; + return result; + } +}; + void GCRuntime::beginCompactPhase() { @@ -5135,22 +5251,56 @@ GCRuntime::compactPhase(JS::gcreason::Reason reason, SliceBudget& sliceBudget, ZoneList relocatedZones; Arena* relocatedArenas = nullptr; - while (!zonesToMaybeCompact.isEmpty()) { - // TODO: JSScripts can move. If the sampler interrupts the GC in the - // middle of relocating an arena, invalid JSScript pointers may be - // accessed. Suppress all sampling until a finer-grained solution can be - // found. See bug 1295775. - AutoSuppressProfilerSampling suppressSampling(rt); + // Collect zones to compact and parallelize when there are multiple zones + typedef Vector ZoneCompactionTaskVector; + ZoneCompactionTaskVector tasks; + + size_t zoneCount = 0; + for (Zone* zone = zonesToMaybeCompact.front(); zone; zone = zone->nextZone()) + zoneCount++; - Zone* zone = zonesToMaybeCompact.front(); - MOZ_ASSERT(zone->isGCFinished()); - zone->setGCState(Zone::Compact); - if (relocateArenas(zone, reason, relocatedArenas, sliceBudget)) - updateZonePointersToRelocatedCells(zone, lock); - zone->setGCState(Zone::Finished); - zonesToMaybeCompact.removeFront(); - if (sliceBudget.isOverBudget()) - break; + // Use parallel compaction for multiple zones, fall back to serial for single zone + if (zoneCount > 1 && tasks.reserve(zoneCount)) { + for (Zone* zone = zonesToMaybeCompact.front(); zone; zone = zone->nextZone()) { + MOZ_ASSERT(zone->isGCFinished()); + tasks.infallibleEmplaceBack(rt, zone, reason); + } + + AutoLockHelperThreadState helperLock; + for (auto& task : tasks) + startTask(task, gcstats::PHASE_COMPACT_MOVE, helperLock); + for (auto& task : tasks) { + joinTask(task, gcstats::PHASE_COMPACT_MOVE, helperLock); + Arena* taskRelocated = task.takeRelocatedArenas(); + if (taskRelocated) { + Arena* tail = taskRelocated; + while (tail->next) + tail = tail->next; + tail->next = relocatedArenas; + relocatedArenas = taskRelocated; + } + } + + // Update pointers for all compacted zones + for (auto& task : tasks) + updateZonePointersToRelocatedCells(task.zone, lock); + + while (!zonesToMaybeCompact.isEmpty()) + zonesToMaybeCompact.removeFront(); + } else { + // Fall back to serial compaction for single zone or OOM + while (!zonesToMaybeCompact.isEmpty()) { + AutoSuppressProfilerSampling suppressSampling(rt); + Zone* zone = zonesToMaybeCompact.front(); + MOZ_ASSERT(zone->isGCFinished()); + zone->setGCState(Zone::Compact); + if (relocateArenas(zone, reason, relocatedArenas, sliceBudget)) + updateZonePointersToRelocatedCells(zone, lock); + zone->setGCState(Zone::Finished); + zonesToMaybeCompact.removeFront(); + if (sliceBudget.isOverBudget()) + break; + } } if (!relocatedZones.isEmpty()) {