diff --git a/src/renderer/src/store/slices/worktree-catalog-reconciliation.test.ts b/src/renderer/src/store/slices/worktree-catalog-reconciliation.test.ts index 474dc8dfb39..dbea6eebb0a 100644 --- a/src/renderer/src/store/slices/worktree-catalog-reconciliation.test.ts +++ b/src/renderer/src/store/slices/worktree-catalog-reconciliation.test.ts @@ -87,7 +87,88 @@ describe('reuseEqualCatalogRows', () => { // Without the cap this walks the whole bucket, so a 64-row bucket costs 64 // deep compares per incoming row. Counting reads keeps the guard deterministic // — a wall-clock assertion would be flaky on shared CI runners. - it('caps the deep compares for one id instead of scanning the whole bucket', () => { + // The cap this replaced gave up matches past its window. The index does not: + // a match at the far end of a large bucket is reused again. + it('reuses a match at the far end of a large duplicate-id bucket', () => { + const bucketSize = 64 + const current = Array.from({ length: bucketSize }, (_, index) => ({ + id: 'dup', + marker: `previous-${index}` + })) + + const reconciled = reuseEqualCatalogRows(current, [{ id: 'dup', marker: 'previous-63' }]) + + expect(reconciled[0]).toBe(current[63]) + }) + + it('consumes each previous row at most once across a large bucket', () => { + const current = Array.from({ length: 32 }, () => ({ id: 'dup', marker: 'same' })) + + const reconciled = reuseEqualCatalogRows(current, [ + { id: 'dup', marker: 'same' }, + { id: 'dup', marker: 'same' } + ]) + + expect(reconciled[0]).toBe(current[0]) + expect(reconciled[1]).toBe(current[1]) + expect(reconciled[0]).not.toBe(reconciled[1]) + }) + + // JSON collapses distinctions the equality walk keeps, so a fingerprint group + // can hold rows that are not actually equal. Rejecting one must not consume it. + it('keeps a rejected same-fingerprint candidate available to later rows', () => { + const instants = Array.from({ length: 16 }, () => new Date(0)) + const current = instants.map((at) => ({ id: 'dup', at })) + + const reconciled = reuseEqualCatalogRows(current, [ + { id: 'dup', at: instants[1] as Date }, + { id: 'dup', at: instants[0] as Date } + ]) + + // Distinct Date objects share a fingerprint but are not structurally equal, + // so resolving the first row must leave the second row's match in place. + expect(reconciled[0]).toBe(current[1]) + expect(reconciled[1]).toBe(current[0]) + }) + + // Leaving rejected candidates in place is only affordable because the group + // scan is itself capped, and nothing else in this suite fails if that cap is + // dropped. Assert the confirms do not SCALE with the group — an absolute bound + // would pin the threshold constant instead, which is a tuning knob. + it('bounds the confirms inside an all-colliding fingerprint group', () => { + const confirmsFor = (groupSize: number): number => { + // One fingerprint, no two rows structurally equal: distinct Date objects. + const current = Array.from({ length: groupSize }, () => ({ id: 'dup', at: new Date(0) })) + let reads = 0 + const incoming = [ + { + id: 'dup', + get at(): Date { + reads++ + return new Date(0) + } + } + ] + + expect(reuseEqualCatalogRows(current as never, incoming as never)[0]).toBe(incoming[0]) + return reads + } + + expect(confirmsFor(400)).toBe(confirmsFor(50)) + }) + + it('still matches when a large bucket cannot be fingerprinted', () => { + // JSON.stringify throws on BigInt, so the index cannot be built and this + // falls back to the capped scan rather than crashing or losing the match. + const row = (marker: string): Record => ({ id: 'dup', marker, size: 1n }) + const current = Array.from({ length: 16 }, (_, index) => row(`previous-${index}`)) + + const reconciled = reuseEqualCatalogRows(current as never, [row('previous-0')] as never) + + expect(reconciled[0]).toBe(current[0]) + }) + + it('keeps the deep compares off the whole bucket', () => { const bucketSize = 64 const current = Array.from({ length: bucketSize }, (_, index) => ({ id: 'dup', @@ -108,7 +189,6 @@ describe('reuseEqualCatalogRows', () => { // No match, so the incoming row is kept — a missed reuse costs identity, never correctness. expect(reconciled[0]).toBe(incoming[0]) - expect(reads).toBeLessThanOrEqual(8) expect(reads).toBeLessThan(bucketSize) }) }) diff --git a/src/renderer/src/store/slices/worktree-catalog-reconciliation.ts b/src/renderer/src/store/slices/worktree-catalog-reconciliation.ts index 500f002dc59..c49bf807fc9 100644 --- a/src/renderer/src/store/slices/worktree-catalog-reconciliation.ts +++ b/src/renderer/src/store/slices/worktree-catalog-reconciliation.ts @@ -2,14 +2,14 @@ import { structuralValuesEqualIgnoringUndefined } from '../../../../shared/struc type CatalogRow = { id: string } -// NOTHING HITS THIS TODAY: both callers key on ids that are unique by -// construction, so buckets stay at 1-3. It only bounds the damage if that ever -// changes — a same-id bucket costs one deep compare per candidate, so an -// unbounded one is O(k^2). A cap beats an index keyed on a second walk that -// would have to stay in step with structuralValuesEqualIgnoringUndefined, and -// reuse is only an optimization: dropping a match past the window costs object -// identity, never correctness. -const MAX_DUPLICATE_ID_SCAN = 8 +// Above this many rows sharing an id, matching switches from a linear scan to +// a fingerprint index: scanning costs one deep compare per candidate, so an +// unbounded bucket would be O(k^2). Below it the scan is cheaper than building +// an index, and that is where every live caller sits — but not because ids are +// globally unique. areWorktreesEqual compares unfiltered cross-host lists in +// which one worktree path legitimately repeats, so a bucket is bounded by the +// hosts sharing that path, which is why it stays far below the threshold. +const DUPLICATE_ID_INDEX_THRESHOLD = 8 export function reuseEqualCatalogRows( current: readonly T[] | undefined, @@ -27,20 +27,42 @@ export function reuseEqualCatalogRows( currentById.set(row.id, [row]) } } + // Stays null unless some id actually exceeds the threshold, so the live path + // allocates exactly what it did before — one array per id and nothing else. + let indexesById: Map | null> | null = null const reconciled = incoming.map((row) => { const candidates = currentById.get(row.id) - if (!candidates) { + if (candidates === undefined) { return row } - const scanLimit = Math.min(candidates.length, MAX_DUPLICATE_ID_SCAN) - for (let index = 0; index < scanLimit; index++) { - const candidate = candidates[index] - if (candidate !== undefined && structuralValuesEqualIgnoringUndefined(candidate, row)) { - candidates.splice(index, 1) - return candidate - } + const existing = indexesById?.get(row.id) + // A bucket only shrinks, so once it is under the threshold with no index it + // stays on the scan, and the two strategies never both consume from it. + if (existing === undefined && candidates.length <= DUPLICATE_ID_INDEX_THRESHOLD) { + return spliceEqualRow(candidates, row, candidates.length) ?? row } - return row + let index = existing + if (index === undefined) { + index = buildFingerprintIndex(candidates) + indexesById ??= new Map() + indexesById.set(row.id, index) + } + if (index === null) { + // JSON.stringify rejects some values the equality walk handles fine + // (BigInt); those fall back to a capped scan so O(k^2) cannot return here. + return spliceEqualRow(candidates, row, DUPLICATE_ID_INDEX_THRESHOLD) ?? row + } + // Confirm inside the fingerprint group WITHOUT discarding non-matches: JSON + // collapses distinctions the walk keeps (two Dates at one instant, -0 vs 0, + // undefined vs null inside an array), and a rejected candidate + // dropped here would be invisible to every later row. Bounded so an + // all-colliding group cannot bring O(k^2) back. + const matches = index.get(catalogRowFingerprint(row) ?? '') + // Consumes at most one previous row, so two duplicates cannot both reuse it. + return ( + (matches === undefined ? null : spliceEqualRow(matches, row, DUPLICATE_ID_INDEX_THRESHOLD)) ?? + row + ) }) return current.length === reconciled.length && current.every((row, index) => row === reconciled[index]) @@ -48,6 +70,59 @@ export function reuseEqualCatalogRows( : reconciled } +function spliceEqualRow(rows: T[], row: T, scanLimit: number): T | null { + const limit = Math.min(rows.length, scanLimit) + for (let index = 0; index < limit; index++) { + const candidate = rows[index] + if (candidate !== undefined && structuralValuesEqualIgnoringUndefined(candidate, row)) { + rows.splice(index, 1) + return candidate + } + } + return null +} + +function buildFingerprintIndex(rows: readonly T[]): Map | null { + const index = new Map() + for (const row of rows) { + const key = catalogRowFingerprint(row) + if (key === null) { + return null + } + const existing = index.get(key) + if (existing) { + existing.push(row) + } else { + index.set(key, [row]) + } + } + return index +} + +// Only a pre-filter — every hit is confirmed with the real equality walk before +// it is reused. That is what stops this being a second equality implementation +// to hold in step with structuralValuesEqualIgnoringUndefined: if the two ever +// disagree the cost is a missed reuse (one fresh object identity), never a wrong +// row. Keys are sorted because property order is not significant, and JSON drops +// undefined-valued keys, which is the ignoring-undefined semantics. +function catalogRowFingerprint(row: unknown): string | null { + try { + return ( + JSON.stringify(row, (_key, value: unknown) => + value !== null && typeof value === 'object' && !Array.isArray(value) + ? Object.fromEntries( + Object.entries(value as Record).sort(([left], [right]) => + left < right ? -1 : left > right ? 1 : 0 + ) + ) + : value + ) ?? 'undefined' + ) + } catch { + return null + } +} + export function catalogRowsEqual( current: readonly T[] | undefined, incoming: readonly T[]