diff --git a/.changeset/quick-trees-rest.md b/.changeset/quick-trees-rest.md new file mode 100644 index 0000000000..e6a78e8d42 --- /dev/null +++ b/.changeset/quick-trees-rest.md @@ -0,0 +1,7 @@ +--- +'@tanstack/db': patch +--- + +Skip unchanged index writes while preserving index bookkeeping after failed +removals. Cache index evaluators and avoid object normalization work for +primitive values to reduce update overhead. diff --git a/packages/db/src/indexes/base-index.ts b/packages/db/src/indexes/base-index.ts index 945221e6fa..26cb09887b 100644 --- a/packages/db/src/indexes/base-index.ts +++ b/packages/db/src/indexes/base-index.ts @@ -1,6 +1,7 @@ import { compileSingleRowExpression } from '../query/compiler/evaluators.js' import { comparisonFunctions } from '../query/builder/functions.js' import { DEFAULT_COMPARE_OPTIONS, deepEquals } from '../utils.js' +import type { CompiledSingleRowExpression } from '../query/compiler/evaluators.js' import type { RangeQueryOptions } from './btree-index.js' import type { CompareOptions } from '../query/builder/types.js' import type { BasicExpression, OrderByDirection } from '../query/ir.js' @@ -99,6 +100,7 @@ export abstract class BaseIndex< protected totalLookupTime = 0 protected lastUpdated = new Date() protected compareOptions: CompareOptions + private compiledIndexEvaluator: CompiledSingleRowExpression | undefined /** * Set by subclasses when constructed with a user-supplied comparator, whose * ordering may not match the WHERE evaluator's relational operators. @@ -210,7 +212,8 @@ export abstract class BaseIndex< protected abstract initialize(options?: any): void protected evaluateIndexExpression(item: any): any { - const evaluator = compileSingleRowExpression(this.expression) + const evaluator = (this.compiledIndexEvaluator ??= + compileSingleRowExpression(this.expression)) return evaluator(item as Record) } diff --git a/packages/db/src/indexes/basic-index.ts b/packages/db/src/indexes/basic-index.ts index b9b06d1925..8eac6f926a 100644 --- a/packages/db/src/indexes/basic-index.ts +++ b/packages/db/src/indexes/basic-index.ts @@ -1,4 +1,8 @@ -import { defaultComparator, normalizeValue } from '../utils/comparison.js' +import { + areSameValueZeroEqual, + defaultComparator, + normalizeValue, +} from '../utils/comparison.js' import { deleteInSortedArray, findInsertPositionInArray, @@ -89,9 +93,17 @@ export class BasicIndex< const normalizedValue = normalizeValue(indexedValue) - if (this.valueMap.has(normalizedValue)) { + this.addToBucket(key, normalizedValue) + + this.indexedKeys.add(key) + this.updateTimestamp() + } + + private addToBucket(key: TKey, normalizedValue: unknown): void { + const keySet = this.valueMap.get(normalizedValue) + if (keySet) { // Value already exists, just add the key to the set - this.valueMap.get(normalizedValue)!.add(key) + keySet.add(key) } else { // New value - add to map and insert into sorted array this.valueMap.set(normalizedValue, new Set([key])) @@ -104,9 +116,6 @@ export class BasicIndex< ) this.sortedValues.splice(insertIdx, 0, normalizedValue) } - - this.indexedKeys.add(key) - this.updateTimestamp() } /** @@ -128,8 +137,15 @@ export class BasicIndex< const normalizedValue = normalizeValue(indexedValue) - if (this.valueMap.has(normalizedValue)) { - const keySet = this.valueMap.get(normalizedValue)! + this.removeFromBucket(key, normalizedValue) + + this.indexedKeys.delete(key) + this.updateTimestamp() + } + + private removeFromBucket(key: TKey, normalizedValue: unknown): void { + const keySet = this.valueMap.get(normalizedValue) + if (keySet) { keySet.delete(key) if (keySet.size === 0) { @@ -138,17 +154,35 @@ export class BasicIndex< deleteInSortedArray(this.sortedValues, normalizedValue, this.compareFn) } } - - this.indexedKeys.delete(key) - this.updateTimestamp() } /** * Updates a value in the index */ update(key: TKey, oldItem: any, newItem: any): void { - this.remove(key, oldItem) - this.add(key, newItem) + let oldValue: unknown + let newValue: unknown + try { + oldValue = normalizeValue(this.evaluateIndexExpression(oldItem)) + newValue = normalizeValue(this.evaluateIndexExpression(newItem)) + } catch { + this.remove(key, oldItem) + this.add(key, newItem) + return + } + + if ( + areSameValueZeroEqual(oldValue, newValue) && + this.valueMap.get(newValue)?.has(key) && + this.indexedKeys.has(key) + ) { + return + } + + this.removeFromBucket(key, oldValue) + this.addToBucket(key, newValue) + this.indexedKeys.add(key) + this.updateTimestamp() } /** diff --git a/packages/db/src/indexes/btree-index.ts b/packages/db/src/indexes/btree-index.ts index 8b92095f01..6379b91b52 100644 --- a/packages/db/src/indexes/btree-index.ts +++ b/packages/db/src/indexes/btree-index.ts @@ -1,6 +1,7 @@ import { compareKeys } from '@tanstack/db-ivm' import { BTree } from '../utils/btree.js' import { + areSameValueZeroEqual, defaultComparator, denormalizeUndefined, normalizeForBTree, @@ -94,19 +95,23 @@ export class BTreeIndex< // Normalize the value for Map key usage const normalizedValue = normalizeForBTree(indexedValue) - // Check if this value already exists - if (this.valueMap.has(normalizedValue)) { + this.addToBucket(key, normalizedValue) + + this.indexedKeys.add(key) + this.updateTimestamp() + } + + private addToBucket(key: TKey, normalizedValue: unknown): void { + const keySet = this.valueMap.get(normalizedValue) + if (keySet) { // Add to existing set - this.valueMap.get(normalizedValue)!.add(key) + keySet.add(key) } else { // Create new set for this value - const keySet = new Set([key]) - this.valueMap.set(normalizedValue, keySet) + const newKeySet = new Set([key]) + this.valueMap.set(normalizedValue, newKeySet) this.orderedEntries.set(normalizedValue, undefined) } - - this.indexedKeys.add(key) - this.updateTimestamp() } /** @@ -127,8 +132,15 @@ export class BTreeIndex< // Normalize the value for Map key usage const normalizedValue = normalizeForBTree(indexedValue) - if (this.valueMap.has(normalizedValue)) { - const keySet = this.valueMap.get(normalizedValue)! + this.removeFromBucket(key, normalizedValue) + + this.indexedKeys.delete(key) + this.updateTimestamp() + } + + private removeFromBucket(key: TKey, normalizedValue: unknown): void { + const keySet = this.valueMap.get(normalizedValue) + if (keySet) { keySet.delete(key) // If set is now empty, remove the entry entirely @@ -139,17 +151,34 @@ export class BTreeIndex< this.orderedEntries.delete(normalizedValue) } } - - this.indexedKeys.delete(key) - this.updateTimestamp() } /** * Updates a value in the index */ update(key: TKey, oldItem: any, newItem: any): void { - this.remove(key, oldItem) - this.add(key, newItem) + let oldValue: unknown + let newValue: unknown + try { + oldValue = normalizeForBTree(this.evaluateIndexExpression(oldItem)) + newValue = normalizeForBTree(this.evaluateIndexExpression(newItem)) + } catch { + this.remove(key, oldItem) + this.add(key, newItem) + return + } + + if ( + areSameValueZeroEqual(oldValue, newValue) && + this.valueMap.get(newValue)?.has(key) + ) { + return + } + + this.removeFromBucket(key, oldValue) + this.addToBucket(key, newValue) + this.indexedKeys.add(key) + this.updateTimestamp() } /** diff --git a/packages/db/src/utils/comparison.ts b/packages/db/src/utils/comparison.ts index 992f0098c5..8ad6c4f3f6 100644 --- a/packages/db/src/utils/comparison.ts +++ b/packages/db/src/utils/comparison.ts @@ -185,6 +185,10 @@ export const UNDEFINED_SENTINEL = `__TS_DB_BTREE_UNDEFINED_VALUE__` * for BTree index operations that need to distinguish undefined values. */ export function normalizeValue(value: any): any { + if (typeof value !== `object` || value === null) { + return value + } + if (value instanceof Date) { return value.getTime() } @@ -226,6 +230,13 @@ export function normalizeForBTree(value: any): any { return normalizeValue(value) } +/** + * Compare values using the equality semantics used by Map keys. + */ +export function areSameValueZeroEqual(a: unknown, b: unknown): boolean { + return a === b || (Number.isNaN(a) && Number.isNaN(b)) +} + /** * Converts the `UNDEFINED_SENTINEL` back to `undefined`. * Needed such that the sentinel is converted back to `undefined` before comparison. diff --git a/packages/db/tests/index-update-short-circuit.test.ts b/packages/db/tests/index-update-short-circuit.test.ts new file mode 100644 index 0000000000..94a990b6aa --- /dev/null +++ b/packages/db/tests/index-update-short-circuit.test.ts @@ -0,0 +1,133 @@ +import { describe, expect, it, vi } from 'vitest' +import { BasicIndex } from '../src/indexes/basic-index.js' +import { BTreeIndex } from '../src/indexes/btree-index.js' +import { PropRef } from '../src/query/ir.js' +import { normalizeValue } from '../src/utils/comparison.js' +import type { BaseIndex } from '../src/indexes/base-index.js' + +type IndexConstructor = new ( + id: number, + expression: PropRef, + name?: string, + options?: unknown, +) => BaseIndex & { + valueMapData: Map> +} + +const indexTypes: Array<[string, IndexConstructor]> = [ + [`BasicIndex`, BasicIndex as IndexConstructor], + [`BTreeIndex`, BTreeIndex as IndexConstructor], +] + +describe.each(indexTypes)(`%s update`, (_indexName, IndexType) => { + function createIndex(options?: unknown) { + return new IndexType(1, new PropRef([`value`]), `test_index`, options) + } + + it(`keeps the existing bucket when the indexed value does not change`, () => { + const index = createIndex() + index.add(`a`, { value: 1, version: 1 }) + const bucket = index.valueMapData.get(1) + const lastUpdated = index.getStats().lastUpdated + + index.update(`a`, { value: 1, version: 1 }, { value: 1, version: 2 }) + + expect(index.valueMapData.get(1)).toBe(bucket) + expect(index.getStats().lastUpdated).toBe(lastUpdated) + expect(index.lookup(`eq`, 1)).toEqual(new Set([`a`])) + }) + + it.each([ + [`undefined`, undefined, undefined], + [`NaN`, Number.NaN, Number.NaN], + [`signed zero`, 0, -0], + [`equal dates`, new Date(1000), new Date(1000)], + [`equal byte arrays`, new Uint8Array([1, 2]), new Uint8Array([1, 2])], + ])(`keeps the existing bucket for %s`, (_caseName, oldValue, newValue) => { + const index = createIndex() + index.add(`a`, { value: oldValue }) + const bucket = index.valueMapData.get(normalizeValue(oldValue)) + expect(bucket).toBeDefined() + + index.update(`a`, { value: oldValue }, { value: newValue }) + + expect(index.valueMapData.get(normalizeValue(newValue))).toBe(bucket) + }) + + it(`moves the key when the indexed value changes`, () => { + const index = createIndex() + index.add(`a`, { value: 1 }) + + index.update(`a`, { value: 1 }, { value: 2 }) + + expect(index.lookup(`eq`, 1)).toEqual(new Set()) + expect(index.lookup(`eq`, 2)).toEqual(new Set([`a`])) + }) + + it(`does not conflate undefined and null`, () => { + const index = createIndex() + index.add(`a`, { value: undefined }) + + index.update(`a`, { value: undefined }, { value: null }) + + expect(index.lookup(`eq`, undefined)).toEqual(new Set()) + expect(index.lookup(`eq`, null)).toEqual(new Set([`a`])) + }) + + it(`does not use comparator equality to skip an update`, () => { + const index = createIndex({ + compareFn: (a: string, b: string) => + a.toLowerCase().localeCompare(b.toLowerCase()), + }) + index.add(`a`, { value: `A` }) + + index.update(`a`, { value: `A` }, { value: `a` }) + + expect(index.lookup(`eq`, `A`)).toEqual(new Set()) + expect(index.lookup(`eq`, `a`)).toEqual(new Set([`a`])) + }) + + it(`preserves the previous error behavior when evaluation fails`, () => { + const index = createIndex() + index.add(`a`, { value: 1 }) + const newItem = Object.defineProperty({}, `value`, { + get() { + throw new Error(`evaluation failed`) + }, + }) + + expect(() => index.update(`a`, { value: 1 }, newItem)).toThrow( + `evaluation failed`, + ) + + expect(index.lookup(`eq`, 1)).toEqual(new Set()) + expect(index.keyCount).toBe(0) + }) +}) + +describe(`BasicIndex update bookkeeping`, () => { + it(`repairs indexed key membership after a failed removal`, () => { + const index = new BasicIndex(1, new PropRef([`value`])) + index.add(`a`, { value: 1 }) + const itemWithThrowingValue = Object.defineProperty({}, `value`, { + get() { + throw new Error(`evaluation failed`) + }, + }) + const warn = vi.spyOn(console, `warn`).mockImplementation(() => {}) + + try { + index.remove(`a`, itemWithThrowingValue) + } finally { + warn.mockRestore() + } + + expect(index.lookup(`eq`, 1)).toEqual(new Set([`a`])) + expect(index.keyCount).toBe(0) + + index.update(`a`, { value: 1 }, { value: 1 }) + + expect(index.lookup(`eq`, 1)).toEqual(new Set([`a`])) + expect(index.keyCount).toBe(1) + }) +}) diff --git a/packages/db/tests/index-update.property.test.ts b/packages/db/tests/index-update.property.test.ts new file mode 100644 index 0000000000..2099c77906 --- /dev/null +++ b/packages/db/tests/index-update.property.test.ts @@ -0,0 +1,126 @@ +import { describe, expect } from 'vitest' +import { fc, test as fcTest } from '@fast-check/vitest' +import { BasicIndex } from '../src/indexes/basic-index.js' +import { BTreeIndex } from '../src/indexes/btree-index.js' +import { PropRef } from '../src/query/ir.js' +import type { BaseIndex } from '../src/indexes/base-index.js' + +type IndexValue = number + +type IndexConstructor = new ( + id: number, + expression: PropRef, +) => BaseIndex + +type IndexAction = + | { type: `put`; key: string; value: IndexValue } + | { type: `delete`; key: string } + +const indexTypes: Array<[string, IndexConstructor]> = [ + [`BasicIndex`, BasicIndex as IndexConstructor], + [`BTreeIndex`, BTreeIndex as IndexConstructor], +] + +const arbitraryValue: fc.Arbitrary = fc.integer({ + min: -3, + max: 3, +}) + +const arbitraryAction: fc.Arbitrary = fc.oneof( + fc.record({ + type: fc.constant(`put` as const), + key: fc.integer({ min: 0, max: 7 }).map(String), + value: arbitraryValue, + }), + fc.record({ + type: fc.constant(`delete` as const), + key: fc.integer({ min: 0, max: 7 }).map(String), + }), +) + +const probeValues: Array = [-3, -2, -1, -0, 0, 1, 2, 3, 99] +const rangeBoundaries: Array = [-2, 0, 2] + +function groupKeysByValue( + rows: Map, +): Map> { + const groups = new Map>() + for (const [key, value] of rows) { + const keys = groups.get(value) + if (keys) { + keys.add(key) + } else { + groups.set(value, new Set([key])) + } + } + return groups +} + +function expectIndexMatchesModel( + index: BaseIndex, + rows: Map, +): void { + const groups = groupKeysByValue(rows) + + expect(index.keyCount).toBe(rows.size) + expect(index.indexedKeysSet).toEqual(new Set(rows.keys())) + expect(index.valueMapData).toEqual(groups) + expect(index.orderedEntriesArray).toEqual( + [...groups].sort(([left], [right]) => left - right), + ) + + for (const value of probeValues) { + expect(index.lookup(`eq`, value)).toEqual(groups.get(value) ?? new Set()) + } + + for (const boundary of rangeBoundaries) { + const keysAtOrAbove = new Set( + [...rows].filter(([, value]) => value >= boundary).map(([key]) => key), + ) + const keysAtOrBelow = new Set( + [...rows].filter(([, value]) => value <= boundary).map(([key]) => key), + ) + + expect(index.rangeQuery({ from: boundary })).toEqual(keysAtOrAbove) + expect(index.rangeQuery({ to: boundary })).toEqual(keysAtOrBelow) + } +} + +describe.each(indexTypes)(`%s update properties`, (_indexName, IndexType) => { + fcTest.prop([ + fc.array(arbitraryAction, { + minLength: 1, + maxLength: 100, + }), + ])( + `matches a reference model across valid operation sequences`, + (actions) => { + const index = new IndexType(1, new PropRef([`value`])) + const rows = new Map() + + for (const action of actions) { + if (action.type === `put`) { + if (rows.has(action.key)) { + index.update( + action.key, + { value: rows.get(action.key) }, + { value: action.value }, + ) + } else { + index.add(action.key, { value: action.value }) + } + rows.set(action.key, action.value) + } else if (rows.has(action.key)) { + index.remove(action.key, { value: rows.get(action.key) }) + rows.delete(action.key) + } + + expectIndexMatchesModel(index, rows) + } + + const rebuilt = new IndexType(2, new PropRef([`value`])) + rebuilt.build([...rows].map(([key, value]) => [key, { value }] as const)) + expectIndexMatchesModel(rebuilt, rows) + }, + ) +})