From d4f7c75cf65343cb41567b2c1635ab5790b9c355 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 20 Jul 2026 06:53:22 +0000 Subject: [PATCH 1/2] test(HTMLRewriter): compare peak RSS, not a point sample that straddles a segment decommit The handler-allocation leak test sampled rss() once after the third pass and once after the sixth. mimalloc decommits a ~30 MB segment on its own cadence between passes, so those two point samples can land on opposite sides of the release: before in the trough, after at the peak reads as a +31 MB 'leak' with nothing retained. 4 attempts on the 4-vCPU Windows CI lane returned 30.6 / 31.0 / 31.0 / 31.2 MB against a 25 MB bound. Compare the peak over each 3-pass window instead. A real leak raises the peak every pass (~50 MB over 3); freed memory leaves it flat regardless of when the decommit fires. Same six passes, same bound. Measured delta is now <1 MB across 20 Windows + 10 Linux runs. --- test/js/workerd/html-rewriter-leak.test.ts | 26 +++++++++++++--------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/test/js/workerd/html-rewriter-leak.test.ts b/test/js/workerd/html-rewriter-leak.test.ts index c51e13d93983..8ba9c5b6c016 100644 --- a/test/js/workerd/html-rewriter-leak.test.ts +++ b/test/js/workerd/html-rewriter-leak.test.ts @@ -47,11 +47,13 @@ test("onEndTag callbacks are released after the rewrite", () => { // LOLHTMLContext.deinit() must destroy those allocations. Previously it only // unprotected the held JSValues and leaked the struct memory. // -// RSS is a high-water mark — Bun.gc(true) collects every wrapper and its -// lol-html builder, but the allocators don't promptly hand pages back to the -// OS. So warmup runs the *same* workload as the measured phase: the allocator -// footprint is established before the baseline, and any growth past that is -// what's actually retained. +// RSS is a high-water mark across the allocator's committed segments. mimalloc +// hands a ~30 MB segment back to the OS on its own schedule, so a single +// `Bun.gc(true); rss()` sample lands on either side of that release — the +// 4-vCPU Windows CI lane saw `after` at the peak and `before` in the trough +// for a +31 MB "delta" with nothing retained. Per-window *peak* RSS is the +// stable observable: a leak raises it every pass (~50 MB over 3), while freed +// memory leaves it flat regardless of when the decommit fires. // // Skipped in debug: at this N a debug pass is ~40s and the extra debug-build // allocation tracking adds enough RSS noise to drown the signal. CI has no @@ -76,10 +78,14 @@ test.skipIf(isDebug)( return process.memoryUsage.rss(); } - pass(); pass(); - const before = pass(); - pass(); pass(); - const after = pass(); + function peakOver(passes) { + let peak = 0; + for (let i = 0; i < passes; i++) peak = Math.max(peak, pass()); + return peak; + } + + const before = peakOver(3); + const after = peakOver(3); process.stdout.write( JSON.stringify({ before, after, deltaMB: (after - before) / 1024 / 1024 }) + "\\n", @@ -113,7 +119,7 @@ test.skipIf(isDebug)( const { deltaMB } = JSON.parse(stdout.trim()); - // Unfixed: ~50 MB over 3 measured passes. Fixed: ±1 MB plateau. + // Unfixed: ~50 MB over 3 measured passes. Fixed: <1 MB. // Threshold sits at ~half the unfixed signal. expect(deltaMB).toBeLessThan(25); expect(exitCode).toBe(0); From 598fcfb51e1cb736cfcee53320986e6849bceff6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 20 Jul 2026 07:25:53 +0000 Subject: [PATCH 2/2] use kernel maxRSS instead of per-pass peak sampling process.resourceUsage().maxRSS is the kernel's monotone peak (VmHWM on Linux, PeakWorkingSetSize on Windows, ru_maxrss on macOS), so it can never miss a peak between sample points the way a userland max-of-N can if mimalloc's decommit cadence shifts. Also emit the per-pass rss() samples in the fixture's JSON and log it before the assertion, so a future red carries the trace in the CI log. --- test/js/workerd/html-rewriter-leak.test.ts | 34 ++++++++++++---------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/test/js/workerd/html-rewriter-leak.test.ts b/test/js/workerd/html-rewriter-leak.test.ts index 8ba9c5b6c016..4976410c0cef 100644 --- a/test/js/workerd/html-rewriter-leak.test.ts +++ b/test/js/workerd/html-rewriter-leak.test.ts @@ -47,13 +47,13 @@ test("onEndTag callbacks are released after the rewrite", () => { // LOLHTMLContext.deinit() must destroy those allocations. Previously it only // unprotected the held JSValues and leaked the struct memory. // -// RSS is a high-water mark across the allocator's committed segments. mimalloc -// hands a ~30 MB segment back to the OS on its own schedule, so a single -// `Bun.gc(true); rss()` sample lands on either side of that release — the -// 4-vCPU Windows CI lane saw `after` at the peak and `before` in the trough -// for a +31 MB "delta" with nothing retained. Per-window *peak* RSS is the -// stable observable: a leak raises it every pass (~50 MB over 3), while freed -// memory leaves it flat regardless of when the decommit fires. +// `process.memoryUsage.rss()` is a point sample; mimalloc hands a ~30 MB +// segment back to the OS on its own cadence between passes, so two point +// samples can straddle that release and read as a +31 MB "delta" with nothing +// retained (observed on the 4-vCPU Windows lane). `resourceUsage().maxRSS` is +// the kernel's monotone peak (VmHWM / PeakWorkingSetSize), so it never dips: +// flat when freed memory is reused, strictly rising (~50 MB over 3 passes) +// when it leaks. // // Skipped in debug: at this N a debug pass is ~40s and the extra debug-build // allocation tracking adds enough RSS noise to drown the signal. CI has no @@ -72,23 +72,23 @@ test.skipIf(isDebug)( } const N = 4000; + const samples = []; function pass() { for (let i = 0; i < N; i++) once(); Bun.gc(true); - return process.memoryUsage.rss(); + samples.push(process.memoryUsage.rss()); } - function peakOver(passes) { - let peak = 0; - for (let i = 0; i < passes; i++) peak = Math.max(peak, pass()); - return peak; - } + // ru_maxrss is KB on Linux/Windows, bytes on macOS; normalize to bytes. + const maxRSS = () => process.resourceUsage().maxRSS * (process.platform === "darwin" ? 1 : 1024); - const before = peakOver(3); - const after = peakOver(3); + for (let i = 0; i < 3; i++) pass(); + const before = maxRSS(); + for (let i = 0; i < 3; i++) pass(); + const after = maxRSS(); process.stdout.write( - JSON.stringify({ before, after, deltaMB: (after - before) / 1024 / 1024 }) + "\\n", + JSON.stringify({ samples, before, after, deltaMB: (after - before) / 1024 / 1024 }) + "\\n", ); `; @@ -117,6 +117,8 @@ test.skipIf(isDebug)( .trim(); expect(filteredStderr).toBe(""); + // Logged so a future red carries the per-pass trace without a repro. + console.log(stdout.trim()); const { deltaMB } = JSON.parse(stdout.trim()); // Unfixed: ~50 MB over 3 measured passes. Fixed: <1 MB.