perf: overlap sidebar-scope loads with worktree scan at startup (#7225)#7306
Conversation
…lyai#7225) Continues the renderer-chain parallelization proposed in stablyai#7225 (proposal 2), on top of the merged stablyai#7266: - Run project-groups → folder-workspaces concurrently with the per-repo `git worktree list` fan-out. Neither reads the repos store nor worktrees, so a slow remote host's 15s scope RPCs no longer queue ahead of the scan. - Raise worktree refresh concurrency 5 → 8 so multi-core machines run fewer sequential scan batches, still bounded so one moment can't launch every git probe at once. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRenderer startup now overlaps project scope loading with worktree and lineage hydration in 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Verify that sidebar scope loads and worktree hydration operations run concurrently before session hydration. This protects against regressions in startup performance under the perf/startup-lag optimization. Additionally, document the rationale for the worktree refresh concurrency limit in the worktrees slice.
AmethystLiang
left a comment
There was a problem hiding this comment.
Thanks, this direction looks good. I pushed a couple of small review fixes:
- added startup-routing regression coverage so sidebar scope loads stay overlapped with worktree/lineage hydration before session hydration
- cleaned up the worktree refresh concurrency rationale comment
Validated with pnpm typecheck, pnpm lint, the affected Vitest suites, and an Electron startup smoke with a persisted local repo. The remaining performance tradeoff is the bounded 8-way worktree scan concurrency, which looks reasonable for this startup path.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/src/app-startup-routing.test.ts (1)
18-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing marker existence checks let ordering assertions silently pass on
indexOfmisses.Only
reposIndexis checked for presence (toBeGreaterThanOrEqual(0)). IfpromiseAllIndex(or any other marker) fails to match and returns-1, the comparisons on Lines 38-40 (... > promiseAllIndex) will trivially pass since any positive index is> -1, hiding a broken/renamed marker instead of failing the test. The same risk applies toindexOf('projectScopeChain', promiseAllIndex)on Line 29: withpromiseAllIndex = -1, the search clamps to index 0 and can silently match the earlierconst projectScopeChain = ...declaration instead of the intended reference insidePromise.all.🔧 Suggested fix: assert presence for all markers before comparing order
const reposIndex = startupBlock.indexOf('actions.fetchReposForAllHosts()') const scopeChainIndex = startupBlock.indexOf('const projectScopeChain = (async () => {') const projectGroupsIndex = startupBlock.indexOf('actions.fetchProjectGroupsForAllHosts()') const folderWorkspacesIndex = startupBlock.indexOf('actions.fetchFolderWorkspacesForAllHosts()') const promiseAllIndex = startupBlock.indexOf('await Promise.all([') const awaitedScopeChainIndex = startupBlock.indexOf('projectScopeChain', promiseAllIndex) const worktreesIndex = startupBlock.indexOf('actions.fetchAllWorktrees()') const lineageIndex = startupBlock.indexOf('actions.fetchWorktreeLineage()') expect(reposIndex).toBeGreaterThanOrEqual(0) + expect(scopeChainIndex).toBeGreaterThanOrEqual(0) + expect(projectGroupsIndex).toBeGreaterThanOrEqual(0) + expect(folderWorkspacesIndex).toBeGreaterThanOrEqual(0) + expect(promiseAllIndex).toBeGreaterThanOrEqual(0) + expect(awaitedScopeChainIndex).toBeGreaterThanOrEqual(0) + expect(worktreesIndex).toBeGreaterThanOrEqual(0) + expect(lineageIndex).toBeGreaterThanOrEqual(0) expect(scopeChainIndex).toBeGreaterThan(reposIndex)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 76abe87b-086a-4b23-ad96-7830a1d23c87
📒 Files selected for processing (2)
src/renderer/src/app-startup-routing.test.tssrc/renderer/src/store/slices/worktrees.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/src/store/slices/worktrees.ts
Summary
Continues the renderer startup-chain parallelization proposed in #7225 (proposal 2), on top of the merged #7266 (which fixed the main-process blocking — proposal 1). Two small changes to the renderer startup effect (
App.tsx) and the worktree slice:fetchProjectGroupsForAllHosts→fetchFolderWorkspacesForAllHostspreviously ran serially before the per-repogit worktree listfan-out. They enumerate hosts viaruntimeEnvironments.listand read neither the repos store nor worktrees, so once repos have landed they are independent of the scan. They now run concurrently with the worktree/lineagePromise.all, so a slow/unreachable remote host's scope RPCs (up to 15s each) no longer queue ahead of the scan. Ordering (folder-workspacesreadsprojectGroups) and the "all settle before hydrate" invariant are preserved.git worktree listis an independent main-process child, so a higher ceiling runs fewer sequential scan batches at startup on multi-core machines — the dominant renderer-chain cost from Slow startup and UI freezing issue analysis / 启动慢、界面卡顿问题分析 #7225. Still bounded so one UI moment can't launch every git probe at once.User-visible effect: faster startup, most noticeably for users with many repos and/or a configured-but-unreachable remote host. No behavior or UI change otherwise.
Screenshots
No visual change. Verified via the
ORCA_STARTUP_DIAGNOSTICS=1startup timeline instead:The four fetches complete within the same window (t≈2980–3000) instead of serializing.
Testing
pnpm lint— oxlint clean on the changed files (App.tsx,worktrees.ts,worktrees.test.ts)pnpm typecheck— changed files type-clean (the 5 pre-existingTS6307project-reference errors are also present on unmodifiedmain, unrelated to this PR)pnpm test— ran the affected suites green:worktrees.test.ts(204 tests, incl. the two "bounds concurrent repo scans" tests now asserting against the exportedWORKTREE_REFRESH_CONCURRENCY) andapp-startup-routing.test.ts(14 tests)pnpm build— not run (renderer-only logic change, no build-surface impact)CONCURRENCY + 2repos so the bound is still exercised (10 > 8)AI Review Report
Reviewed with the AI agent; main risks checked:
projectGroups/folderWorkspacesvsworktreesByRepo/detectedWorktreesByRepo/sortEpoch). Zustandset()stays atomic per key on the single-threaded event loop, so concurrency cannot cause a lost update. ✅folderWorkspaces(consumed by session hydration + terminal reconnect to buildvalidWorktreeIds/pendingReconnectWorktreeIds) andworktreesByRepoare both still fully awaited before the hydrate steps, so no persisted tab/terminal/editor state is dropped.fetchAllWorktreesreads onlyrepos/settings/projectHostSetups(already loaded), notprojectGroups/folderWorkspaces. ✅projectScopeChainpromise is created and passed intoPromise.allin the same synchronous tick (handler attached before any microtask can reject), and a rejection still propagates to the existing outer startupcatch. Terminal behavior unchanged. ✅Security Audit
Low surface. No new IPC handlers, command construction, path handling, auth, secrets, or dependency changes. The concurrency constant only bounds how many existing
git worktree listchild processes run in parallel (raised 5 → 8, still capped — no unbounded spawn). Reordering is pure in-renderer promise scheduling. No user input flows through the changed code. No follow-up required.Notes
🤖 Generated with Claude Code