Skip to content

fix: bound session-shard memory for ranged queries - #1506

Open
iliaal wants to merge 3 commits into
getagentseal:mainfrom
iliaal:fix/oom-bounded-shard-load
Open

iliaal wants to merge 3 commits into
getagentseal:mainfrom
iliaal:fix/oom-bounded-shard-load

Conversation

@iliaal

@iliaal iliaal commented Sep 19, 2026

Copy link
Copy Markdown

Fixes #1504 (session-shard half; the codex-cache rewrite and digest wiring follow as stacked PRs).

What was wrong

Ranged loads assembled whole multi-hundred-MB month shards before filtering, and the pre-lock snapshot overlapped the canonical reload (~787MB duplicate live at once). Retained strings were tokenizer slices pinning whole input chunks.

What changed (this PR)

  • Session month shards decode incrementally, one turn at a time, keeping only in-range turns (same kept/dropped/carry contract, accumulator-based, no per-turn wrappers).
  • Shard loads run serially; pre-lock snapshot released before reload.
  • Out-of-range turns contribute raw dedup keys to cross-file suppression markers (exact sets, same walk order).
  • Retained strings detached from tokenizer buffers: the walker assembles slices and only what the accumulator keeps is flattened (moved down from the aggregate PR so no step in the stack regresses memory on its own).
  • Size gate (SHARD_STREAM_GATE_BYTES = 256MB, shared in shard-stream.ts): shards at or under the gate decode with plain JSON.parse through the same per-record projection; larger ones stream exactly as before. Byte-identical results on both paths are pinned by the size-gate parity suite (kept, sliced, PR-whole, key-overlap, all-dropped, invalid).
  • Provider-scoped queries skip unrelated sections.

Out of scope here by design: the codex result-cache rewrite (stacked PR #1508, follows the same gate), digest wiring of the marker keys (stacked follow-up), aggregate parse mode (stacked PR #1503).

Verification

  • tsc clean; full suite green; locks green.
  • Range-filter suite (incl. 30k-turn record test), shard-stream tests, marker carry/overlap/ordering tests, size-gate parity suite.

Timings (warm cache, medians of 3, 1.2GB shard corpus with one 697MB shard)

command main before (stream-all) after (this PR)
overview -p today 13.8s / 3.1GB 73.2s / 2.8GB 17.6s / 2.7GB
report --period today --format json 13.2s / 2.5GB 76.0s / 2.9GB 17.3s / 2.6GB

The gate restores speed to ~1.3x main (from ~5.5x). The residual is the one over-gate shard still streaming plus per-turn filtering; memory on a corpus this size is dominated by the retained result set itself (main peaks 3GB here). On smaller corpora the detach commit is where the memory win lands (see the stack top).

@iamtoruk

Copy link
Copy Markdown
Member

Thanks for this. The range-filter design holds up: I could not find a path where a filtered load is persisted or seeds a sealed day, and the guards read well. It cannot merge yet though.

Blocker: this PR does not build on its own. On this head, tsc --noEmit gives src/parser.ts(5531,46): error TS2304: Cannot find name 'cacheLoadOpts'. The helper is defined in #1503's commit, not here. At runtime every ranged command exits 1 with ReferenceError: cacheLoadOpts is not defined, and tests/cache-directory-switch.test.ts fails on it. Please move cacheLoadOpts into this commit and run tsc and the suite against this head alone, not the stack.

Then, before we look again:

  1. Rebase on main. You are 16 commits behind. Conflicts: CHANGELOG.md, src/parser.ts (2 hunks, needs a decision on WSL orphans and dropped-key seeding), src/session-cache.ts (1 import), src/codex-cache.ts (5 hunks; main's evictCachedCodexResults has to be rewritten against your overlay model). Please wait for 0.9.25 to land first: it changes the same runParseInner catch blocks and the getEntry fingerprint check that you replace.
  2. Please split this. The session-shard streaming and turn filter is the fix for the issue. The codex-cache.ts rewrite (738 lines) and the dedup-key digesting ride along. The first could land soon; bundled, none of it can.
  3. Drop the digest change or send it separately. Replacing exact dedup keys with a hand-rolled mix saves about 140MB, and a collision silently undercounts a call's cost. The "exact up to 2^-128" comment is not true of four cyrb32 lanes over the same characters. I found 0 collisions across 141,541 keys here, but that is not a guarantee. Also DedupSet extends Set without overriding iteration, so iterating yields digests, and two call sites already need instanceof DedupSet. Make it a plain class so an unadapted consumer is a compile error.
  4. rpcConversationBareKey puts the antigravity key shape inside the generic dedup primitive in session-cache.ts. That belongs in the provider.
  5. Please measure and state the time cost. Warm cache here: report --period week --format json 2s to 25s, --period all 3s to 32s, overview -p today 2s to 14s. Memory roughly halves, wall clock goes up about 10x. That is the path the desktop app and menu bar poll, so it is a real trade we need to decide on with numbers. flattenJsonStrings re-keys every decoded object with delete then reinsert, which pushes them into dictionary mode; worth checking whether the time goes there.
  6. stripFilteredForWrite is Object.fromEntries(Object.entries(files).filter(([, f]) => !f.rangeFiltered)) behind the groupHasFiltered guard. path is bound and unused.
  7. The diff is indented one space right of the repo style: 646 lines are the same line re-added with only whitespace changed. Please reformat; it inflates the diff and creates conflicts.
  8. src/parser.ts:5663 now ends mid-sentence ("Never a correctness gate: on any"). The next comment line was deleted by accident.
  9. providers/antigravity.ts:1252 refers to trackAntigravityKey, which does not exist. The pairing happens in DedupSet.add.

@iliaal
iliaal force-pushed the fix/oom-bounded-shard-load branch from 01d856c to 24ff6c0 Compare September 20, 2026 00:18
Session month shards decode incrementally (one turn at a time,
keeping only in-range turns with the exact kept/dropped/carry
contract); shard loads run serially with the pre-lock snapshot
released before the canonical reload. Out-of-range turns
contribute raw dedup keys to cross-file suppression markers;
retained strings detach from tokenizer buffers. The codex result
cache rewrite, digest wiring, and aggregate parse mode follow as
stacked PRs.
@iliaal
iliaal force-pushed the fix/oom-bounded-shard-load branch from 24ff6c0 to 9986edb Compare September 20, 2026 01:20
@iliaal iliaal changed the title fix: bound cache-load memory for ranged queries fix: bound session-shard memory for ranged queries Sep 20, 2026
@iamtoruk

Copy link
Copy Markdown
Member

Thank you for splitting this. The split worked: this head compiles on its own, tsc is clean, and the 111 touched tests pass against this commit alone. The numbers are exact too. I diffed the full JSON export leaf by leaf against main, cold and warm, for today, week, 30days, month and all. The only difference is the known shellCommands "ls" count from this branch predating main's extractBashCommands line, and it disappears with that line applied.

But as it stands this cannot merge. It is about 12 times slower than main and, on its own, uses more memory too. That is not acceptable for us: this is the path the desktop app and the menu bar poll all day, and we have just spent a release cutting their CPU use.

Warm cache, medians of 3, this head vs main:

command main this PR
report --period today --format json 1.83s / 1007MB 21.19s / 1417MB
report --period week --format json 2.03s / 1008MB 20.95s / 1635MB
report --period all --format json 3.97s / 1671MB 27.09s / 1915MB
overview -p today 1.13s / 1006MB 10.94s / 929MB

So on report today it is 11.6x the wall time and 41% more memory. The memory win only appears at the top of the stack, where #1503 adds the detach-only-retained-strings commit (1b30177). The stack ships the regression first and part of the cure last.

I profiled it (node --cpu-prof, 23.8s sampled): 22.6% stream-chain, 18.2% stream-json, 14.3% the token walk in shard-stream.ts over parser.asStream(), most of the rest is buffer and ucs2 conversion feeding them plus about 1.1s of GC. flattenJsonStrings is 196ms, 0.8%. I pointed you at that last time and I was wrong, sorry.

The comparison that settles it: the whole 166MB of session shards parses in 333ms with plain JSON.parse. The same bytes through the streaming walk cost 15 to 20 seconds.

What I am asking for: gate on file size. JSON.parse below a threshold, stream only above it. The streaming exists for the one shard that passed V8's max string length; a size gate keeps main's speed for everyone and still bounds that case. One if at each entry point. Please also move 1b30177 down into this PR so no single step in the stack is worse than main, and put the before and after timings in the body.

Rebase after 0.9.25 lands please. Against main you conflict in CHANGELOG.md, src/parser.ts, src/providers/types.ts and src/session-cache.ts; the parser and session-cache ones are semantic. The indentation is fixed, thank you.

Per-string flattening at assembly copied every string of every turn
(kept or dropped): ~829k turns x dozens of strings of Buffer + string
churn that GC could not retire under a heap cap, dying in the sept
shard at ~500MB live. streamShardArrayField takes detachStrings
(default true); the filtered loader passes false and flattens only
what it keeps (kept turns + meta at fold time, carry refs + first
project in finishSlice).

(cherry picked from commit 1b30177)
The streaming walk costs 15-20s where JSON.parse needs milliseconds
(525MB of shards in ~11s on this corpus); it exists for the one
shard past V8's max string length. Shards at or under 256MB now take
plain JSON.parse through the same per-record projection
(filterShardFile / validateCachedFile); larger ones stream exactly
as before. A single readFile is already an atomic snapshot, so the
small path needs no fingerprint guard. Byte-identical results on
both paths are pinned by the size-gate parity suite (kept, sliced,
PR-whole, key-overlap, all-dropped, invalid).
@iliaal

iliaal commented Sep 20, 2026

Copy link
Copy Markdown
Author

Gate is in: SHARD_STREAM_GATE_BYTES = 256MB shared in shard-stream.ts, one if at each entry point (loadShardStreaming, loadShardFiltered; codex twins in #1508). Sub-gate shards take plain JSON.parse through the same per-record projection; parity suites pin byte-identical results on both paths. 1b301772 moved down here (cherry-picked). Before/after timings are in the body — 73→17s on a 1.2GB corpus, residual ~1.3x is the one over-gate shard still streaming. tsc + full suite green on this head. Rebase waits on 0.9.25 (not landed yet — will do when it does).

This branch has not been deployed

No deployments
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.

overview -p today crashes with heap OOM on large corpora (cache-load layer)

2 participants