Skip to content

process.memoryUsage: read heapUsed through Bun::heapUsage() and remove the observer that copied it - #43925

Open
robobun wants to merge 3 commits into
mainfrom
robobun/4425bb83/heap-usage-helper
Open

robobun wants to merge 3 commits into
mainfrom
robobun/4425bb83/heap-usage-helper

Conversation

@robobun

@robobun robobun commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Behaviour change: none

Groundwork for #43926, the node:v8 follow-up to #30596. It does not depend on #30596.

Problem

Fix

  • Bun::heapUsage(vm) (BunProcess.h, BunProcess.cpp) returns { total, used }: blockBytesAllocated(), and sizeAfterLastCollection() with total as the fallback before the first collection. Process_functionMemoryUsage reports both from it.
  • Removed: the observer class, its accessor, member and initialiser, and the HeapObserver.h include. BunProcess.cpp was the only reader.
  • Correct because Heap::updateAllocationLimits() stores one value in both counters before the heap notifies observers. The copy always equalled the accessor.
  • Existing tests that cover this code, unchanged and green: process.test.js (describe "process.memoryUsage().heapUsed reports the most recent collection"), v8-module.test.ts, test-v8-collect-gc-profile*.js, worker.test.ts, worker_threads.test.ts.

Background

Downsides

  • None found. Release builds of 73df7bb and this PR (linux x64): text 80676462 -> 80675950 B, sizeof(JSVMClientData) 6160 -> 6136 B, observer calls per collection 2 -> 0. process.memoryUsage() keeps four relations to the collector's own numbers in 30 of 30 runs of each build.
Notes
  • Credit: the deletion is the bindings half of Jarred-Sumner's ae0652c (Evict code a parked process does not use: idle by allocation, module graph at 2 min, the executable's own pages and rebuildable bytecode at 10 min #42383). GC controller: idle collections at 10 s, 2 min and 10 min, nothing paged out; the last drops re-decodable bytecode #43174 replaced that PR and does not touch these files. The commit here carries a Co-authored-by trailer. [JSC] Less memory per function that never runs, and code that can be dropped and decoded again WebKit#628 added the accessor.
  • Proof of no behaviour change, at the pinned WebKit 299c5323: Heap::m_sizeAfterLastCollect has one writer (Heap.cpp:3181). It stores the same currentHeapSize as m_sizeAfterLastFullCollect (:3157) or m_sizeAfterLastEdenCollect (:3167), inside updateAllocationLimits(). didFinishCollection() calls the observers later (:3225). The observer read the counter of the scope it was given, so it always stored the value the accessor returns.
  • Four states, checked with BUN_GC_TIMER_DISABLE=1, 30 runs per build. All four relations hold in 60 of 60 runs, and both builds print the same values:
    • At start-up heapUsed === heapTotal > 0 (851968).
    • After Bun.gc(true), heapUsed equals its return value (254404).
    • After bun:jsc edenGC(), heapUsed equals its return value (254516).
    • After 64 x 1 MB ArrayBuffer, heapUsed equals the usedHeapSize of the last collection that a v8.GCProfiler recorded (54792651).
    • In 4 of 30 runs on main and 2 of 30 on this PR the full collection keeps 112 B more, and heapUsed follows it.
  • Symbols (nm -S on bun-profile): Process_functionMemoryUsage 1311 -> 1312 B. Bun::heapUsage is inlined. The four HeapSizeAfterLastCollection functions (250 B) and the vtable (48 B) are gone.
  • sizeof(WebCore::JSVMClientData) comes from a static_assert probe compiled with the release flags of BunClientData.cpp. The 24 B are the vtable pointer, the Heap& and the counter.
  • Observer calls: on main a breakpoint on HeapSizeAfterLastCollection::didGarbageCollect fires 1000 times over 1000 edenGC() calls. The heap calls willGarbageCollect() on the same observers when a collection starts (Heap.cpp:3052). On this PR the symbols do not exist.
  • Start-up: Heap::m_observers has no inline capacity, so the first addObserver() allocates 16 slots (128 B). During bun -e 'console.log(1)' the folded Vector<T*, 0, CrashOnOverflow, 16>::expandCapacity runs 12 times on main and 11 times here. No always-on observer remains: grep -rn addObserver src/jsc/bindings finds only GCProfilerObserver.
  • Open worker_threads: answer worker.cpuUsage() and worker.getHeapStatistics() on the parent thread #42360 adds a JSC::HeapObserver subclass to BunClientData.h and a member next to m_heapSizeAfterLastCollection. Whichever PR lands second resolves one hunk there, and worker_threads: answer worker.cpuUsage() and worker.getHeapStatistics() on the parent thread #42360 then needs its own HeapObserver.h include.
  • Open fix(memory): include allocations since the last collection in heap statistics #44548 (from a fork) keeps this observer and adds a second counter to it, Heap::totalBytesAllocated() at the last collection, so that heapUsed follows allocation between collections. If that value is wanted, the observer stays, or a JSC accessor for the bytes allocated in the current cycle replaces it. The sum then goes into Bun::heapUsage(). Whichever PR lands second needs a rebase.
  • Suites run on the debug build: process.test.js 180 pass and 5 skip (it needs USER in the environment). v8-module.test.ts 8 pass. test-v8-collect-gc-profile.js, -using.js, -exit-before-stop.js and test-v8-stats.js exit 0. worker.test.ts 42 pass. worker_threads.test.ts 142 pass. bun-jsc.test.ts 41 pass. globals.test.js 20 pass.

no test proof · iteration 1 · the description declares no behaviour change, so there is no failing test to prove; the existing suite in CI is the check

…on() through Bun::heapUsage()

Bun::heapUsage(vm) returns { total, used }: the allocated block bytes, and the
size JSC stored at the end of the most recent collection with total as the
fallback until the first collection finishes. Process_functionMemoryUsage
reports both from it.

The HeapSizeAfterLastCollection observer copied the same counter in
didGarbageCollect(). JSC has an accessor for it now, so the observer, its
member in JSVMClientData and the HeapObserver.h include are removed.

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
@robobun

robobun commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 7:54 PM PT - Sep 24th, 2026

✅ @robobun, your commit 6c627f21ac11713f201ab303b1c2f0d0afa556d9 passed in Build #120504! 🎉


🧪   To try this PR locally:

bunx bun-pr 43925

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

bun-43925 --bun

@robobun

robobun commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review. CI is green.

How it was checked: release builds of main (73df7bb) and of this branch. process.memoryUsage() keeps the same four relations to the collector's numbers on both (30 runs each), and the existing tests pass unchanged on the debug build.

CI: build 120504 of 6c627f2 passed on all 181 jobs. 6c627f2 is an empty commit: build 120455 ran during an outage of gitlab.com and was red in tests that clone from it.

Against current main: the branch merges into main (5866b5f) with no conflict. Main took two upstream WebKit merges (#44615, #45303). The collector moved into its own class, and the stored size now sums the bytes marked, not the bytes visited. Heap::updateAllocationLimits() still stores that size for both collection scopes before the heap notifies observers. Checked on that main with a temporary assertion: the observer's copy equals Heap::sizeAfterLastCollection() in all 36000 process.memoryUsage() calls of a probe (main thread and Worker, eden and full collections). A local debug + ASAN build of the merge with #30596 and #43926 passes v8-module.test.ts (16 of 16), the memoryUsage tests of process.test.js (7 pass, 1 skip) and stubs.test.js (378 of 378). Read again at ea6a2c7 (WebKit 274ebb3cf6): the heap sources are the same, no commit touches the code of this PR, and the branch still merges with no conflict.

PRs: #43925 (this PR) and #43926 (the node:v8 change that reads the new helper, stacked on #30596). Open #44548 (from a fork) keeps the observer that this PR removes. The Notes of this PR say how the two compose.

@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 4c859a2c-a2a0-47ca-b039-cfbe74843ec9

📥 Commits

Reviewing files that changed from the base of the PR and between ddbd24e and 802eaf7.

📒 Files selected for processing (1)
  • src/jsc/bindings/BunProcess.cpp

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Walkthrough

The change replaces the per-VM heap-size observer with a heapUsage helper. Process_functionMemoryUsage uses allocated heap bytes for heapTotal and the last-collection size for heapUsed. If the last-collection size is zero, the helper uses allocated heap bytes for heapUsed.

Changes

Heap usage reporting

Layer / File(s) Summary
Heap usage calculation and process reporting
src/jsc/bindings/BunProcess.h, src/jsc/bindings/BunProcess.cpp, src/jsc/bindings/BunClientData.h, src/jsc/bindings/BunClientData.cpp
Adds the HeapUsage structure and heapUsage helper. Process_functionMemoryUsage uses the helper for heapTotal and heapUsed. Removes the prior observer class, accessor, and stored observer member.

Suggested reviewers: cirospaciari

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 802ea

No actionable regression is established. The change is mergeable after normal build and test checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main change: routing process.memoryUsage heap data through Bun::heapUsage() and removing the redundant observer.
Description check Passed The description explains the problem, fix, behavior preservation, rationale, downsides, and verification results. It does not use the template headings exactly, but it provides the required change sum…
Full details: Description check

Explanation

The description explains the problem, fix, behavior preservation, rationale, downsides, and verification results. It does not use the template headings exactly, but it provides the required change summary and verification information.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review found no issues

No high-confidence issues detected in this change.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review found no issues

No high-confidence issues detected in this change.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants