perf(worktrees): bound the duplicate-id scan in reuseEqualCatalogRows (#14271)

* perf(worktrees): bound the duplicate-id scan in reuseEqualCatalogRows

Rows sharing an id are scanned linearly with a deep compare each, so a bucket
of k duplicates costs O(k^2). Both callers key on ids that are unique by
construction, so this is a bound on damage rather than a live fix.

Cap the scan instead of adding a second index: reuse is only an optimization,
so a missed match yields a new object identity, never a wrong row. A fingerprint
index would buy a little more reuse in a case nothing reaches, at the cost of a
second equality implementation that must stay in step with catalogValuesEqual
with no automated guard.

Worst-case duplicate bucket, no matches: k=1000 134ms -> 1.2ms, k=2000 541ms ->
2.2ms. Unique-id path unchanged (2000 rows: 0.86ms both).

* docs(worktrees): lead the duplicate-id cap comment with its reachability

A reader hitting MAX_DUPLICATE_ID_SCAN should learn first that no caller
produces duplicate ids today, so the cap reads as bounding future damage rather
than fixing something live.
This commit is contained in:
Neil
2026-08-13 18:14:25 -07:00
committed by GitHub
parent eb22e497bb
commit 517a2b3da6
2 changed files with 60 additions and 4 deletions
@@ -71,4 +71,44 @@ describe('reuseEqualCatalogRows', () => {
expect(reconciled[0]).toBe(current[1])
expect(reconciled[1]).toBe(current[0])
})
it('reuses a match inside the duplicate-id scan window', () => {
const current = [
{ id: 'dup', marker: 'a' },
{ id: 'dup', marker: 'b' },
{ id: 'dup', marker: 'c' }
]
const reconciled = reuseEqualCatalogRows(current, [{ id: 'dup', marker: 'c' }])
expect(reconciled[0]).toBe(current[2])
})
// 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', () => {
const bucketSize = 64
const current = Array.from({ length: bucketSize }, (_, index) => ({
id: 'dup',
marker: `previous-${index}`
}))
let reads = 0
const incoming = [
{
id: 'dup',
get marker(): string {
reads++
return 'matches-nothing'
}
}
]
const reconciled = reuseEqualCatalogRows(current, incoming)
// 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)
})
})
@@ -30,6 +30,14 @@ function catalogValuesEqual(left: unknown, right: unknown): boolean {
return true
}
// 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 a second index that would have to stay in
// step with catalogValuesEqual, and reuse is only an optimization: dropping a
// match past the window costs object identity, never correctness.
const MAX_DUPLICATE_ID_SCAN = 8
export function reuseEqualCatalogRows<T extends CatalogRow>(
current: readonly T[] | undefined,
incoming: readonly T[]
@@ -48,10 +56,18 @@ export function reuseEqualCatalogRows<T extends CatalogRow>(
}
const reconciled = incoming.map((row) => {
const candidates = currentById.get(row.id)
const previousIndex = candidates?.findIndex((candidate) => catalogValuesEqual(candidate, row))
return previousIndex !== undefined && previousIndex >= 0
? candidates!.splice(previousIndex, 1)[0]
: row
if (!candidates) {
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 && catalogValuesEqual(candidate, row)) {
candidates.splice(index, 1)
return candidate
}
}
return row
})
return current.length === reconciled.length &&
current.every((row, index) => row === reconciled[index])