Skip to content

cli: --heap-prof node parity, and fix absolute profile paths panicking (+2 tests) - #34842

Merged
cirospaciari merged 2 commits into
claude/callback-throw-uncaughtfrom
claude/heap-prof-cli-parity
Jul 20, 2026
Merged

cli: --heap-prof node parity, and fix absolute profile paths panicking (+2 tests)#34842
cirospaciari merged 2 commits into
claude/callback-throw-uncaughtfrom
claude/heap-prof-cli-parity

Conversation

@cirospaciari

Copy link
Copy Markdown
Member

Extends the cpu-prof CLI parity on the base branch to --heap-prof, and fixes a real crash. Stacked on #34660.

The honest ceiling, established first

JSC has no allocation-sampling profiler. HeapProfiler.h in the prebuilt WebKit is snapshots-only, SamplingProfiler.h is CPU stack sampling, and nm libJavaScriptCore.a exports no sampling-heap symbols. A generated bun heap snapshot carries trace_function_infos: [] — zero allocation-site data exists anywhere.

Node's .heapprofile (measured on the v26.3.0 binary) has real V8-attributed call frames even for -e '0', and 9 of the 11 upstream tests assert a runAllocation frame in the tree. That cannot be produced honestly, so those 9 stay unconverted — the profile bun writes reports the real live-heap size on the (root) frame with no synthetic frames, which is V8's own shape for unattributed allocations.

What's here

  • --heap-prof-interval registered as value-taking (previously 128 parsed as the script name — verified); name/dir/interval without --heap-prof exit 9 with node's exact message, mirroring the base branch's cpu-prof block; default-interval noop like node.
  • Node's Heap.<yyyymmdd>.<hhmmss>.<pid>.0.<seq>.heapprofile filename in local time — the old name had the same non-wall-clock Timespec bug the CPU side fixed (it produced Heap.1540804227270…).
  • Flush on process.exit and self-directed fatal signals via the hook the cpu profiler already uses (Bun__writeCPUProfileBeforeSelfKillBun__writeProfilesBeforeSelfKill).
  • .heapsnapshot capability preserved via v8.writeHeapSnapshot / Bun.generateHeapSnapshot("v8") (smoke-tested).

Real crash fixed

An absolute --heap-prof-dir, --heap-prof-name — or --cpu-prof-name, already shipped on the base branch — hit assertion failed: !is_input_absolute in AutoAbsPath::append and died with exit 134. Both writers now join resolve-style. Verified: absolute paths write the profile and exit 0.

Tests

test-heap-prof-basic and test-heap-prof-invalid-args, verbatim, node-ceiling-checked, 3× green, failing on system bun as control, tamper-checked. The 9 unconvertible files fail only on verifyFrames('runAllocation') — exit codes, dir creation, and SIGINT/exit-55 flush all work.

Regressions: cpu-prof.test.ts 9/9, the base branch's 6 vendored cpu-prof tests green, test/js/node/process unchanged, v8 heap-snapshot smoke OK. cargo check clean on windows-msvc and linux-gnu targets. Bun-side test/cli/heap-prof.test.ts rewritten for the new behavior (13 pass).

--heap-prof-name/-dir/-interval without --heap-prof now exit 9 with node's
"<argv0>: <flag> must be used with --heap-prof" instead of warning and
continuing; --heap-prof-interval was previously not registered at all, so
`bun --heap-prof-interval 128 x.js` parsed 128 as the script. An interval
equal to node's default (512 KiB) is a noop without --heap-prof, like node.

--heap-prof now writes node's V8 sampling-heap-profile JSON
({"head": {...}, "samples": [...]}) under node's filename format
Heap.<yyyymmdd>.<hhmmss>.<pid>.<tid>.<seq>.heapprofile in local time (the old
name also hit the same non-wall-clock Timespec bug the CPU side fixed).
JavaScriptCore has no allocation-site sampler, so the profile reports the
real live-heap size on the (root) frame with no per-function attribution;
the full .heapsnapshot remains available via v8.writeHeapSnapshot() and
Bun.generateHeapSnapshot("v8"). The markdown format (--heap-prof-md) is
unchanged. Heap profiles also flush on a self-directed fatal signal via
process.kill, sharing the CPU profiler's hook.

Writing to an absolute --heap-prof-dir or --heap-prof-name (and an absolute
--cpu-prof-name) previously hit a debug assertion in AutoAbsPath::append;
both writers now use resolve-style join.

Adds two upstream Node v26.3.0 tests, verbatim. The other nine
test-heap-prof-* files assert a runAllocation call frame inside the profile
tree (or per-worker profiles), which requires allocation-stack sampling JSC
does not provide, so they are not vendored.
@robobun

robobun commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator
Updated 1:07 PM PT - Jul 20th, 2026

@autofix-ci[bot], your commit ac412e4 has 3 failures in Build #76349 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 34842

That installs a local version of the PR into your bun-34842 executable, so you can run:

bun-34842 --bun

@github-actions

Copy link
Copy Markdown
Contributor

Found 2 issues this PR may fix:

  1. --heap-prof-dir mishandles absolute paths #32568 - Directly fixes the reported bug: --heap-prof-dir with absolute paths panics because append was used instead of join for path construction
  2. CPU profile not saved when process exits with signal #24787 - PR adds heap profile flushing before self-directed fatal signals (e.g., process.kill(process.pid, "SIGINT")), which addresses the same class of problem (profile not saved on signal exit)

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #32568
Fixes #24787

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. Honor absolute paths for heap and CPU profiler --*-dir/--*-name options #32570 - Also fixes absolute path handling for --heap-prof-dir, --heap-prof-name, --cpu-prof-dir, and --cpu-prof-name profiler options (the AutoAbsPath append → join fix)

🤖 Generated with Claude Code

@cirospaciari
cirospaciari merged commit 2a0e723 into claude/callback-throw-uncaught Jul 20, 2026
72 of 76 checks passed
@cirospaciari
cirospaciari deleted the claude/heap-prof-cli-parity branch July 20, 2026 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants