Skip to content

Cut worktree-switch latency by capping and deferring git reads - #291

Merged
frenchie4111 merged 6 commits into
mainfrom
perf/defer-worktree-git
Aug 22, 2026
Merged

Cut worktree-switch latency by capping and deferring git reads#291
frenchie4111 merged 6 commits into
mainfrom
perf/defer-worktree-git

Conversation

@frenchie4111

Copy link
Copy Markdown
Collaborator

Summary

Worktree switches stalled on git, not on CPU — during a stall perf.log showed lag=1ms while git status --porcelain sat at ~35% CPU statting thousands of files. This branch attacks the wait, not the work.

  • Global read-only git concurrency gate (src/main/git-limiter.ts), cap 4, with strict interactive > bulk priority. Permits are acquired at the leaf exec, never held across a nested await, so helpers that call helpers can't deadlock. Writes (merge, fetch, worktree add) deliberately bypass the gate — they're rare, user-initiated, long, and call read helpers internally.
  • Per-repo dedup + TTL cache for getMainWorktreeStatus. The panel asked directly while worktree:previewMerge asked internally at the same instant; that's now one read. Merge paths pass { force: true } so the gate never sees stale state, and invalidation happens after a merge so a racing read can't repopulate mid-merge.
  • Merge-conflict simulation off the switch critical path. previewMergeConflicts is deferred 500ms and cancelled on worktree change; handleMerge settles the in-flight preview before merging, so the conflict guard is preserved.
  • Cleanup's dirty sweep streams at bulk priority instead of one unbounded Promise.all, and renders per answer. dirtyMap now distinguishes unscanned from scanned-and-clean, and selection requires dirtyKnown — an unscanned worktree must never be auto-armed for deletion.

Measured result

A/B on the reference monorepo (20 worktrees, 80 switches per cell), wall-clock click→populated-panel against baseline src/:

scenario before after Δ
quiet p50 2063ms 1605ms −22%
quiet mean 2159ms 1660ms −23%
contended p50 2191ms 1573ms −28%
contended mean 2387ms 1685ms −29%
contended max 4593ms 3282ms −29%

The structural win is the last row's cause: a concurrent background dirty sweep used to add +228ms to the mean switch. It now adds +25ms.

Both benchmark tables (concurrency sweep and real-switch sweep) are recorded in the git-limiter.ts module comment so the cap of 4 stays falsifiable. The benchmark harness itself is not committed — it takes ~5min and points at a private repo.

Corrections to the originating brief

  • "Concurrency makes it strictly worse" is wrong. Total wall time is flat from cap 4 to cap 64; only per-call latency degrades (p50 77ms → 274ms). The cap is justified by queue fairness and tail latency, not throughput.
  • Cleanup's scan never ran with the modal closed (App.tsx gates on showCleanup), so it was never a background cost — only an in-modal one.
  • The bigger duplicate was worktree:previewMerge recomputing main status, which the brief didn't mention.

Negative results (investigated, not shipped)

  • Demoting XTerminal link-priming to bulk (listAllFiles / listRecentCommitShas): split benchmark gave panels-only mean 1972ms vs everything 1974ms. Priming is never the long pole. No change made.
  • Parallelizing getChangedFiles: already a Promise.all. I misread the [git-op] trace initially — execParts sums to cumulative exec time, not wall time.

Also fixed along the way

  • The main-status TTL was stamped at dispatch, before the awaits. On a repo where the reads outlast the TTL, every entry landed already expired and the cache could never serve anyone — exactly the slow-repo case it exists for. TTL now runs from settle time; in-flight entries are always fresh.
  • Pre-existing stale closures in PRStatusPanel: refreshStatus had deps [worktree.branch] while reading repoRoot/path; handleFix had [] deps over worktree.repoRoot.

Scope

This is switch latency only. It does not address the renderer CPU/RSS jank being handled in a separate worktree, and makes no claim about the app's general stutter.

Test plan

  • npm run typecheck
  • npx electron-vite build
  • npx vitest run — new: 5 limiter tests (cap, queueing, interactive-ahead-of-bulk, permit released on throw, bulk drains after), 6 real-git main-status cache tests (concurrent collapse, TTL hit, invalidation, forced re-read, per-repo keying, failures not cached)
  • A/B latency benchmark above, 80 switches per cell, quiet and contended
  • Manual: switch between worktrees and confirm the PR panel populates without waiting on conflict simulation
  • Manual: open Cleanup on a repo with many worktrees, confirm rows stream in and nothing unscanned is pre-selected for deletion

Known suite noise: 16 failures (8 unique × 2 vitest projects) in git-ops-state.test.ts, path-fix.test.ts, and worktree-watcher.integration.test.ts. These are load-sensitive real-git / fs.watch flakes — they pass in isolation, fail identically on baseline c6a2eb61, and none of them import the modules this branch touches.

🤖 Generated with Claude Code

frenchie4111 and others added 6 commits August 20, 2026 09:13
Panel reads here are I/O-bound, not CPU-bound, so the number that matters
is how long one read waits behind the others rather than total throughput.
Measured on the reference monorepo (18 worktrees, `git status --porcelain`
in each, warm cache), varying only the cap:

  cap=1   total 1626ms   p50   49ms   max  463ms
  cap=4   total  741ms   p50   77ms   max  495ms
  cap=8   total  718ms   p50  155ms   max  534ms
  cap=16  total  732ms   p50  274ms   max  619ms
  cap=64  total  766ms   p50  220ms   max  621ms

Throughput plateaus at 4 — past that, extra parallelism buys nothing and
only inflates per-call latency, since each read shares the disk with 15
others instead of 3. That makes the cap close to free, which is what
lets the second half work: interactive reads are dequeued ahead of bulk
ones, so a background sweep yields to the panel the user is looking at
instead of burying it.

Gating happens at the leaf exec rather than around whole helpers, so a
caller issuing several reads in sequence (getMainWorktreeStatus,
resolveDefaultBaseRef) takes and returns a permit per read instead of
holding one while waiting for another — which would deadlock at the cap.
Writes bypass the gate on purpose: merges, fetches and `worktree add` are
user-initiated, long, and call read helpers internally.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
getMainWorktreeStatus is keyed by repoRoot, not by worktree, so switching
between two worktrees of the same repo recomputes an identical answer. A
single switch also asks for it twice concurrently: MergeLocallyBody calls
it directly while worktree:previewMerge calls it internally. Each miss is
four sequential git spawns, one a full `git status` on the main checkout.

The in-flight entry collapses the concurrent pair; the short TTL covers
flipping back and forth. Reads that gate a mutation force past both --
mergeWorktreeLocally's readiness check must reflect the repo right now,
not up to a TTL ago -- and anything that moves main invalidates outright.
Failures are never cached, so one bad read can't poison a repo for the
whole TTL.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MergeLocallyBody ran previewMergeConflicts on every worktree.path change,
in parallel with the main-status read. It's a real three-way merge
simulation (`git merge-tree --write-tree`) answering a question the user
hasn't asked yet -- it only matters once they're looking at the merge
button -- so it sat on click-to-populated for nothing.

It now waits for the switch to settle, which also means scrubbing through
worktrees fires none of them at all. To keep the conflict guard honest
rather than downgrading it to "whatever had landed by click time",
handleMerge settles the deferred preview (starting one if the timer
hasn't fired yet) and refuses on conflict.

Also fixes stale closures next door: refreshStatus keyed its deps on
worktree.branch while reading repoRoot and path, and handleFix had empty
deps over worktree.repoRoot -- so switching between worktrees sharing a
branch name could act on the previous worktree's repo.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Opening Cleanup fired an unbounded Promise.all of isWorktreeDirty over
every non-main worktree. In the reference session that was a single burst
of ~60 git status spawns whose durations climbed as the queue drained,
worst call 19.8s -- and until the last one landed, the modal showed
nothing and every interactive read queued behind them.

The sweep now runs at 'bulk' priority, so the main process dequeues it
behind whatever panel the user is actually looking at, and answers land
one at a time rather than behind a barrier.

Streaming the results makes "not scanned yet" and "scanned, clean"
observably different, and conflating them is dangerous here: the default
selection is "select unless dirty", so an unscanned worktree would arm
the delete button over work that may not be committed. dirtyMap now
treats an absent key as unknown, and rows tick on as each is proven
clean.

(Contrary to how this reads from the outside, the scan was never running
with the modal closed -- Cleanup is mounted behind showCleanup.)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The cap was originally picked from a sweep microbenchmark (throughput
across N worktrees). That's not the workload it needs to be right for --
a single switch issues ~15-20 reads, which is a different shape. Re-ran
the cap sweep against the real switch benchmark, idle and under a
concurrent dirty sweep, and recorded the table.

Outcome doesn't change the value: 4 and 6 are within run-to-run noise on
the mean, and the tail under contention degrades monotonically as the cap
rises, so 4 keeps the best worst case.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stamping the entry when the read was dispatched is self-defeating on
exactly the repos this cache exists for: where the four spawns take
longer than the TTL, every entry lands already expired and the cache
never serves anyone. An in-flight entry is now always fresh (which is
what makes concurrent callers join), and the clock starts when the value
arrives.

Surfaced by the new tests flaking under a full parallel suite run, where
real git spawns are slow enough to cross the TTL mid-read. Those tests
also now carry explicit timeouts -- they shell out to git, and vitest's
5s default is not a budget real spawns reliably fit inside under load, so
a timeout there would read as a caching regression rather than a busy
machine.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@frenchie4111
frenchie4111 merged commit f516f96 into main Aug 22, 2026
1 check passed
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.

1 participant