perf(codex): stream rollout metadata instead of parsing every line - #156
Open
od0 wants to merge 1 commit into
Open
perf(codex): stream rollout metadata instead of parsing every line#156od0 wants to merge 1 commit into
od0 wants to merge 1 commit into
Conversation
Session listing (p list codex, the share picker, bare resume) called read_metadata on every rollout, which serde-parsed every line of every file — a minute-plus silent stall on a multi-gigabyte sessions tree. read_metadata is now one streaming pass: JSON-parse a bounded head (session_meta, first timestamps, first user prompt — all at the top of this append-only log) plus the final line (newest timestamp), and otherwise just count non-empty lines. 3.9 GB / 421 sessions: ~80 s -> raw-I/O speed. Documented trades: a prompt buried past the head budget reports None; line_count counts non-empty lines, not parsed ones. toolpath-codex 0.6.2. Also picks up the pre-existing rustfmt drift in paths.rs that fails cargo fmt --check on main under the pinned 1.94.0 toolchain.
|
🔍 Preview deployed: https://009fd271.toolpath.pages.dev |
This was referenced Aug 4, 2026
Author
akesling
reviewed
Aug 4, 2026
| let mut first_user: Option<String> = None; | ||
| let mut first_user_fallback: Option<String> = None; | ||
|
|
||
| loop { |
Contributor
There was a problem hiding this comment.
This should be pulled out to a helper for readability.
| let mut first_user_fallback: Option<String> = None; | ||
|
|
||
| loop { | ||
| raw.clear(); |
Contributor
There was a problem hiding this comment.
This code looks rather fragile.
What performance do we get if we use serde over a partial struct with raw values / accepting "undefined" fields? If serde won't give us what we want out of the box, we may want to roll our own partial Visitor or a proper parser that skips things intentionally.
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
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.
Found while dogfooding bare
path resume(#154): the pre-picker gather stalled for 80 seconds with no output./usr/bin/timepointed at codex — every session-listing surface (p list codex, thesharepicker, bare resume) callsread_metadataper rollout file, which full-parsed every JSONL line through serde. On a real tree (421 rollouts, 3.9 GB, 89 files >10 MB) that's ~80 s of pure parse CPU. The old code even predicted this: "If that becomes a bottleneck we'd peek the first line plusstatfor mtime."The fix
read_metadatais now a single streaming pass that exploits the rollout being an append-only log:Measured on the tree above:
p list codex1m20s → 8.5 s in a debug build (release is faster); user-time drops from 72 s to ~6 s, i.e. it's raw I/O now.Deliberate trades (documented on the method)
first_user_messageisNoneif the first prompt appears after the head budget (real sessions surface it within a dozen lines).line_countcounts non-empty lines (unparseable ones included) instead of successfully parsed ones — it only ever approximatedmessage_count.started_at/last_activitycome from head-min/tail-max rather than a global min/max scan — identical for append-only logs.Three new tests pin the tail-timestamp path, tolerant counting over blank/junk lines, and the head-budget bound. Also picks up the pre-existing
paths.rsrustfmt drift that failscargo fmt --checkon main under the pinned 1.94.0 toolchain.toolpath-codex 0.6.1 → 0.6.2 (perf fix, patch bump) in the four conventional places.
Follow-up worth filing separately: the listing surfaces re-scan all files on every invocation; piggybacking session metadata on the sync manifest's stat stamps would make an unchanged tree instant.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.