diff --git a/patches/mimalloc/stats-aggregate-all-theaps.patch b/patches/mimalloc/stats-aggregate-all-theaps.patch new file mode 100644 index 000000000000..2d572e2353b7 --- /dev/null +++ b/patches/mimalloc/stats-aggregate-all-theaps.patch @@ -0,0 +1,21 @@ +--- a/src/stats.c ++++ b/src/stats.c +@@ -618,7 +618,17 @@ + + static bool mi_cdecl mi_heap_aggregate_visitor(mi_heap_t* heap, void* arg) { + mi_stats_t* stats = (mi_stats_t*)arg; +- mi_stats_add_into(stats, mi_heap_get_stats(heap)); ++ // Fold in `heap->stats` plus every thread's theap, not just the caller's: the NULL-theap ++ // path in `_mi_arenas_page_free` decrements `heap->stats.pages` for a page whose +1 may ++ // still sit in another thread's unmerged theap, so summing only `heap->stats` underreports. ++ // Pure read (no merge-and-zero) so the aggregate is a side-effect-free snapshot. Lock order ++ // `subproc->heaps_lock` -> `heap->theaps_lock` matches `mi_prof_set_all_theaps`. ++ mi_lock(&heap->theaps_lock) { ++ mi_stats_add_into(stats, &heap->stats); ++ for (mi_theap_t* t = heap->theaps; t != NULL; t = t->hnext) { ++ mi_stats_add_into(stats, &t->stats); ++ } ++ } + return true; + } + diff --git a/scripts/build/deps/mimalloc.ts b/scripts/build/deps/mimalloc.ts index d27c39dadedf..a5b4acf0c17f 100644 --- a/scripts/build/deps/mimalloc.ts +++ b/scripts/build/deps/mimalloc.ts @@ -24,6 +24,8 @@ export const mimalloc: Dependency = { commit: MIMALLOC_COMMIT, }), + patches: ["patches/mimalloc/stats-aggregate-all-theaps.patch"], + build: cfg => { // ─── Override behavior (global malloc replacement) ─── // ASAN: OFF — ASAN interceptors must see the real malloc. diff --git a/test/js/bun/jsc/heapStats-mimalloc.test.ts b/test/js/bun/jsc/heapStats-mimalloc.test.ts index b6f083c881d3..22d2634f8417 100644 --- a/test/js/bun/jsc/heapStats-mimalloc.test.ts +++ b/test/js/bun/jsc/heapStats-mimalloc.test.ts @@ -1,5 +1,6 @@ import { heapStats } from "bun:jsc"; import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; describe("heapStats() mimalloc integration", () => { test("mimalloc aggregate stats are present", () => { @@ -45,6 +46,61 @@ describe("heapStats() mimalloc integration", () => { } }); + // `pages.current` must account for pages allocated by every thread's theap, not just the + // caller's: the increment lands in the allocating thread's theap, while a cross-thread free + // of an abandoned page decrements heap->stats directly. Summing only the caller's theap + // underreports by the pages other threads allocated (and can go negative under churn). + test("pages.current agrees with the live heap dump across threads", async () => { + using dir = tempDir("heapStats-mimalloc-pages", { + "check.js": ` + import { heapStats } from "bun:jsc"; + const theapsBefore = heapStats().mimalloc.theaps?.current ?? 0; + const code = \` + const hold = []; + for (let i = 0; i < 50000; i++) hold.push({ a: i, b: "str_" + i }); + self.postMessage("ready"); + self.onmessage = () => {}; + \`; + const url = URL.createObjectURL(new Blob([code], { type: "application/javascript" })); + const w = new Worker(url); + await new Promise(r => { w.onmessage = r; }); + const s = heapStats({ dump: true }); + let dump = 0; + for (const h of s.mimallocDump.heaps) dump += h.pages.length; + const stat = s.mimalloc.pages.current; + const binSum = s.mimalloc.page_bins.reduce((a, b) => a + b.current, 0); + const theaps = s.mimalloc.theaps?.current ?? 0; + console.log(JSON.stringify({ stat, dump, binSum, theapsBefore, theaps })); + w.terminate(); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "check.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // Surface stderr/exitCode before parsing so a subprocess crash reports the real diagnostic + // instead of a bare JSON SyntaxError. + expect({ stdout: stdout.trim() || null, stderr, exitCode }).toMatchObject({ + stdout: expect.any(String), + exitCode: 0, + }); + const { stat, dump, binSum, theapsBefore, theaps } = JSON.parse(stdout); + // The Worker allocates on its own thread; if that didn't create a new theap the assertion + // below would be vacuous (only the caller's theap exists, which the old aggregate already + // folded in). The static main theap is not counted, so before is 0 in practice. + expect({ theapsBefore, theaps }).toSatisfy(v => v.theaps > v.theapsBefore); + // The live heap walk is ground truth. Bracket the counter: the lower bound catches the + // pre-fix underreport (~-36 here), the upper bound catches a double-fold. Slack covers the + // few pages that can change between the two reads plus a concurrent merge-and-zero. + expect(stat).toBeGreaterThanOrEqual(0); + expect({ stat, dump }).toSatisfy(v => v.stat >= v.dump - 5 && v.stat <= v.dump + 10); + expect({ binSum, dump }).toSatisfy(v => v.binSum >= v.dump - 5 && v.binSum <= v.dump + 10); + }); + test("dump reflects new heaps and allocations", () => { const before = heapStats({ dump: true }).mimallocDump.heaps.length; // MimallocArena is internal; trigger via something that creates a heap.