Repository navigation
perf(worktrees): run three worktree deletes at once so a batch's third delete doesn't wait for a slot - #24293
brennanb2025 wants to merge 1 commit into
Conversation
…d delete doesn't wait for a slot
Review status: ready for review (head 4f1f95f)Problem: after #23837, Orca runs at most two User-facing change: deleting three large worktrees at once finishes about 4 s sooner (median 21.6 s → 17.6 s, faster in every measured round), and the third card's delete starts about 3 s after you confirm instead of 9–17 s. File reads and new-worktree creation stay as fast as before. What was fixed during review: nothing in the code; the PR description now says WSL deletes share this limit (unmeasured), states the 6–14 s figure that the limit actually causes, and adds the missing template sections. Verified: a separate review traced everything that runs under the limit (only the git delete, its leftover cleanup and the startup finish of interrupted deletes; branch cleanup and archive hooks stay one at a time per repo), ran a live race of 25 rounds with three concurrent removes plus a create in one repo (no failures), and confirmed the test fails if the limit is ignored, off by one, or out of order. Readiness checklist: no P0/P1. CI: 18 pass, 0 fail. Not verified / deferred: more than three deletes at once, slow or spinning disks, Windows with antivirus, WSL, and network filesystems are unmeasured; the in-process leftover cleanup (now up to three at once) was not measured. The cap of three bounds all of these. |
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
WORKTREE_DELETE_CONCURRENCY2 → 3 —src/main/git/worktree-delete-limit.ts. The main-process semaphore now admits three concurrentgit worktree removechildren. The JSDoc records the measured reason (a three-worktree batch finished ~4 s sooner; the cap still bounds disk contention and child count for larger batches).- Queueing test made limit-agnostic —
src/main/git/worktree-delete-limit.test.ts. The test deriveslimitandfirstWavefrom the constant instead of hardcoding[0, 1]/running: 2. It still pins the real behavior: onlylimitoperations start initially, thelimit-th +1 starts only after a slot frees, arrival order is preserved, and the snapshot returns to zero. - Comment correction —
src/shared/worktree/archive-hook-removal-gate.tsnow says "concurrent-delete limit" instead of "two-at-a-time".
Why this is sound
- The limit is deliberately admission-exempt (
admissionExempt: trueatsrc/main/git/worktree-removal.ts:77), so it does not draw from the general/network admission budget.MAX_GIT_CHILDREN = 10counts admitted children only (src/main/git/command-runner/git-admission-state.ts:11-13), so raising this constant does not violate that bound — the existing comment already anticipated a delete limit that moves independently. - Only
git worktree removeand the leftover-file cleanup run under it; branch cleanup and archive hooks remain serialized one-per-repo (worktree-removal.ts:121), so the packed-refs/fetch --pruneraces that motivated that serialization are untouched. - The client reply timeout (
worktreeRemovalReplyTimeoutMs) is a flat budget independent of the limit, so more concurrency only shortens queue wait — it makes timeouts safer, not tighter. - No stale references to the old value remain anywhere in the repo (searched
WORKTREE_DELETE_CONCURRENCY, "two-at-a-time", "delete slot", docs).
One thing to carry forward, not a blocker: the measurement is macOS-only, and the PR body already discloses Windows/WSL/spinning and network disks as unmeasured. The cap of three bounds the exposure, so approving.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe worktree delete concurrency limit increases from two to three. The concurrency test now uses the configured limit to size its gates and check queue progression. The timeout documentation refers to the host’s concurrent-delete limit. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to Three-worktree deletion batches can start the third delete sooner while remaining capped at three concurrent deletes. No concrete issue in the supplied review context blocks merging. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change permits one additional simultaneous deletion on a machine while retaining existing safety checks and queue limits. No introduced security issue was identified, but interruption handling and caller authentication were not fully verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Closing this: a size sweep showed that raising the limit to three doesn't make worktree deletes faster overall, so the limit stays at two. What was measured: deleting three worktrees at once while creating three, at four sizes, limit 2 vs limit 3, three interleaved rounds each (24 runs, M4Air Mac, on top of main 0f91540).
Why it's closed: the disk does the same work either way, so the total barely moves; the earlier "about 4 s faster" result did not reproduce cleanly. The limit only changes which cards wait: three helps a small worktree deleted alongside large ones, but makes the more common equal-size batch show "Deleting…" longer on average. File reads and new-worktree creation stayed fast at every size in both arms. |


What this fixes
Follow-up to #23837 (merged). When you delete several large worktrees at once, the third one waits for a free delete slot before git even starts on it, so its card shows "Deleting…" for an extra 6–14 s while nothing happens to it.
Scope: local and WSL worktree deletes from the desktop app, paired desktops, web, mobile and the CLI, only when three or more are being deleted at the same time on one machine. Single deletes and SSH worktrees are unchanged.
Background: #23837 moved the checkout delete back into git's own process (
git worktree remove) and limited how many of those git deletes run at once on a host to two (WORKTREE_DELETE_CONCURRENCY), on the assumption that two already saturate one disk. Measuring it showed that assumption was wrong.ELI5
Orca lets only a fixed number of worktree deletes run at the same time. With the limit at two, deleting three worktrees made the third wait for one of the first two to finish before it could even start. Raising the limit to three lets all three start together; each takes a little longer because they share the disk, but the whole batch finishes about 4 seconds sooner and the third card no longer waits for a slot.
What Changed
WORKTREE_DELETE_CONCURRENCYgoes from 2 to 3 (src/main/git/worktree-delete-limit.ts), with the comment updated to the measured reason. The cap stays: it still bounds disk contention and the number of git processes for larger batches, since these deletes don't count against the limit Orca puts on its other git commands.For the user: deleting three large worktrees at once finishes about 4 s sooner, and the third card starts its delete about 3 s after you confirm instead of 9–17 s. (The remaining ~3 s is Orca stopping that worktree's open terminal before deleting, which happens with any limit.)
Why
Measured on the M4Air Mac with three local builds of #23837 that differed only in this limit (2, 3, 8): delete three worktrees of 139,000 files each while creating three new ones, three interleaved rounds, load average 4–23.
With three deletes, limits 3 and 8 run the same path (all three at once), so the gap between them is noise. Running three at once makes each delete slower (disk contention), but the batch finishes sooner in every round and nothing else in the app slows down.
Differences from the common pattern
Known weaknesses
Linked Issue
No ticket. Found by measuring #23837 (deleting several worktrees while creating several).
Visual Proof
Testing
src/main/git/worktree-delete-limit.test.tspasses at limit 3 and at limit 2 (queueing order, slot hand-off, slot freed on failure).src/main/runtime/orca-runtime.test.tspasses.tscfor the node and web projects; oxlint, oxfmt and the changed-code quality gate.Review
Reviewed by a separate agent: the code change is sound. Only
git worktree remove, its leftover cleanup and the startup finish of interrupted deletes run under this limit; branch cleanup and archive hooks stay one at a time per repo. A live race of 25 rounds (three concurrent removes plus one concurrent create in one repo, Git 2.50) had no failures and left no stale entries. The test fails if the limit is ignored, off by one, or hands slots out in the wrong order.Agent skill upstream boundary
Notes
Security: none. Cross-platform: measured on macOS; WSL and Windows share the cap and are unmeasured. Remote SSH: not touched. Mobile: not touched. Backwards compatibility: no stored data or wire change. Performance: covered above.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)