Skip to content

GarbageCollectionController: fire up to 2 Full GCs before going to slow interval#29280

Closed
Jarred-Sumner wants to merge 15 commits into
mainfrom
jarred/idle-full-gc
Closed

GarbageCollectionController: fire up to 2 Full GCs before going to slow interval#29280
Jarred-Sumner wants to merge 15 commits into
mainfrom
jarred/idle-full-gc

track GC-driven shrinks in .pending so re-growth is detected

a52ca18
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 5, 2026 in 33m 16s

Code review found 1 important issue

Found 3 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/jsc/GarbageCollectionController.zig:188-190 #idle_full_gcs_fired not cleared by allocation-driven path while already in fast mode (regression of resolved review)
🟡 Nit src/jsc/GarbageCollectionController.zig:49-50 New bun.getenvZ() call sites added despite deprecation; prefer bun.env_var pattern

Annotations

Check failure on line 190 in src/jsc/GarbageCollectionController.zig

See this annotation in the file changed.

@claude claude / Claude Code Review

#idle_full_gcs_fired not cleared by allocation-driven path while already in fast mode (regression of resolved review)

Commit af48886 re-gated the `.fast` resets in `updateGCRepeatTimer` on a genuine slow→fast transition, which re-opens the issue from the resolved review thread at this line: when `processGCTimerWithHeapSize()` observes growth while already in fast mode, `updateGCRepeatTimer(.fast)` is now a complete no-op and `#idle_full_gcs_fired` is never cleared. So allocation that resumes between tick 30 and tick 31 cannot cancel reduction mode, and the next repeating-timer tick can fire a spurious second `c

Check warning on line 50 in src/jsc/GarbageCollectionController.zig

See this annotation in the file changed.

@claude claude / Claude Code Review

New bun.getenvZ() call sites added despite deprecation; prefer bun.env_var pattern

nit: `bun.getenvZ()` is marked for sunset ("You likely do not need this function. See the pattern in env_var.zig… TODO: Sunset this function when its last usage is removed" — src/bun.zig:925-927), and this file already uses the preferred `bun.env_var.BUN_TRACK_LAST_FN_NAME.get()` pattern eight lines up. Consider adding `BUN_GC_TIMER_INTERVAL` / `BUN_GC_RUNS_UNTIL_SKIP_RELEASE_ACCESS` (kind.unsigned) and `BUN_GC_TIMER_DISABLE` (kind.boolean) to `src/bun_core/env_var.zig` and reading them via `bun