Skip to content

Avoid rereading rollout history during cold resume - #27031

Merged
zanie-oai merged 2 commits into
mainfrom
zb/fix-double-resume-parse
Jun 9, 2026
Merged

Avoid rereading rollout history during cold resume#27031
zanie-oai merged 2 commits into
mainfrom
zb/fix-double-resume-parse

Conversation

@zanie-oai

@zanie-oai zanie-oai commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • reuse the history-bearing StoredThread loaded while probing for a running thread
  • avoid rereading and reparsing the rollout when that probe finds no active process
  • reload after shutting down a loaded thread because shutdown may flush newer rollout items
  • add a regression test that verifies cold resume performs one history-bearing store read

Problem

thread/resume first reads the persisted thread with history while checking whether the thread is
already running. When no running process exists, cold resume currently falls through to
resume_thread_from_rollout, which reads and parses the same history again.

That duplicate work grows with rollout size and remains on the synchronous resume path even when
the caller requests excludeTurns.

Background

The duplicate read was introduced by #24528, which fixed resume overrides for idle cached
threads. To support resumes specified by rollout path, resume_running_thread began loading the
stored thread with history so it could resolve the canonical thread ID and determine whether a
cached CodexThread was already loaded.

That history is needed when the loaded-thread path handles the request. On a cold miss, however,
the function's boolean result could only report that no loaded thread handled the request. It
discarded the history-bearing StoredThread, and the normal cold-resume path immediately loaded
and parsed the same rollout again.

This change preserves the idle cached-thread behavior from #24528 while allowing the cold-resume
path to reuse the probe result.

Performance

I benchmarked real retained rollouts using isolated CODEX_HOME directories, explicit rollout
paths, debug builds of the commit and its exact parent, and alternating parent/patch order. The
table below uses thread/resume with excludeTurns: true; response payload sizes were identical.

Rollout size Records Parent median Patch median Median paired saving
6 MB 3,574 541 ms 441 ms 132 ms
30 MB 15,220 1.505 s 1.041 s 701 ms
60 MB 31,453 2.644 s 1.742 s 970 ms
149 MB 100,874 10.506 s 7.156 s 3.350 s
559 MB 259,734 27.759 s 16.725 s 9.836 s

The absolute saving increases with thread size, as expected when removing one complete JSONL
history read and parse. Total resume time is also content-dependent, so the relationship is not
perfectly linear.

I also tested full-history resume with excludeTurns: false. The response payload was
byte-identical between variants, and the same size-dependent improvement remained visible:

Rollout size Parent median Patch median Median paired saving
6 MB 1.052 s 904 ms 270 ms
30 MB 2.667 s 1.762 s 924 ms
60 MB 8.464 s 6.272 s 3.680 s
149 MB 26.719 s 12.118 s 14.601 s
559 MB 40.359 s 25.475 s 16.590 s

Validation

  • just test -p codex-app-server cold_thread_resume_reuses_non_local_history_probe
  • just fix -p codex-app-server -p codex-thread-store
  • just fmt

Comment thread codex-rs/app-server/src/request_processors/thread_processor.rs Outdated
@zanie-oai
zanie-oai marked this pull request as ready for review June 8, 2026 20:12
@zanie-oai
zanie-oai merged commit 6a9a49b into main Jun 9, 2026
31 checks passed
@zanie-oai
zanie-oai deleted the zb/fix-double-resume-parse branch June 9, 2026 16:16
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants