Skip to content

perf(agent-browser): validate small journal records natively during replay - #306

Merged
fitchmultz merged 3 commits into
mainfrom
perf/issue-303-session-restore
Oct 25, 2026
Merged

fitchmultz merged 3 commits into
mainfrom
perf/issue-303-session-restore

Conversation

@fitchmultz

@fitchmultz fitchmultz commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

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

  • Journal records are framed with native byte search.
  • A record up to 1 MiB is parsed with native JSON (strict UTF-8, object root) only when a quick byte scan shows nesting within 1024 levels and at most 64 Ki structural bytes ({, [, ,, : outside strings). That keeps a native parse to a few MiB, inside the existing 16 MiB per-record reserve.
  • Larger or denser records, and every record native JSON rejects, go through the existing bounded token projector, fed in the same 64 KiB chunks as file reads. Accepted records, the first reported error, ancestry checks and the incomplete-tail rule match main.
  • Replay still never loads Pi's in-memory entries.

Review remediation (ae48f15, after a213a18)

Finding Disposition
4 of 124 malformed-input comparisons differed from main: 64 KiB–1 MiB records with a syntax or nesting error before invalid UTF-8 reported the UTF-8 error Fixed. Fallback now feeds the token projector 64 KiB chunks, like main. Rebuilt 80-case / 160-comparison probe: 0 differences from main (a213a18: the same 4). Owner test extended with both cases; it fails on a213a18 and passes on main and this head.
A ~1 MiB record of dense empty containers needed 48–64 MiB temporary heap (main ≤ 16) Fixed with a bounded cutoff. Smallest passing V8 old-space cap, one record per fresh process (main / a213a18 / this head, MiB): empty objects 12/64/12, empty arrays 12/64/12, densest accepted distinct keys 12/16/16, densest accepted objects 12/16/12, plain string 12/12/12. New owner test scans a ~1 MiB dense record under a 24 MiB cap: heap-out-of-memory on a213a18, passes on main and this head.
Test the 128 MiB contract on adversarial native-path input Done. large-browser-journal.mjs now adds three off-branch records at the 1 MiB native boundary. Under --max-old-space-size=128 it 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.
Measurements overstated ("restore" speedup; import time as startup proof) Fixed. Numbers below are a journal-read micro-benchmark only. The 69–105 / 71–80 ms figures in the earlier description were extension import time, not restore, and are removed. The benchmark journals contain only records under 1 MiB, so it shows nothing about the >1 MiB fallback (unchanged code path).
Refs #303 / not a full fix Kept. See the incremental-route outline below.

Evidence

Journal-read micro-benchmark only: readBrowserEntries plus the page-state reducer on compiled dist, fresh process per sample, 5 rounds with rotating side order. It does not start Pi, the extension's session_start handler 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.

Journal main median (range) a213a18 median this head median (range) Output
40 MB, 5,181 records 664 ms (580–932) 209 ms 255 ms (226–277) identical across all 15 samples
285 MB, 2,081 records 2,732 ms (2,546–3,190) 429 ms 591 ms (545–696) identical across all 15 samples

The density pre-scan costs about as much as decode plus JSON.parse on quote-heavy text, so this head is 20–40% slower than a213a18 but still about 2.6x and 4.6x faster than main. 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.
  • New a structurally dense record under 1 MiB scans within the per-record heap reserve.
  • Storage-recovery + storage-growth 51/51. npm run quality exit 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:

  • A third-party in-place edit of a middle record that keeps the header and last record byte-identical would be missed; today's full rescan catches it. The storage doc already puts behind-host mutation of old ancestors outside the append-only admission contract, but extending that to record validation narrows what a restore detects.
  • Cold start (new process) still needs a full scan; only same-process switches and tree navigation benefit. A persisted sidecar index would add new on-disk state and its own invalidation story.
  • Reusing ctx.sessionManager entries (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.

@fitchmultz

Copy link
Copy Markdown
Owner Author

Main now includes #307 as 0e654eb77ebd3a73dbcd216ff108431c60756e37. It changes the npm-root development graph from Undici 8.10.2 to exact 8.12.3; the extension runtime dependencies and Pi support floor are unchanged. Official Pi hosts retain their own dependency graph.

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
@fitchmultz

Copy link
Copy Markdown
Owner Author

Incorporated main's #307 with a normal merge (no rebase or force push): head 618df44e7107c9529099cdaa37ec563610d9898b, tree d0bf32a9016b7f1d1f3cda7a872593a797527373. The PR's own diff against main is unchanged from ae48f15.

Reran the gates whose inputs changed, on this exact tree (Node 24.22.1, agent-browser 0.39.2):

  • npx npm@12.2.0 ci --ignore-scripts: pass, 0 vulnerabilities, development Undici 8.12.3.
  • npm run quality: pass (1,502 tests pass, 49 opt-in skips, 0 fail; probes 66 pass, 9 skips; live command-reference check pass).
  • npm run verify -- startup-profile --samples 3: median 66.9 ms against the 250 ms budget (extension import only, not journal restore).

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.

@fitchmultz
fitchmultz merged commit a955047 into main Oct 25, 2026
13 checks passed
@fitchmultz
fitchmultz deleted the perf/issue-303-session-restore branch October 25, 2026 20:43
@fitchmultz

Copy link
Copy Markdown
Owner Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant