Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 17 additions & 18 deletions Source/JavaScriptCore/bytecode/CodeBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1372,6 +1372,8 @@ ALWAYS_INLINE bool CodeBlock::shouldJettisonDueToOldAge(const ConcurrentJSLocker
#if USE(BUN_JSC_ADDITIONS)
JITType type = jitType();
Seconds ttl = timeToLive(type);
// One clock read per collection (Heap), not one per block visited.
ApproximateTime now = vm().heap.currentGCStartApproximateTime();

// Optimizing tiers: a DFG block ages like a baseline one, using the tier-up counter its code already decrements at
// returns and loop back-edges as the sign of life. FTL code, and DFG code compiled without tier-up checks, has no
Expand All @@ -1390,24 +1392,20 @@ ALWAYS_INLINE bool CodeBlock::shouldJettisonDueToOldAge(const ConcurrentJSLocker
Seconds quietFor = Seconds(Options::optimizedCodeAgingQuietSeconds());
if (Options::useEagerCodeBlockJettisonTiming()) [[unlikely]]
quietFor = std::min(quietFor, ttl);
return ApproximateTime::now() - std::max(m_creationTime, heap.lastActiveCollectionTime()) >= quietFor;
return now - std::max(m_creationTime, heap.lastActiveCollectionTime()) >= quietFor;
}

if (timeSinceCreation() < ttl)
return false;

if (Options::useExecutionCountForCodeBlockAging()) {
// LLInt and Baseline CodeBlocks already tick an execution counter on
// function entry and loop back-edges. If that counter has moved since we
// last sampled it, the block is demonstrably still running regardless of
// wall-clock age, so renew its lease instead of throwing away a warm block
// that the next iteration will immediately relink, re-profile and re-JIT.
// LLInt and Baseline CodeBlocks already tick an execution counter on function entry and loop back-edges (a DFG
// block, its tier-up counter). If it has moved since the last collection that looked, the block ran in between:
// renew its lease. The snapshot is refreshed on every look - including while the block is still inside its
// lease - so "moved" always means "since the previous collection", never "since some collection before the
// last burst of work" (which kept idle code alive for one extra lease every time, and forever when the
// embedder's idle collections come in pairs).
//
// The snapshot lives in m_previousCounter, which updateActivity() in
// reconcileWeakReferencesAtGCEnd also writes for UnlinkedCodeBlock aging when
// VM::useUnlinkedCodeBlockJettisoning() is enabled. Both sites store the
// same current count for the same tier, so they agree; outside that mode
// updateActivity() never touches the field.
// The snapshot lives in m_previousCounter, which updateActivity() in reconcileWeakReferencesAtGCEnd also writes
// for UnlinkedCodeBlock aging when VM::useUnlinkedCodeBlockJettisoning() is enabled. Both sites store the same
// current count for the same tier, so they agree; outside that mode updateActivity() never touches the field.
float currentCount = 0;
bool hasCounter = false;
switch (type) {
Expand All @@ -1434,13 +1432,14 @@ ALWAYS_INLINE bool CodeBlock::shouldJettisonDueToOldAge(const ConcurrentJSLocker
}
if (hasCounter && currentCount != m_previousCounter) {
m_previousCounter = currentCount;
Comment on lines 1431 to 1432

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Moving the counter snapshot ahead of the TTL gate means m_previousCounter = currentCount is now written during marking on every look, so updateActivity()'s later m_previousCounter < count test is always false and m_unlinkedCode->resetAge() is never called for LLInt/Baseline/DFG blocks; under useUnlinkedCodeBlockJettisoning (mini/no-JIT mode) the UnlinkedCodeBlock's age hits maxAge after ~7 GCs and dies the moment its CodeBlock ages out, forcing a full re-parse on the next call where the base branch kept the bytecode cached. Fix: have updateActivity() decide activity from the flag this pass already computed (e.g. reset age whenever shouldJettisonDueToOldAge renewed the lease) instead of re-comparing m_previousCounter.

Extended reasoning...

shouldJettisonDueToOldAge() runs from the executable's visitChildren during marking; with the reorder, an unmarked LLInt/Baseline block whose counter moved hits line 1431 and stores m_previousCounter = currentCount before the TTL early-out. The block is then strongly visited (line 1298 returns true), added to the CodeBlock set (line 1278), and at GC end reconcileWeakReferencesAtGCEnd → updateActivity() (guarded by VM::useUnlinkedCodeBlockJettisoning(), true when !useJIT() or forceMiniVMMode) reads the same counter into count and tests m_previousCounter < count at CodeBlock.cpp:1988 — now always equal, so resetAge() at 1990 is skipped. On the base branch the TTL gate returned before touching m_previousCounter, so updateActivity() still saw the previous GC's snapshot, observed movement, and reset the UnlinkedCodeBlock's age each collection (only the single renewal GC per lease skipped it). After the change nothing ever resets m_age; UnlinkedCodeBlock::visitChildren (UnlinkedCodeBlock.cpp:103) increments it every GC to maxAge=7, and once the CodeBlock finally…

Verification: normal — the reorder introduces a regression the base does not have, in the VM::useUnlinkedCodeBlockJettisoning() configuration (VM.h:766-769 → !useJIT() || forceMiniVMMode() || Options::useUnlinkedCodeBlockJettisoning()). Mechanism, verified against both branches: - ScriptExecutable::visitCodeBlockEdge (ScriptExecutable.cpp:565) calls shouldVisitStrongly() during marking, which at…

// Push the effective creation time forward so the block is not
// considered for old-age jettison again until leaseMultiplier * ttl
// has elapsed with no observed execution.
m_creationTime = ApproximateTime::now() + ttl * (Options::codeBlockAgingLeaseMultiplier() - 1.0);
// Ran since the last look: the lease runs leaseMultiplier * ttl from now with no observed execution.
m_creationTime = now + ttl * (Options::codeBlockAgingLeaseMultiplier() - 1.0);
return false;
}
}

if (now - m_creationTime < ttl)
return false;
#else
if (timeSinceCreation() < timeToLive(jitType()))
return false;
Expand Down
3 changes: 2 additions & 1 deletion Source/JavaScriptCore/heap/Heap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1574,12 +1574,13 @@ NEVER_INLINE bool Heap::runBeginPhase(GCConductor conn)
m_currentRequest = m_requests.first();
}
#if USE(BUN_JSC_ADDITIONS)
m_currentGCStartApproximateTime = ApproximateTime::now();
// Accumulated across collections, so a mutator that works steadily but is collected often (each cycle small) still
// reads as active; only a genuinely quiet stretch leaves the stamp to age.
m_bytesAllocatedSinceLastActiveCollection += totalBytesAllocatedThisCycle();
if (m_bytesAllocatedSinceLastActiveCollection > Options::optimizedCodeAgingQuietAllocationMB() * MB) {
m_bytesAllocatedSinceLastActiveCollection = 0;
m_lastActiveCollectionTime = ApproximateTime::now();
m_lastActiveCollectionTime = m_currentGCStartApproximateTime;
}
#endif

Expand Down
4 changes: 4 additions & 0 deletions Source/JavaScriptCore/heap/Heap.h
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,9 @@ class Heap {
// When a collection last began that found the mutator had allocated more than a trickle since the one before: the
// mutator was at work then. Idle optimized code ages against this (CodeBlock::shouldJettisonDueToOldAge).
ApproximateTime lastActiveCollectionTime() const { return m_lastActiveCollectionTime; }
// Read once when the current (or last) collection began; CodeBlock aging measures against it instead of reading the
// clock for every block it visits.
ApproximateTime currentGCStartApproximateTime() const { return m_currentGCStartApproximateTime; }
// The collection in progress was requested by the embedder because the application went idle (GCRequest::isIdle).
bool isIdleCollection() const { return m_currentRequest.isIdle; }
#endif
Expand Down Expand Up @@ -887,6 +890,7 @@ class Heap {
size_t m_bytesAllocatedBeforeLastEdenCollect { 0 };
#if USE(BUN_JSC_ADDITIONS)
ApproximateTime m_lastActiveCollectionTime;
ApproximateTime m_currentGCStartApproximateTime;
size_t m_bytesAllocatedSinceLastActiveCollection { 0 };
#endif
size_t m_sizeAfterLastCollect { 0 };
Expand Down
7 changes: 7 additions & 0 deletions Source/JavaScriptCore/runtime/ScriptExecutableInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ inline void ScriptExecutable::jettisonCodeBlockEdgeIfDead(VM& vm, WriteBarrier<C
if (vm.heap.isMarked(codeBlock))
return;

#if USE(BUN_JSC_ADDITIONS)
// An optimizing block the old-age check let go this cycle died of old age, not of a dead weak reference: say so,
// so jettison() takes its old-age path (no exit-site tally, baseline code cache released) rather than the dead-weak-reference one.
if (codeBlock->agedOut())
codeBlock->jettison(Profiler::JettisonDueToOldAge);
else
#endif
if (codeBlock->shouldJettisonDueToWeakReference(vm))
codeBlock->jettison(Profiler::JettisonDueToWeakReference);
else
Expand Down
Loading