mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
perf(store): match duplicate catalog ids through a fingerprint index (#14452)
* perf(store): match duplicate catalog ids through a fingerprint index The cap bounded the O(k^2) scan by giving up: past the eighth candidate it stopped looking, so a large bucket reused 8 rows instead of all of them. Index the bucket by a canonical JSON fingerprint instead and confirm every hit with the real equality walk, so a fingerprint that disagrees with it costs a missed reuse, never a wrong row — which is what stops it being a second equality implementation to hold in step with structuralValuesEqualIgnoringUndefined. Buckets at or below the threshold keep the plain scan, and the index map is only allocated once some id exceeds it, so the live path (ids unique by construction, buckets of 1) allocates exactly what it did before. Values JSON rejects but the walk accepts, such as BigInt, fall back to the capped scan. Reversed 1000-row bucket: 8 rows reused -> 1000. Live path unchanged (4000 rows, median of 9 trials: 2.014ms -> 1.907ms). * fix(store): stop the fingerprint index from consuming rejected candidates JSON collapses distinctions the equality walk keeps — two Dates at one instant, -0 vs 0, undefined vs null inside an array, NaN vs null — so a fingerprint group can hold rows that are not structurally equal. Shifting a candidate out to test it discarded it on rejection, making it invisible to every later incoming row. Confirm within the group without removing non-matches, splicing out only the row that actually matches. Still bounded by the same threshold, so an all-colliding group cannot bring O(k^2) back. * test(store): pin the cap on the fingerprint-group scan Leaving rejected candidates in place is only affordable because the group scan is itself capped, and nothing in the suite failed when that cap was removed. Count reads against a 200-row all-colliding group: one to fingerprint the incoming row plus at most the threshold in confirms. Dropping the cap makes this 201 instead of 9. * test(store): assert the group-scan cap by size invariance, not an absolute reads <= 9 pinned the value of DUPLICATE_ID_INDEX_THRESHOLD, not the existence of the cap: retuning it 8 -> 16 failed the test though nothing behavioral or asymptotic changed. Compare the confirm count at two group sizes instead, which still fails when the cap is removed (401 vs 51) and survives the retune. The neighbouring test had the same shape and is reshaped with it. Also correct the threshold comment: ids are not globally unique. areWorktreesEqual compares unfiltered cross-host lists where one worktree path repeats, so buckets are bounded by the hosts sharing that path — which the suite already pins.
This commit is contained in:
@@ -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<string, unknown> => ({ 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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<T extends CatalogRow>(
|
||||
current: readonly T[] | undefined,
|
||||
@@ -27,20 +27,42 @@ export function reuseEqualCatalogRows<T extends CatalogRow>(
|
||||
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<string, Map<string, T[]> | 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<T extends CatalogRow>(
|
||||
: reconciled
|
||||
}
|
||||
|
||||
function spliceEqualRow<T>(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<T>(rows: readonly T[]): Map<string, T[]> | null {
|
||||
const index = new Map<string, T[]>()
|
||||
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<string, unknown>).sort(([left], [right]) =>
|
||||
left < right ? -1 : left > right ? 1 : 0
|
||||
)
|
||||
)
|
||||
: value
|
||||
) ?? 'undefined'
|
||||
)
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
export function catalogRowsEqual<T extends CatalogRow>(
|
||||
current: readonly T[] | undefined,
|
||||
incoming: readonly T[]
|
||||
|
||||
Reference in New Issue
Block a user