Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions patches/mimalloc/stats-aggregate-all-theaps.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
--- a/src/stats.c
+++ b/src/stats.c
@@ -619,6 +619,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 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.
+ // `mi_heap_get_stats` above already merged (and zeroed) the caller's theap, so including
+ // it again is a no-op. Lock order `subproc->heaps_lock` -> `heap->theaps_lock` matches
+ // `mi_prof_set_all_theaps` and the fork-prepare path.
+ mi_lock(&heap->theaps_lock) {
+ for (mi_theap_t* t = heap->theaps; t != NULL; t = t->hnext) {
+ mi_stats_add_into(stats, &t->stats);
+ }
+ }
return true;
}

2 changes: 2 additions & 0 deletions scripts/build/deps/mimalloc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
49 changes: 49 additions & 0 deletions test/js/bun/jsc/heapStats-mimalloc.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand Down Expand Up @@ -45,6 +46,54 @@ 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 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, 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]);
expect(stderr).toBe("");
Comment thread
robobun marked this conversation as resolved.
Outdated
const { stat, dump, binSum, theaps } = JSON.parse(stdout);
// A Worker allocates on its own thread and must create at least one theap; without that
// precondition the assertion below would be vacuous.
expect(theaps).toBeGreaterThan(0);
Comment thread
robobun marked this conversation as resolved.
Outdated
// The live heap walk is ground truth. A small slack covers the few pages that can change
// between the two reads; before the fix the counter sat tens of pages below the walk here.
expect(stat).toBeGreaterThanOrEqual(0);
expect({ stat, dump }).toSatisfy(v => v.stat >= v.dump - 5);
expect({ binSum, dump }).toSatisfy(v => v.binSum >= v.dump - 5);
expect(exitCode).toBe(0);
});

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.
Expand Down
Loading