perf(worktree): skip unchanged store updates and content-aware persist (#1992)

* perf(worktree): skip unchanged store updates and content-aware persist

- Add content-aware equality check before setState in all three discovery
  loops (SessionSidebar, ElectronMiniChatApp, MobileApp). Compares
  Map size and per-entry length + element references — avoids triggering
  16+ subscriber re-renders when discovery finds the same worktrees.
- Add content-hash guard to persistWorktreeMap subscription with try-catch.
  Avoids redundant localStorage writes when the Map reference changed but
  the content is identical. Serialization errors are caught and skipped.

Contributes to #1990

* perf(worktree): extract shared worktreeMapsEqual, fix comparison, avoid double serialization

- Extract worktreeMapsEqual() into worktreeManager.ts as a shared utility
  comparing worktree maps by path (not reference identity). This replaces
  the inline reference-comparison logic in all three discovery loops
  (SessionSidebar, ElectronMiniChatApp, MobileApp) that was ineffective
  because readStableProjectWorktrees creates new object instances on
  each call after cache expiry, making item !== value[i] always true.
- Pass pre-serialized JSON to persistWorktreeMap to avoid double
  JSON.stringify on every persist. The subscriber already computes the
  serialized string for the content-hash check; pass it through instead
  of re-serializing inside persistWorktreeMap.
- Deduplicate 3 copies of the same comparison logic into the shared util.

* refactor(worktree): make worktreeMapsEqual generic over path-bearing type

The helper's equality contract is element-wise path comparison,
not anything specific to WorktreeMetadata. Generifying on
`T extends { path: string }` documents the contract at the type
level and keeps it reusable for any future map-of-arrays shape
that has a path field. Call sites stay compatible since
WorktreeMetadata has a required `path: string`.

No runtime change.

* refactor(worktree-store): clarify persist hash name and signature

Drop the optional preSerialized parameter from persistWorktreeMap —
its only caller (the subscriber) already builds the serialized
string for the content-compare, so the dual-path body is dead code.
persistWorktreeMap now takes the serialized string directly.

Rename lastPersistedWorktreeHash → lastPersistedWorktreeSerialized
(the variable holds the full JSON string, not a hash) and drop the
try/catch around JSON.stringify: it cannot realistically throw on
Map.entries() of WorktreeMetadata (no circular refs, no BigInt, no
custom toJSON). The try/catch around setItem stays — it can throw
on quota errors.

No behavior change in the success path.

* docs(worktree): trim repeated call-site comments

Replace the 5-line explanation block (copy-pasted in all three
discovery loops) with a one-liner that points at the worktreeMapsEqual
JSDoc. The '16+ subscribers' framing is also dropped — the helper
itself is general-purpose and the precise number was fuzzy.

* fix(worktree): compare branch in worktreeMapsEqual to avoid stale sidebar label

The helper compared entries by path only. An external git checkout
between discoveries changes branch (and the derived label /
headState) while path stays the same, so the helper returned true
and the store update was skipped — leaving a stale branch label in
the sidebar until the next worktree create/remove or project switch,
since there is no periodic worktree-list refresh.

Compare branch in the inner loop alongside path. Tighten the generic
constraint to T extends { path: string; branch: string } so the
contract is documented at the type level.

worktreeStatus is intentionally NOT compared: status transitions go
through setStoredWorktreeStatus, which writes a fresh Map reference
that the persist subscriber picks up directly. Adding worktreeStatus
to the contract would also force the sidebar to detect status changes
that the persist path already handles, and would couple this helper
to a field whose semantics differ from the discovery path.

Fixes the staleness concern raised by openchamber-bot in PR #1992.

* test(worktree): cover worktreeMapsEqual edge cases

Documents the helper's equality contract and guards against
regressions in the path+branch comparison. Eight cases:

- two empty maps
- identical entries (path and branch match in order)
- same path, different branch — the F1 regression case
- different paths at the same index
- per-project array length mismatch
- project-key count mismatch
- positional reorder (helper is order-sensitive)
- non-first-entry branch difference (subset detection)

All 10 tests in the file pass (2 existing + 8 new).

* ci: retrigger checks

* test(worktree): add benchmark for worktreeMapsEqual and persist path

Documents the actual cost of the PR #1992 optimizations on representative
sizes (1-1000 worktrees per project, 1-50 projects), so future contributors
can reproduce the numbers and detect regressions in the equality helper or
the persist subscriber.

Run with: `bun run packages/ui/src/lib/worktrees/worktreeManager.bench.ts`

Measured on V8 (one example run):
- worktreeMapsEqual early-exit (50×20 with first project differing):
  412 ns/op vs 33,034 ns/op full sweep — ~80x speedup when any project
  actually changed.
- F1 path+branch overhead vs path-only (10×50): +2.3 µs (+15.8%) on a
  full sweep; on the early-exit path the F1 cost is irrelevant.
- Stringify dedup in persistWorktreeMap subscriber: 67% saved (552 µs
  per persist on 10×50). This is the main absolute win of the PR.
- Content-compare guard: 19-29 ns/op, free relative to the stringify it
  gates.

Bench file is standalone (import.meta.main guard) — does not run as part
of `bun test`, does not import React, does not touch localStorage.

---------

Co-authored-by: bashrusakh <bashrusakh@users.noreply.github.com>
This commit is contained in:
Leonid
2026-07-11 16:06:58 +03:00
committed by GitHub
co-authored by bashrusakh
parent 76c3bb5fd9
commit 6d7ea82d86
7 changed files with 381 additions and 21 deletions
+10 -5
View File
@@ -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();
+11 -5
View File
@@ -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();
@@ -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<SessionSidebarProps> = ({
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.
@@ -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<string, WorktreeMetadata[]> => {
const map = new Map<string, WorktreeMetadata[]>();
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 = <T extends { path: string }>(
a: Map<string, T[]>,
b: Map<string, T[]>,
): 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<void> {
// 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();
}
@@ -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<void> => {
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> = {},
): WorktreeMetadata => ({
path,
branch,
projectDirectory: '/repo',
label: branch,
...overrides,
});
test('returns true for two empty maps', () => {
const a = new Map<string, WorktreeMetadata[]>();
const b = new Map<string, WorktreeMetadata[]>();
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<string, WorktreeMetadata[]>([['/repo', [wt('/r/main', 'main')]]]);
const b = new Map<string, WorktreeMetadata[]>([
['/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);
});
});
@@ -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 = <T extends { path: string; branch: string }>(
a: Map<string, T[]>,
b: Map<string, T[]>,
): 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<string, { value: WorktreeMetadata[]; at: number }>();
const _worktreeListInflight = new Map<string, Promise<WorktreeMetadata[]>>();
+12 -5
View File
@@ -538,9 +538,9 @@ const loadPersistedWorktreeMap = (): Map<string, WorktreeMetadata[]> => {
}
}
const persistWorktreeMap = (map: Map<string, WorktreeMetadata[]>): 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)
}
}
})