Skip to content

perf: overlap sidebar-scope loads with worktree scan at startup (#7225)#7306

Merged
AmethystLiang merged 3 commits into
stablyai:mainfrom
xianjianlf2:perf/startup-lag
Jul 4, 2026
Merged

perf: overlap sidebar-scope loads with worktree scan at startup (#7225)#7306
AmethystLiang merged 3 commits into
stablyai:mainfrom
xianjianlf2:perf/startup-lag

Conversation

@xianjianlf2

@xianjianlf2 xianjianlf2 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

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:

  1. Overlap the sidebar-scope loads with the worktree scan. fetchProjectGroupsForAllHostsfetchFolderWorkspacesForAllHosts previously ran serially before the per-repo git worktree list fan-out. They enumerate hosts via runtimeEnvironments.list and 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/lineage Promise.all, so a slow/unreachable remote host's scope RPCs (up to 15s each) no longer queue ahead of the scan. Ordering (folder-workspaces reads projectGroups) and the "all settle before hydrate" invariant are preserved.
  2. Raise worktree refresh concurrency 5 → 8. Each repo's git worktree list is 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=1 startup timeline instead:

renderer-fetch-repos-done            durationMs=87
renderer-fetch-worktrees-done        durationMs=1     ← now overlapped with:
renderer-fetch-project-groups-done   durationMs=25
renderer-fetch-folder-workspaces-done durationMs=2

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-existing TS6307 project-reference errors are also present on unmodified main, 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 exported WORKTREE_REFRESH_CONCURRENCY) and app-startup-routing.test.ts (14 tests)
  • pnpm build — not run (renderer-only logic change, no build-surface impact)
  • Updated the two concurrency-bound tests to reference the exported constant and use CONCURRENCY + 2 repos so the bound is still exercised (10 > 8)

AI Review Report

Reviewed with the AI agent; main risks checked:

  • Store-write races: the two overlapped branches write disjoint keys (projectGroups/folderWorkspaces vs worktreesByRepo/detectedWorktreesByRepo/sortEpoch). Zustand set() stays atomic per key on the single-threaded event loop, so concurrency cannot cause a lost update. ✅
  • Session/reconnect correctness: mapped the dependency graph — folderWorkspaces (consumed by session hydration + terminal reconnect to build validWorktreeIds/pendingReconnectWorktreeIds) and worktreesByRepo are both still fully awaited before the hydrate steps, so no persisted tab/terminal/editor state is dropped. fetchAllWorktrees reads only repos/settings/projectHostSetups (already loaded), not projectGroups/folderWorkspaces. ✅
  • Error handling / unhandled rejections: the concurrent projectScopeChain promise is created and passed into Promise.all in the same synchronous tick (handler attached before any microtask can reject), and a rejection still propagates to the existing outer startup catch. Terminal behavior unchanged. ✅
  • Concurrency change: affects parallelism only, not scan results (each repo scanned exactly once); still bounded.
  • Cross-platform (macOS / Linux / Windows): no keyboard shortcuts, UI labels, file paths, or shell invocations touched. Both changes are platform-agnostic (JS promise ordering + a bound on main-process git child processes). Slow startup and UI freezing issue analysis / 启动慢、界面卡顿问题分析 #7225 is Windows-focused and this reduces its scan-batch count there. No Electron platform-specific surface touched.

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 list child 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

…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]>
@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d824c28c-f39c-46a1-8b0a-a94ada43363c

📥 Commits

Reviewing files that changed from the base of the PR and between 04159af and 2592dc2.

📒 Files selected for processing (1)
  • src/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

📝 Walkthrough

Walkthrough

Renderer startup now overlaps project scope loading with worktree and lineage hydration in App.tsx. Startup routing tests now use a narrower startup-block marker and assert the ordering of startup actions. The worktree refresh concurrency constant is exported, increased from 5 to 8, and reused in the worktree slice tests for scan-count and concurrency assertions.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: overlapping sidebar-scope loads with startup worktree scans.
Description check ✅ Passed The description matches the template and includes summary, screenshots, testing, AI review, security audit, and notes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@AmethystLiang
AmethystLiang self-requested a review July 4, 2026 07:44
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 AmethystLiang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/renderer/src/app-startup-routing.test.ts (1)

18-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Missing marker existence checks let ordering assertions silently pass on indexOf misses.

Only reposIndex is checked for presence (toBeGreaterThanOrEqual(0)). If promiseAllIndex (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 to indexOf('projectScopeChain', promiseAllIndex) on Line 29: with promiseAllIndex = -1, the search clamps to index 0 and can silently match the earlier const projectScopeChain = ... declaration instead of the intended reference inside Promise.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

📥 Commits

Reviewing files that changed from the base of the PR and between f559cb3 and 04159af.

📒 Files selected for processing (2)
  • src/renderer/src/app-startup-routing.test.ts
  • src/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

@AmethystLiang
AmethystLiang merged commit 9964d1c into stablyai:main Jul 4, 2026
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.

2 participants