Repository navigation
perf(standalone): madvise(MADV_DONTNEED) embedded source pages after entrypoint load - #29320
Conversation
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Updated 4:31 PM PT - Apr 21st, 2026
❌ @cirospaciari, your commit 4969ef7 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 29320That installs a local version of the PR into your bun-29320 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a non-Windows public Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is technically correct and conservative — MADV_DONTNEED is a safe hint. However, since a competing PR (#29313) covers the same ground with an additional mmap(MAP_FIXED) overlay for macOS, a human should decide which variant (or combination) to land.
Extended reasoning...
Overview
The PR adds (~30 lines in StandaloneModuleGraph.zig) and calls it from after the entrypoint is loaded and initial microtasks drain. It uses to hint the kernel that the embedded source section pages are no longer hot. Windows is excluded at comptime; errors are caught and emitted as debug-only warnings (non-fatal).
Security risks
None. is a pure hint — it changes no mapping protections, cannot cause SIGSEGV/SIGBUS, and does not alter backing store. The implementation does not expose any user-controlled data to the syscall.
Level of scrutiny
Low-risk, focused change. The logic is self-contained, reuses existing helpers, and the madvise call is guarded against failure. The call site is appropriate (after the microtask drain, before the watcher loop).
Other factors
A bot flagged this as a potential duplicate of PR #29313, which implements the same optimization but additionally uses on macOS for a more aggressive reclaim. Both PRs are intentionally open so the tradeoff can be evaluated. That design decision — conservative madvise-only vs. more aggressive mmap overlay — requires a human call on which variant(s) to merge.
- std.posix.madvise hits `unreachable` on unexpected errnos; this is a best-effort hint, so call libc directly and just log on failure. - Add a debug-only test that compiles a TLA entrypoint as a standalone binary and asserts the hintSourcePagesDontNeed debugWarn fires — verifying the call site at bun.js.zig:466 is reached even when the entrypoint promise is still pending. loadEntryPoint() returns without blocking, so control falls through to the madvise before the main event loop spins.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/js/bun/compile/standalone-madvise-tla.test.ts`:
- Around line 41-42: The test currently asserts that stderr contains the exact
string "hintSourcePagesDontNeed: MADV_DONTNEED", which is brittle because
hintSourcePagesDontNeed() is best-effort; change the assertion to be tolerant by
checking that stderr indicates the callsite was reached rather than only the
success text — e.g., replace the strict
expect(stderr).toContain("hintSourcePagesDontNeed: MADV_DONTNEED") with a check
that stderr contains "hintSourcePagesDontNeed:" and then assert it either
includes "MADV_DONTNEED" or the failure variant (use an OR-style check or a
regex) so the test passes when the function logs success or a best-effort
failure; keep the exitCode assertion as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b30a01e1-a0f9-456f-8f28-187c9043e39a
📒 Files selected for processing (2)
src/StandaloneModuleGraph.zigtest/js/bun/compile/standalone-madvise-tla.test.ts
…sertion - Comment at the call site now says "initial synchronous evaluation is done (TLA may still be pending)" instead of claiming microtasks are drained, which is inaccurate when the entrypoint awaits. - Skip the hint under --watch/--hot since those re-read source on every reload (degenerate for standalone, but cheap to be defensive). - Test asserts the "hintSourcePagesDontNeed:" prefix so either the success or failure debugWarn variant proves the call site is reached.
There was a problem hiding this comment.
LGTM — all prior review feedback has been addressed in the latest commit.
Extended reasoning...
Overview
This PR adds StandaloneModuleGraph.hintSourcePagesDontNeed() in src/StandaloneModuleGraph.zig, calls it from src/bun.js.zig after entrypoint load, and adds a TLA-specific test. The change is scoped to standalone compiled binaries on macOS/Linux only.
Security risks
None. madvise(MADV_DONTNEED) is a kernel hint with no effect on mapping validity or protection bits. No sensitive data is exposed, no auth code is touched, and there is no window of inaccessibility. The call is purely advisory.
Level of scrutiny
Low-to-medium. This is a performance optimization that is safe by design (hint-only, file-backed pages fault back in transparently). The correctness risk is essentially zero.
Other factors
All three issues raised in prior reviews have been addressed in the latest commit: (1) the --watch/--hot guard is present, (2) the TLA comment is accurate, and (3) std.c.madvise replaces the std.posix call with a documented rationale. The test assertion is appropriately tolerant. No bugs were found by the automated system.
…entrypoint load (oven-sh#29320) ### What does this PR do? For compiled standalone binaries (`bun build --compile`), call `madvise(MADV_DONTNEED)` on the embedded `__BUN,__bun` (Mach-O) / `.bun` (ELF) section once the entrypoint module has been parsed and the initial microtasks have drained. The section holds the bundled JS source text; after JSC has parsed it into bytecode the source pages are only re-read for stack-trace line lookup or lazy `require()` of modules not yet loaded, both of which fault back in transparently from the file-backed mapping. Does nothing in the regular `bun` interpreter (no embedded section) or on Windows. ### Why is this safe `MADV_DONTNEED` is a hint to the kernel — it doesn't change the mapping, protection, or backing store. Pages remain valid and reads continue to work whether or not the kernel reclaims them. There is no window where the range is inaccessible, and the call cannot introduce a SIGSEGV/SIGBUS that wouldn't have happened anyway. How aggressively the kernel acts on the hint is platform- and pressure-dependent; the call is correct on every POSIX target regardless. The pages are clean and file-backed, so a later read faults them back in from the executable on disk at page-fault cost. ### Implementation `StandaloneModuleGraph.hintSourcePagesDontNeed()` reuses the existing `Macho.getData()` / `ELF.getData()` section lookup, page-aligns the range, and calls `std.posix.madvise(.., MADV.DONTNEED)`. Called from `bun.js.zig` immediately after the post-entrypoint `releaseWeakRefs/runGC/vm.tick()` block. ### Relationship to oven-sh#29313 This is the conservative variant — `madvise` only, on both macOS and Linux. oven-sh#29313 additionally uses `mmap(MAP_FIXED)` overlay on macOS where `madvise` is treated as a hint that may not immediately reclaim file-backed pages. Both PRs are open so the tradeoff can be evaluated independently. ### How did you verify your code works? `zig build check-all` passes (macOS/Linux/Windows semantic analysis).
…entrypoint load (oven-sh#29320) ### What does this PR do? For compiled standalone binaries (`bun build --compile`), call `madvise(MADV_DONTNEED)` on the embedded `__BUN,__bun` (Mach-O) / `.bun` (ELF) section once the entrypoint module has been parsed and the initial microtasks have drained. The section holds the bundled JS source text; after JSC has parsed it into bytecode the source pages are only re-read for stack-trace line lookup or lazy `require()` of modules not yet loaded, both of which fault back in transparently from the file-backed mapping. Does nothing in the regular `bun` interpreter (no embedded section) or on Windows. ### Why is this safe `MADV_DONTNEED` is a hint to the kernel — it doesn't change the mapping, protection, or backing store. Pages remain valid and reads continue to work whether or not the kernel reclaims them. There is no window where the range is inaccessible, and the call cannot introduce a SIGSEGV/SIGBUS that wouldn't have happened anyway. How aggressively the kernel acts on the hint is platform- and pressure-dependent; the call is correct on every POSIX target regardless. The pages are clean and file-backed, so a later read faults them back in from the executable on disk at page-fault cost. ### Implementation `StandaloneModuleGraph.hintSourcePagesDontNeed()` reuses the existing `Macho.getData()` / `ELF.getData()` section lookup, page-aligns the range, and calls `std.posix.madvise(.., MADV.DONTNEED)`. Called from `bun.js.zig` immediately after the post-entrypoint `releaseWeakRefs/runGC/vm.tick()` block. ### Relationship to oven-sh#29313 This is the conservative variant — `madvise` only, on both macOS and Linux. oven-sh#29313 additionally uses `mmap(MAP_FIXED)` overlay on macOS where `madvise` is treated as a hint that may not immediately reclaim file-backed pages. Both PRs are open so the tradeoff can be evaluated independently. ### How did you verify your code works? `zig build check-all` passes (macOS/Linux/Windows semantic analysis).
…ce (#37357) ### What does this PR do? Adds `BUN_FEATURE_FLAG_DISABLE_STANDALONE_MADVISE`, a runtime opt-out for the behavior introduced in #29320: a `bun build --compile` executable calls `madvise(MADV_DONTNEED)` on its embedded source section once the entrypoint has loaded, so anything that touches the source afterwards (a lazy `require()`, resolving a stack trace) pages it back in from the executable on disk. Deployments where that re-read is undesirable can set the flag in the environment of the running executable to keep the pages resident: ```sh BUN_FEATURE_FLAG_DISABLE_STANDALONE_MADVISE=1 ./myapp ``` The compiled binary reads it at runtime, nothing is baked in at compile time. It follows the existing `BUN_FEATURE_FLAG_DISABLE_*` escape hatches: registered in `env_var.rs`, checked in `hint_source_pages_dont_need()` (which returns early, sharing the existing `len == 0` return), falsy values such as `0` keep the default. No effect on Windows or on the plain interpreter, which never issue the hint. Like the other `BUN_FEATURE_FLAG_DISABLE_*` escape hatches it is not documented. #37356 is the alternative of removing the madvise call altogether; the two are mutually exclusive. ### How did you verify your code works? `test/js/bun/compile/standalone-madvise-tla.test.ts` now runs the same compiled binary three times with `BUN_DEBUG_StandaloneModuleGraph=1`: with the variable unset (explicitly removed from the child env so an ambient value on the host cannot leak in), set to `0`, and set to `1`. The first two must log a `hintSourcePagesDontNeed:` line, the last must not log one at all, since the flag path returns before logging anything. The test stays debug-only because it relies on the scoped logger. Against a debug build without this change the `1` case fails as expected: ``` Expected to not contain: "hintSourcePagesDontNeed:" Received: "before-await\nafter-await\n[standalonemodulegraph] hintSourcePagesDontNeed: MADV_DONTNEED 4096 bytes\n" ``` With the change, `bun bd test test/js/bun/compile/standalone-madvise-tla.test.ts` passes (22 assertions). <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/compile/standalone-madvise-tla.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
2 participants
What does this PR do?
For compiled standalone binaries (
bun build --compile), callmadvise(MADV_DONTNEED)on the embedded__BUN,__bun(Mach-O) /.bun(ELF) section once the entrypoint module has been parsed and the initial microtasks have drained. The section holds the bundled JS source text; after JSC has parsed it into bytecode the source pages are only re-read for stack-trace line lookup or lazyrequire()of modules not yet loaded, both of which fault back in transparently from the file-backed mapping.Does nothing in the regular
buninterpreter (no embedded section) or on Windows.Why is this safe
MADV_DONTNEEDis a hint to the kernel — it doesn't change the mapping, protection, or backing store. Pages remain valid and reads continue to work whether or not the kernel reclaims them. There is no window where the range is inaccessible, and the call cannot introduce a SIGSEGV/SIGBUS that wouldn't have happened anyway. How aggressively the kernel acts on the hint is platform- and pressure-dependent; the call is correct on every POSIX target regardless.The pages are clean and file-backed, so a later read faults them back in from the executable on disk at page-fault cost.
Implementation
StandaloneModuleGraph.hintSourcePagesDontNeed()reuses the existingMacho.getData()/ELF.getData()section lookup, page-aligns the range, and callsstd.posix.madvise(.., MADV.DONTNEED). Called frombun.js.zigimmediately after the post-entrypointreleaseWeakRefs/runGC/vm.tick()block.Relationship to #29313
This is the conservative variant —
madviseonly, on both macOS and Linux. #29313 additionally usesmmap(MAP_FIXED)overlay on macOS wheremadviseis treated as a hint that may not immediately reclaim file-backed pages. Both PRs are open so the tradeoff can be evaluated independently.How did you verify your code works?
zig build check-allpasses (macOS/Linux/Windows semantic analysis).