perf(jsonl): read each transcript once per render instead of five times - #568
Merged
sirmalloc merged 2 commits intoSep 2, 2026
Merged
Conversation
A render reads the session transcript from five call sites: token metrics, session duration, speed metrics, compaction stats and thinking effort. Neither reader held a cache, so each one re-read and re-split the whole file. On a 507.6 MB transcript that is 2,551 MB read from disk for one repaint, a 5.0x amplification. Memoize both readers per process, keyed on file identity plus size and modification time. Measured on that transcript over three interleaved rounds: 10,814 ms to 5,774 ms, peak RSS 3,106 MB to 1,795 MB, disk reads 2,551 MB to 521 MB, output byte-identical. Key on device and inode rather than the path string. Windows accepts many spellings of one path and this codebase mixes them: transcript_path arrives with backslashes while the glob in jsonl-blocks yields forward slashes for the same file, so a path-keyed cache stores the largest file in the working set twice. Take the stat as bigint because inodes here exceed Number.MAX_SAFE_INTEGER and a plain stat loses low bits. Let getAllTimestampsFromFile opt out. That sweep visits each transcript once across as many as the lookback selects, so caching cannot hit, and retaining those lines would hold arbitrarily many whole transcripts for the life of the process. Measured at zero cache hits over four runs against a corpus of 18,837 transcripts, and retaining them raised peak RSS from 140 MB to 410 MB on a ten-file probe. Return readonly string[], since five callers now share one array.
The case asserted that a backslash and a forward-slash spelling of one transcript share a cache entry. A backslash separates paths on Windows but is an ordinary filename character elsewhere, so on the Linux runner that spelling named a file which does not exist and the read threw. Build the spellings by hand for the platform in hand. Every platform gets the dot-segment and doubled-separator forms, which path.join would have normalized back into the string they must differ from, and Windows adds the slash-direction form. Verified against a path-keyed identity with the Windows-only spelling suppressed, so the case still fails on the Linux subset alone.
This was referenced Aug 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A render reads the session transcript five times. Neither reader caches, so each caller re-reads and
re-splits the whole file:
getTokenMetrics,getSessionDuration,getSpeedMetricsCollection(
jsonl-metrics.ts:94,162,548),getCompactionStats(compaction.ts:94) andgetTranscriptThinkingEffort(jsonl-metadata.ts:47).507.6 MB transcript, three interleaved rounds, disk reads from
GetProcessIoCounters:Output byte-identical. Amplification 5.0x → 1.03x.
Keyed on
dev+ino, not the path string.transcript_patharrives with backslashes while theglob at
jsonl-blocks.ts:43yields forward slashes for the same file, so a path key stores it twice.Stat is bigint because inodes here exceed
Number.MAX_SAFE_INTEGER. Size and mtime form the version,so an appended transcript is re-read.
The block sweep opts out (
{ cache: false }). It visits each transcript once, so caching cannothit; measured 0 hits over four runs, and retaining its reads took peak RSS from 140 MB to 410 MB on a
ten-file probe.
Readers return
readonly string[], since five callers share one array.#551 conflicts textually — both rewrite these two functions. Intents compose: #551 streams (fixing
#550's ceiling and peak), this removes the repeat walks.
18 tests. Verified by mutation, each mutation asserted applied: cap to 1, cap to 1e6, version dropping
size, version dropping mtime, eviction removed, identity by path, no move-to-end,
cache: falseignored on each reader, version ignored on read. All ten fail the suite.