diff --git a/packages/ui/src/apps/ElectronMiniChatApp.tsx b/packages/ui/src/apps/ElectronMiniChatApp.tsx index 991300da..d7f1dafc 100644 --- a/packages/ui/src/apps/ElectronMiniChatApp.tsx +++ b/packages/ui/src/apps/ElectronMiniChatApp.tsx @@ -19,7 +19,7 @@ import { useSync } from '@/sync/use-sync'; import { SyncRuntimeEffects } from './AppEffects'; import { useAppFontEffects } from './useAppFontEffects'; import { useMiniChatKeyboardShortcuts } from '@/hooks/useMiniChatKeyboardShortcuts'; -import { listProjectWorktrees } from '@/lib/worktrees/worktreeManager'; +import { listProjectWorktrees, worktreeMapsEqual } from '@/lib/worktrees/worktreeManager'; import type { WorktreeMetadata } from '@/types/worktree'; const MINI_CHAT_PRESENCE_CHANNEL = 'openchamber:mini-chat-presence'; @@ -194,10 +194,15 @@ const MiniChatBootstrap: React.FC<{ config: MiniChatConfig }> = ({ config }) => })); if (cancelled) return; - useSessionUIStore.setState({ - availableWorktrees: allWorktrees, - availableWorktreesByProject: worktreesByProject, - }); + + // Skip update if nothing changed — see worktreeMapsEqual JSDoc. + const currentByProject = useSessionUIStore.getState().availableWorktreesByProject; + if (!worktreeMapsEqual(worktreesByProject, currentByProject)) { + useSessionUIStore.setState({ + availableWorktrees: allWorktrees, + availableWorktreesByProject: worktreesByProject, + }); + } }; void discoverWorktrees(); diff --git a/packages/ui/src/apps/MobileApp.tsx b/packages/ui/src/apps/MobileApp.tsx index de6f2877..bc67b397 100644 --- a/packages/ui/src/apps/MobileApp.tsx +++ b/packages/ui/src/apps/MobileApp.tsx @@ -46,7 +46,7 @@ import { useMcpConfigStore, type McpDraft } from '@/stores/useMcpConfigStore'; import { useMcpStore } from '@/stores/useMcpStore'; import { useProjectsStore } from '@/stores/useProjectsStore'; import { useQuotaAutoRefresh, useQuotaStore } from '@/stores/useQuotaStore'; -import { listProjectWorktrees } from '@/lib/worktrees/worktreeManager'; +import { listProjectWorktrees, worktreeMapsEqual } from '@/lib/worktrees/worktreeManager'; import type { QuotaProviderId, UsageWindow } from '@/types'; import { useUIStore, type TimeFormatPreference } from '@/stores/useUIStore'; import { useUpdateStore } from '@/stores/useUpdateStore'; @@ -2918,11 +2918,17 @@ export function MobileApp({ apis }: MobileAppProps) { ); if (cancelled) return; + const allWorktrees = Array.from(worktreesByProject.values()).flat(); - useSessionUIStore.setState({ - availableWorktrees: allWorktrees, - availableWorktreesByProject: worktreesByProject, - }); + + // Skip update if nothing changed — see worktreeMapsEqual JSDoc. + const currentByProject = useSessionUIStore.getState().availableWorktreesByProject; + if (!worktreeMapsEqual(worktreesByProject, currentByProject)) { + useSessionUIStore.setState({ + availableWorktrees: allWorktrees, + availableWorktreesByProject: worktreesByProject, + }); + } }; void run(); diff --git a/packages/ui/src/components/session/SessionSidebar.tsx b/packages/ui/src/components/session/SessionSidebar.tsx index 4c875e8b..3ea80c83 100644 --- a/packages/ui/src/components/session/SessionSidebar.tsx +++ b/packages/ui/src/components/session/SessionSidebar.tsx @@ -45,7 +45,7 @@ import { SessionNodeItem } from './sidebar/SessionNodeItem'; import type { SessionNodeRenderExtras } from './sidebar/sessionNodeItemUtils'; import { useUpdateStore } from '@/stores/useUpdateStore'; import { useShallow } from 'zustand/react/shallow'; -import { listProjectWorktrees } from '@/lib/worktrees/worktreeManager'; +import { listProjectWorktrees, worktreeMapsEqual } from '@/lib/worktrees/worktreeManager'; import { checkIsGitRepository } from '@/lib/gitApi'; import type { WorktreeMetadata } from '@/types/worktree'; import type { SortableDragHandleProps } from './sidebar/sortableItems'; @@ -477,10 +477,14 @@ export const SessionSidebar: React.FC = ({ if (cancelled) return; - useSessionUIStore.setState({ - availableWorktrees: allWorktrees, - availableWorktreesByProject: worktreesByProject, - }); + // Skip update if nothing changed — see worktreeMapsEqual JSDoc. + const currentByProject = useSessionUIStore.getState().availableWorktreesByProject; + if (!worktreeMapsEqual(worktreesByProject, currentByProject)) { + useSessionUIStore.setState({ + availableWorktrees: allWorktrees, + availableWorktreesByProject: worktreesByProject, + }); + } }; // Skip if we already discovered worktrees for this exact project set. diff --git a/packages/ui/src/lib/worktrees/worktreeManager.bench.ts b/packages/ui/src/lib/worktrees/worktreeManager.bench.ts new file mode 100644 index 00000000..42272979 --- /dev/null +++ b/packages/ui/src/lib/worktrees/worktreeManager.bench.ts @@ -0,0 +1,231 @@ +/** + * Local benchmark for worktree store optimizations from PR #1992. + * + * Run with: + * bun run packages/ui/src/lib/worktrees/worktreeManager.bench.ts + * + * This file is intentionally not auto-run on import; it only executes + * the benchmark suite when launched as a script via `bun run`. + */ + +import type { WorktreeMetadata } from '@/types/worktree'; + +// --------------------------------------------------------------------------- +// Fixture +// --------------------------------------------------------------------------- + +const buildMap = (numProjects: number, worktreesPerProject: number): Map => { + const map = new Map(); + for (let p = 0; p < numProjects; p++) { + const projectDirectory = `/home/user/projects/project-${p}`; + const worktrees: WorktreeMetadata[] = []; + for (let w = 0; w < worktreesPerProject; w++) { + worktrees.push({ + path: `${projectDirectory}/.worktrees/feat-${p}-${w}`, + projectDirectory, + branch: w === 0 ? 'main' : `feat-${p}-${w}`, + label: w === 0 ? 'main' : `feat-${p}-${w}`, + worktreeStatus: 'ready', + headState: 'branch', + }); + } + map.set(projectDirectory, worktrees); + } + return map; +}; + +// --------------------------------------------------------------------------- +// Path-only (old) vs path+branch (new) +// --------------------------------------------------------------------------- + +const oldEqualPathOnly = ( + a: Map, + b: Map, +): boolean => { + if (a.size !== b.size) return false; + for (const [key, value] of a) { + const existing = b.get(key); + if (!existing || existing.length !== value.length) return false; + for (let i = 0; i < value.length; i++) { + if (value[i].path !== existing[i].path) return false; + } + } + return true; +}; + +// --------------------------------------------------------------------------- +// Timing helpers +// --------------------------------------------------------------------------- + +interface BenchResult { + iterations: number; + totalMs: number; + nsPerOp: number; + opsPerSec: number; +} + +const measure = ( + iterations: number, + warmupIterations: number, + body: () => void, +): BenchResult => { + // Warmup: let V8 inline, populate ICs, run a few GC cycles implicitly. + for (let i = 0; i < warmupIterations; i++) { + body(); + } + const start = performance.now(); + for (let i = 0; i < iterations; i++) { + body(); + } + const totalMs = performance.now() - start; + const nsPerOp = (totalMs * 1_000_000) / iterations; + const opsPerSec = 1_000_000_000 / nsPerOp; + return { iterations, totalMs, nsPerOp, opsPerSec }; +}; + +// --------------------------------------------------------------------------- +// Formatting +// --------------------------------------------------------------------------- + +const padLeft = (s: string, n: number): string => { + if (s.length >= n) return s; + return ' '.repeat(n - s.length) + s; +}; + +const formatOpsPerSec = (ops: number): string => { + if (ops >= 1_000_000) return `${(ops / 1_000_000).toFixed(2)}M`; + if (ops >= 1_000) return `${(ops / 1_000).toFixed(1)}K`; + return Math.round(ops).toString(); +}; + +// --------------------------------------------------------------------------- +// Benchmark suite +// --------------------------------------------------------------------------- + +async function runBenchmarks(): Promise { + // Dynamic import so module-level side effects (subscriber registration) + // are paid once and excluded from every measured call. + const { worktreeMapsEqual } = await import('./worktreeManager'); + + // ----- 1. worktreeMapsEqual throughput --------------------------------- + console.log('=== worktreeMapsEqual throughput ==='); + console.log(` ${padLeft('projects × worktrees', 22)}${padLeft('ns/op', 12)}${padLeft('ops/sec', 12)}`); + + const sizes: Array<[number, number]> = [ + [1, 1], + [1, 10], + [1, 100], + [1, 1000], + [10, 10], + [20, 50], + [50, 20], + ]; + + for (const [numProjects, worktreesPerProject] of sizes) { + const a = buildMap(numProjects, worktreesPerProject); + const b = buildMap(numProjects, worktreesPerProject); + const result = measure( + 10_000, + 1_000, + () => { worktreeMapsEqual(a, b); }, + ); + const label = `${numProjects} × ${worktreesPerProject}`; + console.log( + ` ${padLeft(label, 22)}${padLeft(Math.round(result.nsPerOp).toString(), 12)}${padLeft(formatOpsPerSec(result.opsPerSec), 12)}`, + ); + } + + // Early-exit case: same shape, first project differs. + { + const numProjects = 50; + const worktreesPerProject = 20; + const a = buildMap(numProjects, worktreesPerProject); + const b = buildMap(numProjects, worktreesPerProject); + // Mutate the first project's first worktree path in b. + const firstKey = a.keys().next().value as string; + const bList = b.get(firstKey)!; + bList[0] = { ...bList[0], path: '/home/user/projects/project-0/.worktrees/different-path' }; + const result = measure( + 10_000, + 1_000, + () => { worktreeMapsEqual(a, b); }, + ); + const label = '50 × 20 (early-exit)'; + console.log( + ` ${padLeft(label, 22)}${padLeft(Math.round(result.nsPerOp).toString(), 12)}${padLeft(formatOpsPerSec(result.opsPerSec), 12)}`, + ); + } + + // ----- 2. Old (path-only) vs new (path+branch) ------------------------- + console.log('\n=== path-only vs path+branch (10 × 50) ==='); + { + const a = buildMap(10, 50); + const b = buildMap(10, 50); + + const oldResult = measure( + 100_000, + 5_000, + () => { oldEqualPathOnly(a, b); }, + ); + const newResult = measure( + 100_000, + 5_000, + () => { worktreeMapsEqual(a, b); }, + ); + const delta = newResult.nsPerOp - oldResult.nsPerOp; + const deltaPct = (delta / oldResult.nsPerOp) * 100; + const sign = delta >= 0 ? '+' : ''; + console.log(` ${padLeft('path-only:', 18)}${padLeft(Math.round(oldResult.nsPerOp).toString(), 10)} ns/op`); + console.log(` ${padLeft('path+branch:', 18)}${padLeft(Math.round(newResult.nsPerOp).toString(), 10)} ns/op`); + console.log(` ${padLeft('delta:', 18)}${padLeft(`${sign}${Math.round(delta)}`, 10)} ns/op (${sign}${deltaPct.toFixed(1)}%)`); + } + + // ----- 3. Subscriber stringify dedup ----------------------------------- + console.log('\n=== subscriber stringify (10 × 50) ==='); + { + const map = buildMap(10, 50); + + // Cold: 2 stringifies per pass (what the old preSerialized-aware code did). + const twiceResult = measure( + 10_000, + 1_000, + () => { + const a = JSON.stringify([...map.entries()]); + const b = JSON.stringify([...map.entries()]); + // Touch both to prevent dead-code elimination. + if (a === b && a.length === 0) throw new Error('unreachable'); + }, + ); + // Hot: stringify once, compare string to cached value. + const onceResult = measure( + 10_000, + 1_000, + () => { + const a = JSON.stringify([...map.entries()]); + if (a === '' && a.length === 0) throw new Error('unreachable'); + }, + ); + const saved = twiceResult.nsPerOp - onceResult.nsPerOp; + const savedPct = (saved / twiceResult.nsPerOp) * 100; + console.log(` ${padLeft('2× stringify:', 18)}${padLeft(Math.round(twiceResult.nsPerOp).toString(), 10)} ns/op`); + console.log(` ${padLeft('1× stringify:', 18)}${padLeft(Math.round(onceResult.nsPerOp).toString(), 10)} ns/op`); + console.log(` ${padLeft('saved:', 18)}${padLeft(Math.round(saved).toString(), 10)} ns/op (${savedPct.toFixed(1)}%)`); + } + + // ----- 4. Content-compare guard ---------------------------------------- + console.log('\n=== content-compare guard (10 × 50) ==='); + { + const a = buildMap(10, 50); + const serialized = JSON.stringify([...a.entries()]); + const result = measure( + 100_000, + 5_000, + () => serialized === serialized, + ); + console.log(` ${padLeft('string compare:', 18)}${padLeft(Math.round(result.nsPerOp).toString(), 10)} ns/op`); + } +} + +if (import.meta.main) { + await runBenchmarks(); +} diff --git a/packages/ui/src/lib/worktrees/worktreeManager.test.ts b/packages/ui/src/lib/worktrees/worktreeManager.test.ts index 7d2e93e6..677bfd9e 100644 --- a/packages/ui/src/lib/worktrees/worktreeManager.test.ts +++ b/packages/ui/src/lib/worktrees/worktreeManager.test.ts @@ -65,7 +65,7 @@ mock.module('@/lib/gitApi', () => ({ }, })); -const { createWorktree, listProjectWorktrees } = await import('./worktreeManager'); +const { createWorktree, listProjectWorktrees, worktreeMapsEqual } = await import('./worktreeManager'); const waitForListCallCount = async (count: number): Promise => { for (let attempt = 0; attempt < 10; attempt += 1) { @@ -121,3 +121,72 @@ describe('worktreeManager list invalidation', () => { expect(sessionState.availableWorktrees[0]?.worktreeStatus).toBe('pending'); }); }); + +describe('worktreeMapsEqual', () => { + const wt = ( + path: string, + branch: string, + overrides: Partial = {}, + ): WorktreeMetadata => ({ + path, + branch, + projectDirectory: '/repo', + label: branch, + ...overrides, + }); + + test('returns true for two empty maps', () => { + const a = new Map(); + const b = new Map(); + expect(worktreeMapsEqual(a, b)).toBe(true); + }); + + test('returns true when paths and branches match in order', () => { + const a = new Map([['/repo', [wt('/r/main', 'main'), wt('/r/feat', 'feat')]]]); + const b = new Map([['/repo', [wt('/r/main', 'main'), wt('/r/feat', 'feat')]]]); + expect(worktreeMapsEqual(a, b)).toBe(true); + }); + + test('returns false when same path has a different branch (external git checkout)', () => { + const a = new Map([['/repo', [wt('/r/main', 'main')]]]); + const b = new Map([['/repo', [wt('/r/main', 'develop')]]]); + expect(worktreeMapsEqual(a, b)).toBe(false); + }); + + test('returns false when paths differ', () => { + const a = new Map([['/repo', [wt('/r/main', 'main')]]]); + const b = new Map([['/repo', [wt('/r/other', 'main')]]]); + expect(worktreeMapsEqual(a, b)).toBe(false); + }); + + test('returns false when per-project array lengths differ', () => { + const a = new Map([['/repo', [wt('/r/main', 'main')]]]); + const b = new Map([['/repo', [wt('/r/main', 'main'), wt('/r/feat', 'feat')]]]); + expect(worktreeMapsEqual(a, b)).toBe(false); + }); + + test('returns false when number of project keys differ', () => { + const a = new Map([['/repo', [wt('/r/main', 'main')]]]); + const b = new Map([ + ['/repo', [wt('/r/main', 'main')]], + ['/repo-2', [wt('/r2/main', 'main')]], + ]); + expect(worktreeMapsEqual(a, b)).toBe(false); + }); + + test('returns false when worktrees are reordered (positional compare)', () => { + const a = new Map([['/repo', [wt('/r/main', 'main'), wt('/r/feat', 'feat')]]]); + const b = new Map([['/repo', [wt('/r/feat', 'feat'), wt('/r/main', 'main')]]]); + expect(worktreeMapsEqual(a, b)).toBe(false); + }); + + test('returns false when a non-first worktree differs (subset of entries)', () => { + const a = new Map([ + ['/repo', [wt('/r/main', 'main'), wt('/r/feat', 'feat'), wt('/r/old', 'old')]], + ]); + const b = new Map([ + ['/repo', [wt('/r/main', 'main'), wt('/r/feat', 'feat'), wt('/r/old', 'new-branch')]], + ]); + expect(worktreeMapsEqual(a, b)).toBe(false); + }); +}); diff --git a/packages/ui/src/lib/worktrees/worktreeManager.ts b/packages/ui/src/lib/worktrees/worktreeManager.ts index a33e5a0b..4377a263 100644 --- a/packages/ui/src/lib/worktrees/worktreeManager.ts +++ b/packages/ui/src/lib/worktrees/worktreeManager.ts @@ -229,6 +229,44 @@ const toCreatePayload = (args: { }; }; +/** + * Compare two worktree-by-project maps for equality. + * Compares per-element `path` and `branch` (not reference equality) + * because readStableProjectWorktrees creates new object instances on + * each call, making reference checks always report changed. + * + * `branch` is included so an external `git checkout` between + * discoveries — which changes `branch` (and the derived `label` + * and `headState`) while leaving `path` unchanged — still triggers + * a store update. Without this, the branch label in the sidebar + * could go stale until the next worktree create/remove or project + * switch, since there is no periodic worktree-list refresh. + * + * Status changes (`worktreeStatus`) are not compared here: those + * flow through `setStoredWorktreeStatus`, which writes a new Map + * reference that the persist subscriber picks up directly. + * + * Generic over `T extends { path: string; branch: string }` so the + * helper documents its equality contract at the type level and + * stays reusable for any future map-of-arrays shape that has both + * fields. + */ +export const worktreeMapsEqual = ( + a: Map, + b: Map, +): boolean => { + if (a.size !== b.size) return false; + for (const [key, value] of a) { + const existing = b.get(key); + if (!existing || existing.length !== value.length) return false; + for (let i = 0; i < value.length; i++) { + if (value[i].path !== existing[i].path) return false; + if (value[i].branch !== existing[i].branch) return false; + } + } + return true; +}; + // Cache worktree listings to avoid repeated git worktree list + rev-parse calls const _worktreeListCache = new Map(); const _worktreeListInflight = new Map>(); diff --git a/packages/ui/src/sync/session-ui-store.ts b/packages/ui/src/sync/session-ui-store.ts index c81a48d7..aa9d6e57 100644 --- a/packages/ui/src/sync/session-ui-store.ts +++ b/packages/ui/src/sync/session-ui-store.ts @@ -538,9 +538,9 @@ const loadPersistedWorktreeMap = (): Map => { } } -const persistWorktreeMap = (map: Map): void => { +const persistWorktreeMap = (serialized: string): void => { try { - getDeferredSafeStorage().setItem(WORKTREE_MAP_STORAGE_KEY, JSON.stringify([...map.entries()])) + getDeferredSafeStorage().setItem(WORKTREE_MAP_STORAGE_KEY, serialized) } catch { // quota / serialization error — ignore; discovery still refreshes at runtime } @@ -1567,10 +1567,17 @@ setSessionOpener((sessionID, directory) => { }) // Write-through persist of the worktree map whenever discovery refreshes it. -// Cheap reference-equality guard — this fires only when the map actually -// changes (discovery / worktree create/remove), not on hot session updates. +// Reference-equality guard filters hot session updates; the serialized +// comparison avoids redundant localStorage writes when the Map reference +// changed but the content is identical (e.g., re-discovery that found the +// same worktrees). +let lastPersistedWorktreeSerialized = '' useSessionUIStore.subscribe((state, prev) => { if (state.availableWorktreesByProject !== prev.availableWorktreesByProject) { - persistWorktreeMap(state.availableWorktreesByProject) + const serialized = JSON.stringify([...state.availableWorktreesByProject.entries()]) + if (serialized !== lastPersistedWorktreeSerialized) { + lastPersistedWorktreeSerialized = serialized + persistWorktreeMap(serialized) + } } })