Repository navigation
perf(server): a checkpoint capture starts 8 git processes instead of 10 - #21315
josephv123 wants to merge 1 commit into
Conversation
Every capture looked up the common git dir, whether HEAD resolves and the absolute index path with three separate `git rev-parse` processes. One `rev-parse --git-common-dir --path-format=absolute --git-path index --verify --quiet HEAD` prints all three, so a capture now runs 8 git processes (6 with an unborn HEAD) instead of 10 (7). Each process start blocks the server's event loop for longer as its memory grows: at about 1 GB RSS a capture finished 46-66 ms sooner. When the output is not the expected two paths and HEAD's oid (Git before 2.31 echoes --path-format, and a path with a newline spans lines), the paths are read with the previous commands; a failed lookup still reports the same error.
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a contained performance fix that reduces redundant Git processes during existing checkpoint captures while preserving the existing capture and fallback behavior. The accompanying tests cover the main and compatibility paths, and no product defaults, APIs, schemas, or sensitive areas are changed. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description check |  |
The description covers the required Problem, Change, Scope and approval, and Verification sections. It explains the issue, implementation, compatibility behavior, tests, benchmarks, known failures, an… |
| Title check | |
The title clearly identifies the performance change and the reduction from 10 to 8 Git processes during checkpoint capture. It is concise and uses a conventional commit-style prefix. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Description check
-
Explanation
The description covers the required Problem, Change, Scope and approval, and Verification sections. It explains the issue, implementation, compatibility behavior, tests, benchmarks, known failures, and untested environments.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
This branch has not been deployed
1 participant
Problem
Every checkpoint capture starts 10 git processes one after another, and three of them are
git rev-parselookups that one process can answer. Captures run at the start and end of every turn, under a per-worktree lock, so a turn waits for each process to start and exit. On Linux each start also blocks the server's event loop for longer as the server's memory grows (libuv usesfork(); about 23 ms per start at 513 MB RSS, #21289). In #21289 a maintainer confirmed the T3 side: "a checkpoint capture on the common path runs 10 git processes in sequence".From the trace of the profiling run in #21289 (scratch server at
origin/main1b5c2d1, 6 concurrent Claude threads with 4 short turns each; since then the capture has only gained-cflags):rev-parse --git-common-dir,rev-parse --verify HEAD,config --bool core.sparseCheckout,rev-parse --path-format=absolute --git-path index,read-tree --reset HEAD,ls-files,add -A,write-tree,commit-treeandupdate-ref.rev-parselookups took 11–12 ms (p50), almost all of it starting and stopping the process.Change
One
git rev-parse --git-common-dir --path-format=absolute --git-path index --verify --quiet HEADreplaces the three lookups inapps/server/src/vcs/GitVcsDriver.ts. It prints the same values the separate commands did:--git-common-dircomes before--path-format, so it keeps its default form:.git,../../.gitfrom a subdirectory, or an absolute path in a linked worktree. The capture resolves that string as before.--git-path indexafter--path-format=absoluteprints the same absolute index path as the old lookup.--verifyprints HEAD's oid after both paths. With--quiet, an unborn HEAD exits 1 after printing only the paths, so exit 1 still means "no commit yet".A capture now starts 8 git processes instead of 10, or 6 instead of 7 with an unborn HEAD. Everything after the lookups is unchanged: the sparse checkout, skip-worktree, split index and nested repository handling, the durability flags, and the timeouts (the three lookups used the default 30 s, and so does the new one).
If the output isn't exactly two paths and an oid, the capture reads the paths with the old commands:
--path-formatand echoes it as an extra line. Its old index lookup also printed--path-format=absoluteplus a relative path, which the capture can't stat, so it rebuilt the index from HEAD; it still does. HEAD's existence comes from the exit code, so Git 2.30 still starts 9 processes per capture, as before.rev-parse --git-common-dir, which fails with the same error as before.I left the
core.sparseCheckoutprobe alone. Caching it per repository would save one more process, but Git can read that value from the repository'sconfigorconfig.worktree(wheregit sparse-checkoutwrites it onceextensions.worktreeConfigis on), global and system config,include.pathfiles,includeIf "onbranch:…"sections that change with HEAD, andGIT_CONFIG_*variables. A stale value changes whethergit addgets--sparseand how skip-worktree entries are read, so it needs its own design.Scope and approval
This is a focused performance fix inside the existing capture: three read-only lookups become one, with no change to what a checkpoint contains, which refs it writes, or the flags Git writes them with. #21289 established the problem, and the maintainer's comment linked above confirms the 10-process capture.
fork()cost. It doesn't touchGitVcsDriver.ts, and this PR doesn't touch process spawning. The two stack: with or without the spawn host, every process still costs an exec, Git's startup and an exit wait inside the per-worktree lock, and the spawn host only applies on Linux.durableWriteconstant in the same file. This PR doesn't touch it.git merge-treereports no conflict, and both suites below pass on the two branches merged together (76 passed).Verification
Tests, Git 2.53.0 on Linux:
cd apps/server && vp test run src/vcs/GitVcsDriver.test.ts src/checkpointing/CheckpointStore.test.ts: 76 passed. The 72 existing tests are unchanged; 4 are new.origin/mainboth fail, recording 10 and 7 commands.origin/main, which shows the fallback keeps today's behavior: the same error, the same 10 commands, the same checkpoint, and an untouched user index.cd apps/server && vp run --no-cache typecheck: only the two existingDuplicate identifier 'Path'errors inGrokAdapterV2.test.ts, whichmainalso has.vp lintandvp fmt --checkon both changed files: clean.Older Git: I built Git 2.30.9 from the kernel.org tarball and ran both suites with it first on
PATH, once onorigin/mainwith the new tests added and once on this branch. The per-test results match exactly: 48 passed and 28 failed on each. The failures are:sparse-checkout set --sparse-index(Git 2.32+);listAuthoredPathscases that uselog --diff-merges=remerge(Git 2.36+);A capture on Git 2.30.9 runs the same 9 processes in the same order on both.
Benchmark (scratch test, not committed):
CheckpointStore.captureCheckpointthrough the realVcsDriverRegistryandVcsProcesslayers, with git processes counted atProcessRunner. The setup:mainand this branch.Each cell is the median of the three rounds' p50 wall time, with the range across rounds:
main(10 processes)The event loop's busy time per capture (
performance.eventLoopUtilization()) fell by about the same amount, for example from 305 to 243 ms on the t3code checkout at ~1 GB, so the time saved is mostly the main thread blocked while starting processes. Before measuring the t3code checkout I rangit statusin it so its index wasn't racily clean. Straight after a checkout, every capture re-hashes most files (about 1.3 s per capture), which hides the difference.Not checked: macOS and Windows; Git versions other than 2.53.0 and 2.30.9; a fresh multi-thread server profile with this branch.
Written by Claude Opus 5.5 in Claude Code, orchestrated from T3 Code.