perf(vscode): pre-spawned rs fmt standby for the active editor - #6
Merged
Conversation
A cold format pays ~130ms of process start-up and config load before any formatting happens. The fmt stack now keeps one pre-spawned rs fmt child parked on stdin for the active editor's file, so the next format of that file writes to a process that has already paid both. The standby is bounded on purpose: one process, one file, one request. It is armed 2s after the active editor settles (immediately at registration and after a format), killed whenever it could have gone stale — a config content edit, a detection change, restart or dispose — and reaped after five idle minutes. Anything it cannot serve falls back to the cold path unchanged, and the whole mechanism is invisible above debug level except for the hot/cold marker on the completion line. CONTEXT.md records the vocabulary the code and logs use, and the fmt gotcha in AGENTS.md now states the exception and its limits.
The isolated playground profile installs no extensions, so --disable-extensions only contributed its own notification on every launch. Git is a built-in and ignores that flag entirely: it offered to open the parent repository each time, because every fixture lives inside this repo. Disable that one extension instead.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53424eff2e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An active editor the provider matches but the stack cannot resolve — an undetected folder, a missing or too-old rstack package — logged the skip and returned, leaving the previous file's standby parked for up to five idle minutes. The editor had still moved on, so that standby no longer tracked it and could serve a programmatic request for a file the user had left. Arming already kills a standby it replaces; this path has nothing to arm, so it has to kill on its own. An editor holding nothing formattable at all still leaves the standby alone: a peek at the output panel must not cost the user their warm process.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b0efb09eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The docstring justified keeping a standby across an ineligible editor with a peek at the output panel, which cannot happen: the active editor is the focused one or, when nothing is focused, the most recently changed one, so focusing a panel leaves it pointing at the same file and fires no change. Name the cases that do reach it — no text document at all, or one this stack does not format — and say why neither is worth a kill. CONTEXT.md stated the invariant more strongly than the code holds it; it now states both outcomes.
…p to expire The docstring justified keeping a standby across an ineligible editor with a peek at the output panel, which cannot happen: the active editor is the focused one or, when nothing is focused, the most recently changed one, so focusing a panel leaves it pointing at the same file and fires no change. Name the cases that do reach it — no text document at all, or one this stack does not format — and say why neither is worth a kill. CONTEXT.md stated the invariant more strongly than the code holds it; it now states both outcomes. 'Reap' reads as jargon for what the idle timer does, so the term is now 'expire' in the glossary, the log reason and the tests.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54789e4133
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The config watcher listened for content changes only, on the reasoning that a create or a delete moves the detection signature and arrives as a detection change instead. That holds for a real create or delete, not for the delete/create pair an editor emits when it saves by atomically replacing the file: the config path is gone and back within one scan, the signature records paths rather than content, and no detection change fires. Both halves were also ignored here, so nothing invalidated the standby and the parked process kept serving the config it had loaded at spawn — the same file then formatted one way hot and another way cold. Watch all three events. A redundant kill costs nothing: it is idempotent and the active editor re-arms through the usual debounce.
2 tasks
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
1 participant
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.
What
Follow-up to #5, which left the ~130 ms cold-start latency on the table. Every format request paid it: process start-up (~70-140 ms), CLI bootstrap (~25 ms) and config load (~20-30 ms) all happened after the keystroke.
The fmt stack now keeps one pre-spawned
rs fmtchild parked on stdin for the active editor's file, so the next format of that file writes to a process that has already paid all three.rs fmt --stdin-filepathfixes the target path in argv and starts its config load concurrently with draining stdin, which is exactly why the standby is scoped this narrowly — one process, one file, one request.FileSystemWatcher, since detection only tracks the config file list), a detection change, restart, dispose, or the parked process exiting on its own.rs fmtspeaks LSP; a setting shipped now would be a setting to deprecate later.hot/coldmarker (Formatting completed in 12ms (hot)); every arm/consume/kill decision logs its reason at debug.run.tssplits intospawnRsFmt(start a child, park it) andserveRsFmt(write and collect), with the existingrunRsFmtas their composition, so the cold and hot paths share one implementation of the protocol.CONTEXT.mdrecords the vocabulary (standby / arm / consume / hot / expire) that the code, logs and this PR use, and the fmt gotcha inpackages/vscode/AGENTS.mdnow states the exception and its limits: not a daemon, no pool, no cross-request state, retires with the upstream LSP.Also quiets the F5 playground: the isolated profile installs no extensions, so
--disable-extensionsonly added its own notification, while the built-in Git extension (which that flag never affected) offered to open the parent repository on every launch.Test plan
pnpm lint— 0 errors, 0 type errors.pnpm test:unit— all passing, including a newstandby.test.tsthat drives arm/consume/kill/reap against real spawned stub processes (sharedstubProcess.tsscaffolding, not a build entry).pnpm test:e2e:vscode— exit 0. The fmt suite gains a hot-path test: show the document, poll until the standby reports it as armed, format, then assertlastServe() === 'hot'— the cold path produces identical text, so that assertion is the only proof the request actually consumed the standby.Checklist