mirror of https://github.com/garrytan/gstack.git
stop materializing response bodies in requestfinished listener
The Bun-side accelerant on the gbrowser-OOM investigation. Pre-fix, the per-page requestfinished listener called \`await res.body()\` just to read .length — Playwright fetches the bytes from Chromium across CDP into a Bun Buffer, only for the listener to discard the buffer after a single length read. On a long-lived headed browser with media-heavy pages this is multi-GB/hour of Buffer allocation churn. Bun GCs it, but the cross-process CDP traffic + transient allocation pressure feeds the OOM trajectory. The fix: req.sizes() pulls from the Network.loadingFinished event Chromium already emits. No body materialization. Accurate for chunked transfer, gzip-compressed responses, and streaming media — the cases where a naive Content-Length header read (the original review's proposal) would have missed the size entirely (Codex flag on the eng review, D10 USE_CDP_EVENT_BATCHED). The D10 stretch goal — replacing N per-page listeners with a single context-level CDP listener via Target.setAutoAttach — is deferred and tracked in TODOS. The listener architecture change is significantly more plumbing than the leak fix and not on the critical path for stopping the body materialization. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
98b2ae8103
commit
78afd75e72
|
|
@ -1710,23 +1710,38 @@ export class BrowserManager {
|
|||
}
|
||||
});
|
||||
|
||||
// Capture response sizes via response finished
|
||||
// Capture response sizes via requestfinished — but DO NOT call
|
||||
// response.body() here. Pre-fix, this listener materialized every
|
||||
// response body across CDP just to read .length: multi-GB/hour of
|
||||
// Buffer churn on long-lived headed Chromium with media-heavy
|
||||
// pages, the primary Bun-side accelerant on the gbrowser-OOM
|
||||
// investigation. req.sizes() pulls from the Network.loadingFinished
|
||||
// event Chromium already emits — accurate for chunked transfer,
|
||||
// gzip-compressed responses, and streaming media, all the cases
|
||||
// where the previous Content-Length-header approach would have
|
||||
// missed the size.
|
||||
//
|
||||
// The "single context-level CDP listener" architecture (D10's
|
||||
// stretch goal — would reduce per-page listener count from N to 1
|
||||
// via Target.setAutoAttach) is deferred. TODOS.md tracks it.
|
||||
page.on('requestfinished', async (req) => {
|
||||
try {
|
||||
const res = await req.response();
|
||||
if (res) {
|
||||
const url = req.url();
|
||||
const body = await res.body().catch(() => null);
|
||||
const size = body ? body.length : 0;
|
||||
for (let i = networkBuffer.length - 1; i >= 0; i--) {
|
||||
const entry = networkBuffer.get(i);
|
||||
if (entry && entry.url === url && !entry.size) {
|
||||
networkBuffer.set(i, { ...entry, size });
|
||||
break;
|
||||
}
|
||||
const sizes = await req.sizes().catch(() => null);
|
||||
if (!sizes) return;
|
||||
const url = req.url();
|
||||
const size = sizes.responseBodySize ?? 0;
|
||||
for (let i = networkBuffer.length - 1; i >= 0; i--) {
|
||||
const entry = networkBuffer.get(i);
|
||||
if (entry && entry.url === url && !entry.size) {
|
||||
networkBuffer.set(i, { ...entry, size });
|
||||
break;
|
||||
}
|
||||
}
|
||||
} catch {}
|
||||
} catch {
|
||||
// Best-effort: requestfinished fires for aborted/cached requests too,
|
||||
// where sizes() is unavailable. Missing size is acceptable; an
|
||||
// unbounded throw would noise the console for every cache hit.
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue