Repository navigation
perf(agent-browser): validate small journal records natively during replay - #306
Conversation
|
Main now includes #307 as Before this PR's final integration, incorporate the updated main/development lock without force-pushing shared history, then complete the affected gates on the final tree. Do that at a safe boundary, not by editing files while the current checks run. Existing measurements can be reused only where their actual source, dependency and input identities still apply. This does not close #303 or turn this replay fast path into an O(new-data) implementation. The remaining restore work and its acceptance conditions remain necessary. AI-generated by github-backlog-coordinator. |
… order (#303) Records up to 1 MiB now parse natively only when a byte pre-scan shows nesting within 1024 levels and at most 64 Ki structural bytes, so a dense record no longer needs 48-64 MiB of temporary heap. Fallback input reaches the token projector in 64 KiB chunks again, so a syntax or nesting error before invalid UTF-8 is reported first, as on main. Refs #303
|
Incorporated main's #307 with a normal merge (no rebase or force push): head Reran the gates whose inputs changed, on this exact tree (Node 24.22.1, agent-browser 0.39.2):
Not rerun because the dependency change does not touch them: the journal-read benchmark, the equivalence/precedence/heap probes, the capped-heap regression's RED/GREEN, and the 128 MiB large-journal proof. The 80-case equivalence probe also gives identical results on Node 24.21.0 (the CI pin) and 24.22.1. Remote checks on this head: all complete and passing (Windows full qualification and native-qualification skipped by design); Pi Durable Review and Cursor report no findings. #303 stays open; this PR remains a partial improvement. |
|
Installed-delivery verification is now complete for the combined #307 + #306 source at a955047 (tree d0bf32a9). The canonical approved dependency/build refresh passed. The configured extension was replaced as a complete source/dist/dependency generation, retaining the original dirty lock and whole old generation. An independent check matched all 369 compiled files to the retained generation manifest. Nine real SDK/extension/session-restoration cases passed on the fork and official Pi across old, staged, cross-generation and installed consumers. Current refs dispatch; wrong-branch and superseded-snapshot refs are refused before click dispatch. These use a controlled upstream browser CLI: they do not claim real Chrome or visual coverage. All synthetic journals, including the failed fixture attempt, remain retained. The two unrelated linked worktrees affected by relocating their common Git directory were repaired using native Git. Independent checks confirm their administrative bytes, HEAD/index/refs and non-Git metadata remain unchanged. The old root is retained because it is their required Git common directory. Disposable swap/repair/rollback checks passed; no live rollback was needed. This completes installed delivery of the safe full-scanner/fallback changes. It does not establish O(new-data), authorize a metadata/prefix cache, or close #303. No npm publication or forced reload/restart of existing sessions occurred. |
Reads session journals faster when restoring browser state on startup, resume, switch and tree navigation. Refs #303 — this is a partial improvement, not the solution requested in #303: every restore still reads and validates the whole journal, so cost still grows with file size. #303 stays open.
What changed
{,[,,,:outside strings). That keeps a native parse to a few MiB, inside the existing 16 MiB per-record reserve.main.Review remediation (ae48f15, after a213a18)
main: 64 KiB–1 MiB records with a syntax or nesting error before invalid UTF-8 reported the UTF-8 errormain. Rebuilt 80-case / 160-comparison probe: 0 differences frommain(a213a18: the same 4). Owner test extended with both cases; it fails on a213a18 and passes onmainand this head.mainand this head.large-browser-journal.mjsnow adds three off-branch records at the 1 MiB native boundary. Under--max-old-space-size=128it passes on this head (max RSS 429,792 KiB) and also on a213a18 (451,200 KiB): the old 64 MiB spike still fit inside 128 MiB. The per-record regression is caught by the new capped-heap test, not by this proof.Refs #303/ not a full fixEvidence
Journal-read micro-benchmark only:
readBrowserEntriesplus the page-state reducer on compileddist, fresh process per sample, 5 rounds with rotating side order. It does not start Pi, the extension'ssession_starthandler or a browser, so it is not an end-to-end restore, resume or session-switch measurement. Synthetic, credential-free journals; every record is under 1 MiB (largest 547 KB), so the >1 MiB fallback (unchanged code path) is not exercised. Host: macOS arm64, 18 CPUs, Node 24.22.1, load average 14–16 during the run.mainmedian (range)The density pre-scan costs about as much as decode plus
JSON.parseon quote-heavy text, so this head is 20–40% slower than a213a18 but still about 2.6x and 4.6x faster thanmain. Cost is still proportional to file size.Other checks on this head's tree:
node --max-old-space-size=128 test/helpers/large-browser-journal.mjs(1.61 GB journal incl. the three dense records): pass, max RSS 429,792 KiB. Same helper on a213a18: also pass (451,200 KiB).npm run verify -- startup-profile --samples 3: entrypoint import + factory median 66.5 ms (budget 250 ms). This is import timing only, not journal restore.Tests
journal scanning keeps token-projection acceptance for retained and streamed records: now also covers a structurally dense retained record and first-error precedence (syntax before invalid UTF-8, nesting before invalid UTF-8) in 64 KiB+ records.a structurally dense record under 1 MiB scans within the per-record heap reserve.npm run qualityexit 0 (main suite 1,502 pass / 49 opt-in skips, 0 fail; probes 66 pass / 9 skips; live command-reference check pass).Toward #303 (not implemented here)
The O(new bytes) route has to survive how Pi actually writes journals. Official Pi 1.0.2 and 1.4.0 rewrite the session file in place (
openSync(path, "w"), same inode) on empty-file init, load-time version migration and branched-session creation; the maintained fork rewrites atomically (new inode). A(path, size, mtime)key alone can miss an in-place rewrite.A route that fits those writers: an in-process scan cache keyed by path, device, inode and header id, storing the end offset of the last complete record plus digests of the header record and that last record. Reuse only when identity matches, size ≥ end, and both digests still match; then scan only the suffix and keep ancestry validation and capture checks as today. Every Pi rewrite changes the inode (fork) or the header record (migration/init) or the path (branch), so all are detected. Limits that need an explicit decision before building it:
ctx.sessionManagerentries (the other direction in Session restore re-parses the entire journal on every session_start and session_tree #303) conflicts with the existing contract that replay never loads Pi's in-memory entries.