Skip to content

perf: overlap workspace preparation with model startup - #701

Open
knqiufan wants to merge 3 commits into
OpenBMB:mainfrom
knqiufan:codex/perf-687-incremental-checkpoints
Open

knqiufan wants to merge 3 commits into
OpenBMB:mainfrom
knqiufan:codex/perf-687-incremental-checkpoints

Conversation

@knqiufan

@knqiufan knqiufan commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Re-scoped onto current main (journal-v2 checkpoints).

Upstream now records turn changes through command frames and increments instead of scanning the whole workspace in beginTurn, so the incremental checkpoint cache this PR originally carried no longer applies. This PR now overlaps the remaining pre-request work — the file-artifact baseline — with model startup, and gates every write entry point behind that readiness.

Refs #687 (the turn-start checkpoint scan half is provided upstream).

Changes

  • Start one shared preparation task after durable input acceptance: beginTurn still persists the turn's checkpoint record, and FileArtifactCollector.start prepares the artifact baseline.
  • With no matching pre-request hooks, the model request proceeds immediately. Tools, custom validation, matching hooks, subagents, background tasks and plugins await readiness.
  • Matching UserPromptSubmit / InstructionsLoaded / PreModelRequest / compaction hooks wait for protection; unmatched hooks never dispatch early.
  • Preparation failure aborts an in-flight provider, blocks tools, records the failure and releases leases. Terminal success, artifact output and checkpoint finalization wait for preparation.
  • Closing the stream or cancelling aborts preparation, closes the model iterator and joins cleanup; the artifact scan supports cooperative cancellation.
  • Restore/undo formats, leases and conflict checks are untouched.

Validation

Node 22.23.2, Windows 11:

Check Result
npm run build (tsc + plugins) passed
node --test dist/tests/session/workspace-preparation.spec.js 29/29 passed, repeated runs
npm run test:checkpoints remaining failures are the pre-existing Windows rename/timing flakes and reproduce on unchanged main

Not included

  • The incremental checkpoint cache, its tests and its docs from the earlier revision, which targeted the removed turn-start scan.
  • The old macOS benchmark numbers. They must be re-measured against the journal-v2 baseline; scripts/benchmark-turn-startup.ts is kept for that.

@knqiufan
knqiufan force-pushed the codex/perf-687-incremental-checkpoints branch from 9520b62 to 6a27a3f Compare November 3, 2026 13:00
@knqiufan knqiufan changed the title perf: reuse checkpoints and overlap workspace preparation with model startup perf: overlap workspace preparation with model startup Nov 3, 2026
@knqiufan
knqiufan force-pushed the codex/perf-687-incremental-checkpoints branch from 6a27a3f to eddb094 Compare November 3, 2026 13:15
Prepare the file-artifact baseline after durable input acceptance so the model
can start immediately, and gate tools, matching lifecycle hooks, subagents and
background writes behind that readiness.

Re-scoped after upstream moved checkpoints to journal v2: the full-workspace
checkpoint scan at turn start no longer exists, so this change no longer carries
an incremental checkpoint cache. It keeps the single preparation task, the
barrier for every write entry point, cancellation/lease cleanup, failure
provenance and the artifact baseline's cooperative cancellation.

Refs OpenBMB#687.
@knqiufan
knqiufan force-pushed the codex/perf-687-incremental-checkpoints branch from eddb094 to 1e2a3b9 Compare November 10, 2026 07:08
@xhd0728

xhd0728 commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

The re-scoped implementation looks sound and is worth landing as a startup-latency improvement. Overlapping the artifact baseline with model startup has a concrete benefit, while the shared readiness barrier keeps tools, matching hooks, subagents, background tasks, and terminal finalization behind workspace preparation.

The 29 preparation tests and 84 related tests passed locally, and Web and Desktop CI are green. In a synthetic 2,000-file workspace with a fixed 50 ms local model delay, the median first-text latency across the four warm runs fell from about 80 ms to 52 ms. This is a scoped benchmark, not a general end-to-end speedup claim.

Before merging, please update docs/parallel-workspace-preparation.md to follow CONTRIBUTING.md: maintained developer documentation must be in English, and implementation history or one-run validation narratives belong in the PR or CI artifacts. Keep the durable readiness/cancellation contract and benchmark usage in the canonical documentation.

I did not find a blocking implementation issue. Once that documentation cleanup is done, I recommend merging; its priority is lower than the correctness fixes in #702 and #703.

The guide states the readiness and cancellation rules and how to run the
startup benchmark. Implementation history stays in the pull request.
@knqiufan

knqiufan commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor Author

@xhd0728 Thanks for the review. docs/parallel-workspace-preparation.md is now the English contract: it keeps the readiness and cancellation rules and the benchmark usage, and the implementation history stays in this PR. I also linked it from docs/README.md.

Ubuntu DEB x64 aborted while Vite rendered chunks at the 2560 MB Node heap cap.

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.

2 participants