Heap::setInitialAllocationBudget: let an embedder defer the first collection while building a large live graph - #533
Conversation
…lection while it builds a large live object graph Raises the first cycle's eden/heap budget and holds the allocation-paced GC timers until the first collection has run; after that, sizing follows the usual rules and minimums. Used by Bun for compiled executables, where the whole precompiled module graph is decoded and evaluated up front and early collections free nothing.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 5 included reviews per hour; 1 remains after this review. WalkthroughChangesInitial Allocation Budget
Merge Risk: ⚪ Minimal · up to This change defers the first garbage collection for an explicitly configured initial allocation budget, with normal collection behavior restored afterward; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the API, behavior, motivation, and observed impact. However, it does not include the required bug title and Bugzilla link, review status, or a changed-file/function list from the repository template.
Warning Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use Comment |
There was a problem hiding this comment.
Beyond the inline finding, I also checked whether the widened m_maxEdenSize outlives the first cycle when the first collection is an Eden GC — it does persist through the Eden branch of updateAllocationLimits(), but with the graph actually live the minEdenToOldGenerationRatio check forces the next collection to Full, which recomputes both limits from proportionalHeapSize(), so it self-corrects.
Extended reasoning...
The concern that an over-estimated initial budget would become a permanent eden size was examined: the Eden branch at Heap.cpp:2630-2648 leaves m_maxEdenSize untouched, but when the initial graph is live as intended, currentHeapSize is close to m_maxHeapSize, so edenToOldGenerationRatio falls below the threshold and m_shouldDoFullCollection is set — the following Full GC then resets both m_maxHeapSize and m_maxEdenSize from proportionalHeapSize(currentHeapSize, m_ramSize). If the estimate is wildly high and little is live, the inflated eden size lingers for one extra cycle, which was judged acceptable for an opt-in embedder hint guarded under USE(BUN_JSC_ADDITIONS).
Preview Builds
|
…tial-budget re-enable latch
| m_reenableEdenActivityCallback = false; | ||
| m_reenableFullActivityCallback = false; |
There was a problem hiding this comment.
🔴 setGarbageCollectionTimerEnabled() unconditionally assigns m_reenableEdenActivityCallback/m_reenableFullActivityCallback, but those members are declared inside #if USE(BUN_JSC_ADDITIONS) in Heap.h — any non-Bun build (upstream JSCOnly, or USE_BUN_JSC_ADDITIONS=OFF) fails to compile here where the base branch built cleanly. Fix: wrap these two assignments in #if USE(BUN_JSC_ADDITIONS) so the member references only exist when the members do.
Extended reasoning...
Heap.h lines 898-901 declare the two bool members inside #if USE(BUN_JSC_ADDITIONS); the other two use sites in Heap.cpp (updateAllocationLimits at 2651-2656 and setInitialAllocationBudget at 2815-2835) are correctly guarded. The follow-up commit that addressed the earlier review comment added lines 2721-2722 to Heap::setGarbageCollectionTimerEnabled() with no guard. When the tree is built with USE(BUN_JSC_ADDITIONS) off — the CI matrix and downstream ports that don't set -DUSE_BUN_JSC_ADDITIONS=ON — the compiler sees references to two undeclared class members and errors out. Base branch: setGarbageCollectionTimerEnabled() had no such references and compiled in both configurations.
Verification: normal — Heap.cpp:2718-2727 shows Heap::setGarbageCollectionTimerEnabled() is not inside any #if USE(BUN_JSC_ADDITIONS) block, and lines 2721-2722 unconditionally assign m_reenableEdenActivityCallback = false; / m_reenableFullActivityCallback = false;. In Heap.h the diff declares those two members inside #if USE(BUN_JSC_ADDITIONS) ... #endif (the block added at private: after… | normal…
…raph (#40816) ### What does this PR do? Two changes to how a `bun build --compile` executable treats the GC heap during startup. Depends on oven-sh/WebKit#533 (`Heap::setInitialAllocationBudget`), now merged; `WEBKIT_VERSION` is pinned to its merged SHA `d71031a973e6`. 1. **Don't collect while the embedded module graph is loading.** A compiled app decodes and evaluates its whole module graph before any of it can become garbage, but the heap starts with Bun's 8 MB budget: a large app pays an eden collection at 8 MB that frees nothing, which (via `minEdenToOldGenerationRatio`) forces a full re-mark at ~16 MB, then more edens from the allocation-paced timer. When a standalone graph is present, give the first cycle a budget equal to the embedded payload size (clamped 8–128 MB); after the first collection JSC's normal sizing applies. Small executables (payload < 8 MB) are unchanged. 2. **Don't `deleteAllUnlinkedCodeBlocks` + synchronous full GC after the entry point loads** when running from a standalone graph. `Run::start` does this to drop transpiler/linker garbage after `bun run`; a compiled app has none, the unlinked code blocks it deletes were just decoded from the bytecode cache (so they get decoded again on first call), and the collection lands on the main thread right as the program starts its real work. | Claude Code CLI 2.1.252, Linux x64 | before | after | |---|---|---| | collections before the REPL prompt | 4 (E 8 MB → F 16 → E 37 → E 51; ~18 ms pause) | **0** (first eden ≈ 60 MB, after first paint) | | `claude --help` (hyperfine ×30) | 177 ms | **165 ms** | | time to prompt (median of 20, ×2) | 442 / 455 ms | **427 / 433 ms** | | RSS at prompt | 197 MB | 203 MB | ### How did you verify your code works? `bundler_compile_splitting.test.ts`, `bun-build-compile.test.ts`; `BUN_JSC_logGC=1` on the CLI above (counts in the table); a compiled fixture that allocates ~100 MB of garbage at startup still collects it (`Bun.gc` + `process.memoryUsage`). --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Adds
Heap::setInitialAllocationBudget(size_t)underUSE(BUN_JSC_ADDITIONS).An embedder that is about to build a large object graph it knows is entirely live (Bun decoding and evaluating a compiled executable's whole precompiled module graph) can raise the first cycle's budget so the heap does not run an eden collection at the 8 MB default — which finds nothing to free and, via
minEdenToOldGenerationRatio, forces a full re-mark right after — and holds the allocation-pacedGCActivityCallbacktimers until that first collection. Once any collection has run (budget reached, explicit request, memory pressure), limits and timers are back to the normal rules;largeHeapSizeand the post-full minimum are untouched.For the Claude Code CLI this takes collections before the first prompt from 4 (E 8 MB, F 16 MB, E 37 MB, E 51 MB; ~18 ms of pause) to 0, with the first eden at ~60 MB after first paint. Used by oven-sh/bun (PR to follow).