From 09543d4dc7a81956fbceb921c4e1d1e46b76914e Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Thu, 2 Jul 2026 20:41:50 -0500 Subject: [PATCH] feat: better faceting performance --- nx.json | 10 +- package.json | 1 + .../columnFacetingFeature.types.ts | 9 +- .../columnFacetingFeature.utils.ts | 71 ++++++-- .../column-faceting/createFacetedRowModel.ts | 4 + .../createFacetedUniqueValues.ts | 13 +- .../columnFacetingFeature.test.ts | 152 ++++++++++++++++ perf-done.md | 130 +++++++++++++- perf-todo.md | 167 ++---------------- 9 files changed, 367 insertions(+), 190 deletions(-) create mode 100644 packages/table-core/tests/unit/features/column-faceting/columnFacetingFeature.test.ts diff --git a/nx.json b/nx.json index 3446cbf064..015165eca4 100644 --- a/nx.json +++ b/nx.json @@ -29,23 +29,23 @@ }, "targetDefaults": { "test:lib": { - "dependsOn": ["^build"], + "dependsOn": ["^build", "build"], "inputs": ["default", "^public"], "outputs": ["{projectRoot}/coverage"], "cache": true }, "test:eslint": { - "dependsOn": ["^build"], + "dependsOn": ["^build", "build"], "inputs": ["default", "^public"], "cache": true }, "test:types": { - "dependsOn": ["^build"], + "dependsOn": ["^build", "build"], "inputs": ["default", "^public"], "cache": true }, "test:e2e": { - "dependsOn": ["^build"], + "dependsOn": ["^build", "build"], "inputs": ["default", "^public"], "cache": true }, @@ -55,7 +55,7 @@ "cache": true }, "build": { - "dependsOn": ["^build"], + "dependsOn": ["^build", { "target": "format", "projects": "table" }], "inputs": ["default", "^public"], "outputs": ["{projectRoot}/build", "{projectRoot}/dist"], "cache": true diff --git a/package.json b/package.json index f321e794be..7481c5aa38 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ }, "nx": { "includedScripts": [ + "format", "test:docs", "test:knip", "test:sherif" diff --git a/packages/table-core/src/features/column-faceting/columnFacetingFeature.types.ts b/packages/table-core/src/features/column-faceting/columnFacetingFeature.types.ts index b7ff8c96de..7489e0d794 100644 --- a/packages/table-core/src/features/column-faceting/columnFacetingFeature.types.ts +++ b/packages/table-core/src/features/column-faceting/columnFacetingFeature.types.ts @@ -52,9 +52,12 @@ export interface CachedRowModel_Faceted< in out TFeatures extends TableFeatures, in out TData extends RowData, > { - facetedRowModel?: (columnId: string) => () => RowModel - facetedMinMaxValues?: (columnId: string) => [number, number] - facetedUniqueValues?: (columnId: string) => Map + facetedRowModels?: Record RowModel> + facetedMinMaxValues?: Record undefined | [number, number]> + facetedUniqueValues?: Record Map> + globalFacetedRowModel?: () => RowModel + globalFacetedMinMaxValues?: () => undefined | [number, number] + globalFacetedUniqueValues?: () => Map } export interface Table_ColumnFaceting< diff --git a/packages/table-core/src/features/column-faceting/columnFacetingFeature.utils.ts b/packages/table-core/src/features/column-faceting/columnFacetingFeature.utils.ts index 8c76af0f48..c2e3d39cc5 100644 --- a/packages/table-core/src/features/column-faceting/columnFacetingFeature.utils.ts +++ b/packages/table-core/src/features/column-faceting/columnFacetingFeature.utils.ts @@ -1,3 +1,4 @@ +import { makeObjectMap } from '../../utils' import type { CellData, RowData } from '../../types/type-utils' import type { TableFeatures } from '../../types/TableFeatures' import type { RowModel } from '../../core/row-models/coreRowModelsFeature.types' @@ -23,9 +24,16 @@ export function column_getFacetedMinMaxValues< column: Column_Internal, table: Table_Internal, ): [number, number] | undefined { - const facetedMinMaxValuesFn = - table.options.features.facetedMinMaxValues?.(table, column.id) ?? - (() => undefined) + const facetedMinMaxValues = (table._rowModels.facetedMinMaxValues ??= + makeObjectMap()) + let facetedMinMaxValuesFn = facetedMinMaxValues[column.id] + + if (!facetedMinMaxValuesFn) { + facetedMinMaxValuesFn = facetedMinMaxValues[column.id] = + table.options.features.facetedMinMaxValues?.(table, column.id) ?? + (() => undefined) + } + return facetedMinMaxValuesFn() } @@ -49,9 +57,17 @@ export function column_getFacetedRowModel< column: Column_Internal | undefined, table: Table_Internal, ): RowModel { - const facetedRowModelFn = - table.options.features.facetedRowModel?.(table, column?.id ?? '') ?? - (() => table.getPreFilteredRowModel()) + const columnId = column?.id ?? '' + const facetedRowModels = (table._rowModels.facetedRowModels ??= + makeObjectMap()) + let facetedRowModelFn = facetedRowModels[columnId] + + if (!facetedRowModelFn) { + facetedRowModelFn = facetedRowModels[columnId] = + table.options.features.facetedRowModel?.(table, columnId) ?? + (() => table.getPreFilteredRowModel()) + } + return facetedRowModelFn() } @@ -74,9 +90,16 @@ export function column_getFacetedUniqueValues< column: Column_Internal, table: Table_Internal, ): Map { - const facetedUniqueValuesFn = - table.options.features.facetedUniqueValues?.(table, column.id) ?? - (() => new Map()) + const facetedUniqueValues = (table._rowModels.facetedUniqueValues ??= + makeObjectMap()) + let facetedUniqueValuesFn = facetedUniqueValues[column.id] + + if (!facetedUniqueValuesFn) { + facetedUniqueValuesFn = facetedUniqueValues[column.id] = + table.options.features.facetedUniqueValues?.(table, column.id) ?? + (() => new Map()) + } + return facetedUniqueValuesFn() } @@ -95,9 +118,13 @@ export function table_getGlobalFacetedMinMaxValues< TFeatures extends TableFeatures, TData extends RowData, >(table: Table_Internal): undefined | [number, number] { - const facetedMinMaxValuesFn = - table.options.features.facetedMinMaxValues?.(table, '__global__') ?? - (() => undefined) + if (!table._rowModels.globalFacetedMinMaxValues) { + table._rowModels.globalFacetedMinMaxValues = + table.options.features.facetedMinMaxValues?.(table, '__global__') ?? + (() => undefined) + } + + const facetedMinMaxValuesFn = table._rowModels.globalFacetedMinMaxValues return facetedMinMaxValuesFn() } @@ -117,9 +144,13 @@ export function table_getGlobalFacetedRowModel< TFeatures extends TableFeatures, TData extends RowData, >(table: Table_Internal): RowModel { - const facetedRowModelFn = - table.options.features.facetedRowModel?.(table, '__global__') ?? - (() => table.getPreFilteredRowModel()) + if (!table._rowModels.globalFacetedRowModel) { + table._rowModels.globalFacetedRowModel = + table.options.features.facetedRowModel?.(table, '__global__') ?? + (() => table.getPreFilteredRowModel()) + } + + const facetedRowModelFn = table._rowModels.globalFacetedRowModel return facetedRowModelFn() } @@ -138,8 +169,12 @@ export function table_getGlobalFacetedUniqueValues< TFeatures extends TableFeatures, TData extends RowData, >(table: Table_Internal): Map { - const facetedUniqueValuesFn = - table.options.features.facetedUniqueValues?.(table, '__global__') ?? - (() => new Map()) + if (!table._rowModels.globalFacetedUniqueValues) { + table._rowModels.globalFacetedUniqueValues = + table.options.features.facetedUniqueValues?.(table, '__global__') ?? + (() => new Map()) + } + + const facetedUniqueValuesFn = table._rowModels.globalFacetedUniqueValues return facetedUniqueValuesFn() } diff --git a/packages/table-core/src/features/column-faceting/createFacetedRowModel.ts b/packages/table-core/src/features/column-faceting/createFacetedRowModel.ts index 9c944aa2ab..f0a3001a91 100644 --- a/packages/table-core/src/features/column-faceting/createFacetedRowModel.ts +++ b/packages/table-core/src/features/column-faceting/createFacetedRowModel.ts @@ -69,6 +69,10 @@ function _createFacetedRowModel< } if (globalFilter) filterableIds.push('__global__') + if (!filterableIds.length) { + return preRowModel + } + const filterRowsImpl = ( row: Row & Partial>, ) => { diff --git a/packages/table-core/src/features/column-faceting/createFacetedUniqueValues.ts b/packages/table-core/src/features/column-faceting/createFacetedUniqueValues.ts index 8c6ce5c86e..2afcf7de7b 100644 --- a/packages/table-core/src/features/column-faceting/createFacetedUniqueValues.ts +++ b/packages/table-core/src/features/column-faceting/createFacetedUniqueValues.ts @@ -51,14 +51,11 @@ function _createFacetedUniqueValues< for (let j = 0; j < values.length; j++) { const value = values[j] - if (facetedUniqueValues.has(value)) { - facetedUniqueValues.set( - value, - (facetedUniqueValues.get(value) ?? 0) + 1, - ) - } else { - facetedUniqueValues.set(value, 1) - } + const previousValue = facetedUniqueValues.get(value) + facetedUniqueValues.set( + value, + previousValue === undefined ? 1 : previousValue + 1, + ) } } diff --git a/packages/table-core/tests/unit/features/column-faceting/columnFacetingFeature.test.ts b/packages/table-core/tests/unit/features/column-faceting/columnFacetingFeature.test.ts new file mode 100644 index 0000000000..11c1a95197 --- /dev/null +++ b/packages/table-core/tests/unit/features/column-faceting/columnFacetingFeature.test.ts @@ -0,0 +1,152 @@ +import { describe, expect, it, vi } from 'vitest' +import { + columnFacetingFeature, + columnFilteringFeature, + constructTable, + coreFeatures, + createFacetedMinMaxValues, + createFacetedRowModel, + createFacetedUniqueValues, + createFilteredRowModel, + filterFns, +} from '../../../../src' +import { storeReactivityBindings } from '../../../../src/store-reactivity-bindings' +import { + column_getFacetedMinMaxValues, + column_getFacetedRowModel, + column_getFacetedUniqueValues, + table_getGlobalFacetedMinMaxValues, + table_getGlobalFacetedRowModel, + table_getGlobalFacetedUniqueValues, +} from '../../../../src/static-functions' +import type { ColumnDef } from '../../../../src' + +type Person = { + firstName: string + status: string | undefined +} + +const features = { + ...coreFeatures, + columnFacetingFeature, + columnFilteringFeature, + coreReactivityFeature: storeReactivityBindings(), + facetedMinMaxValues: createFacetedMinMaxValues(), + facetedRowModel: createFacetedRowModel(), + facetedUniqueValues: createFacetedUniqueValues(), + filteredRowModel: createFilteredRowModel(), + filterFns, +} + +const columns: Array> = [ + { + accessorKey: 'firstName', + id: 'firstName', + }, + { + accessorKey: 'status', + filterFn: 'equalsString', + id: 'status', + }, +] + +const data: Array = [ + { firstName: 'Alice', status: 'active' }, + { firstName: 'Bob', status: undefined }, + { firstName: 'Carol', status: 'active' }, +] + +function makeTable(columnFilters: Array<{ id: string; value: unknown }>) { + return constructTable({ + data, + columns, + features, + initialState: { + columnFilters, + }, + }) +} + +describe('column faceting row model', () => { + it('reuses the pre-filtered row model when only the faceted column is filtered', () => { + const table = makeTable([{ id: 'status', value: 'active' }]) + const statusColumn = table.getColumn('status')! + + expect(statusColumn.getFacetedRowModel()).toBe( + table.getPreFilteredRowModel(), + ) + }) + + it('still applies filters from other columns', () => { + const table = makeTable([{ id: 'status', value: 'active' }]) + const firstNameColumn = table.getColumn('firstName')! + + expect( + firstNameColumn.getFacetedRowModel().rows.map((row) => row.original), + ).toEqual([ + { firstName: 'Alice', status: 'active' }, + { firstName: 'Carol', status: 'active' }, + ]) + }) + + it('counts unique values without dropping undefined facet values', () => { + const table = makeTable([]) + const statusColumn = table.getColumn('status')! + + expect(statusColumn.getFacetedUniqueValues()).toEqual( + new Map([ + ['active', 2], + [undefined, 1], + ]), + ) + }) + + it('caches faceted factory functions per column and global context', () => { + const facetedRowModel = vi.fn( + (table: any, _columnId: string) => () => table.getPreFilteredRowModel(), + ) + const facetedMinMaxValues = vi.fn( + (_table: any, _columnId: string) => () => [1, 2] as [number, number], + ) + const facetedUniqueValues = vi.fn( + (_table: any, _columnId: string) => () => + new Map([['cached', 1]]), + ) + + const cacheFeatures = { + ...coreFeatures, + coreReactivityFeature: storeReactivityBindings(), + facetedMinMaxValues, + facetedRowModel, + facetedUniqueValues, + } + + const table = constructTable({ + data, + columns: columns as any, + features: cacheFeatures, + }) as any + const statusColumn = table.getColumn('status')! + + column_getFacetedRowModel(statusColumn, table) + column_getFacetedRowModel(statusColumn, table) + table_getGlobalFacetedRowModel(table) + table_getGlobalFacetedRowModel(table) + + column_getFacetedMinMaxValues(statusColumn, table) + column_getFacetedMinMaxValues(statusColumn, table) + table_getGlobalFacetedMinMaxValues(table) + table_getGlobalFacetedMinMaxValues(table) + + column_getFacetedUniqueValues(statusColumn, table) + column_getFacetedUniqueValues(statusColumn, table) + table_getGlobalFacetedUniqueValues(table) + table_getGlobalFacetedUniqueValues(table) + + expect(facetedRowModel).toHaveBeenCalledTimes(2) + expect(facetedRowModel).toHaveBeenNthCalledWith(1, table, 'status') + expect(facetedRowModel).toHaveBeenNthCalledWith(2, table, '__global__') + expect(facetedMinMaxValues).toHaveBeenCalledTimes(2) + expect(facetedUniqueValues).toHaveBeenCalledTimes(2) + }) +}) diff --git a/perf-done.md b/perf-done.md index a1bd20ba44..d515c425bd 100644 --- a/perf-done.md +++ b/perf-done.md @@ -7,10 +7,58 @@ Entries are sorted by adjusted effectiveness score descending. ## Counts -- **Entries:** 33 -- **Source findings:** 31 +- **Entries:** 37 +- **Source findings:** 35 - **Cross-cutting sweeps:** 2 +## Score 9 + +## 62. B1: Faceted row model does a full O(R) rebuild when its filter set is empty — Score: 9 + +**Status:** `[x]` done +**Implementation note:** Added the post-exclusion empty-filter guard in `createFacetedRowModel`, returning `preRowModel` directly when the only active filter is the faceted column's own filter. Added regression coverage for identity reuse and for preserving filtering by other columns. + +**Location:** `packages/table-core/src/features/column-faceting/createFacetedRowModel.ts:59–84` +**Category:** `big-o` (short-circuit) + +Hot path: Per state-change: every columnFilters/globalFilter keystroke, per faceted column. The extremely common case "the only active filter is on the faceted column itself" (one filter input with a facet dropdown/range on the same column) yields `filterableIds.length === 0`. `filterRowsImpl` then returns `true` for every row, yet `filterRows` still walks all R rows, pushes into a new `flatRows` array, inserts R entries into a new `rowsById` map, and calls `constructRow` clones for rows with subRows. The result is content-identical to `preRowModel`. At R=100k this is a full O(R) pass with R keyed-map insertions per keystroke, per faceted column. + +**Before** + +```ts +if (!preRowModel.rows.length || (!columnFilters?.length && !globalFilter)) { + return preRowModel +} + +const filterableIds: Array = [] +if (columnFilters) { + for (let i = 0; i < columnFilters.length; i++) { + const id = columnFilters[i]!.id + if (id !== columnId) filterableIds.push(id) + } +} +if (globalFilter) filterableIds.push('__global__') +// ... +return filterRows(preRowModel.rows, filterRowsImpl, table) +``` + +**After** + +```ts +if (globalFilter) filterableIds.push('__global__') + +if (!filterableIds.length) { + return preRowModel +} +``` + +**Big-O:** O(R) → O(1) for the excluded-own-filter-only case: ~100k iterations + ~100k object-map insertions + 2 array allocations avoided per keystroke per faceted column (~10-30ms at 100k rows). Verified bonus: `getFacetedUniqueValues`/`getFacetedMinMaxValues` memoDeps key on `.flatRows` identity, so the stable `preRowModel` reference also converts the downstream O(R) facet-map rebuild into a memo hit per keystroke. + +**Risk:** Returns `preRowModel` by reference instead of a fresh model with identical contents; same referential behavior as the existing empty-filter branch. For hierarchical data there is a flatRows-order note: the current all-pass `filterRows` emits child-before-parent flatRows while `preRowModel` is parent-first, so facet `Map` insertion order can shift (content identical; the existing no-filter branch already returns parent-first, so precedent exists). The early return never reads tags, so B17's ordering constraint does not apply. +**Verification:** CONFIRMED, raised 8 → 9; verifier added the downstream facet-map memo-hit win and the hierarchical flatRows-order note. + +--- + ## Score 8 ## 35. `row_getLeftVisibleCells` / `row_getRightVisibleCells` use `.find` in pin loop — Score: 8 @@ -523,6 +571,7 @@ Deps coverage (everything `table_getColumnOffsets` reads transitively): `table_g **Implementation note:** Fixed in PR #6367 (47fc97d2f). `grouping` + `groupedColumnMode` added to the memoDeps tuples in columnPinningFeature.ts (6 registrations), columnVisibilityFeature.ts (`table_getVisibleLeafColumns` / `table_getVisibleFlatColumns`), and columnOrderingFeature.ts (`column_getIndex`, closing the `groupedColumnMode` gap). `header_getStart`'s vestigial `position` dep slot was also dropped (the half of A8 that belonged to this fix; A8's remaining closure-to-stack scope lives in todo #114). Two regression tests added in columnSizingFeature.utils.test.ts, verified to fail against the pre-fix deps. **Location:** + - `packages/table-core/src/features/column-sizing/columnSizingFeature.ts:56–63, 67–74, 95–102` (`column_getStart`, `column_getAfter`, `header_getStart` deps) - `packages/table-core/src/features/column-pinning/columnPinningFeature.ts:254–277` (`table_get{Left,Right,Center}LeafColumns` deps) and `:283–309` (`table_get{Left,Center,Right}VisibleLeafColumns` deps) - `packages/table-core/src/features/column-visibility/columnVisibilityFeature.ts:94–101` (`table_getVisibleLeafColumns` deps) and `:86–101` (`table_getVisibleFlatColumns`, same gap; added during verification) @@ -1585,6 +1634,57 @@ return [facetedMinValue, facetedMaxValue] --- +## 89. B16+E13: Faceted factories re-invoked per call, constructing throwaway tableMemo instances — Score: 5 + +**Status:** `[x]` done +**Implementation note:** Added table-level `_rowModels` caches for per-column faceted row models, unique values, and min/max values, plus single global slots for the three global faceted variants. This preserves each factory's inner memo as the invalidation authority and avoids rebuilding `tableMemo` closures on repeated reads. Added regression coverage that direct static utility calls construct each per-column/global factory only once. + +**Location:** `packages/table-core/src/features/column-faceting/columnFacetingFeature.utils.ts:44–56` (also 18–30, 69–81, 94–145), vs. the cached pattern at `src/core/row-models/coreRowModelsFeature.utils.ts:65–75` +**Category:** `memoization`, `allocation` + +Hot path: per state-change: every faceted deps-change per faceted column; per call in the no-prototype fallback. `table.options.features.facetedRowModel` is the factory from `createFacetedRowModel()`; each invocation builds a brand-new `tableMemo` (memo closure, debug-name parsing in dev, scheduling wiring) whose single-slot cache is used exactly once and discarded, so its memoization is structurally dead. The filtered/sorted/grouped models avoid this by caching the constructed memo in `table._rowModels`. On the registered-feature path the column prototype memo caches results, hiding the cost as "factory + tableMemo construction per deps-change per faceted column". In the fallback path (facet factories registered without `columnFacetingFeature`), every single call performs a full O(R) recompute. Same pattern for `column_getFacetedMinMaxValues`, `column_getFacetedUniqueValues`, and the three `table_getGlobalFaceted*` fns. + +**Before** + +```ts +export function column_getFacetedRowModel<...>( + column: ..., + table: Table_Internal, +): RowModel { + const facetedRowModelFn = + table.options.features.facetedRowModel?.(table, column?.id ?? '') ?? + (() => table.getPreFilteredRowModel()) + return facetedRowModelFn() +} +``` + +**After** + +(B16's table-level `_rowModels`-style cache, preferred over E13's on-column cache because it covers the three global variants and matches existing typing) + +```ts +export function column_getFacetedRowModel<...>(column, table): RowModel { + const columnId = column?.id ?? '' + const cache = (table._rowModels.facetedRowModels ??= makeObjectMap()) + let facetedRowModelFn = cache[columnId] + if (!facetedRowModelFn) { + facetedRowModelFn = cache[columnId] = + table.options.features.facetedRowModel?.(table, columnId) ?? + (() => table.getPreFilteredRowModel()) + } + return facetedRowModelFn() +} +``` + +(plus `facetedUniqueValues`/`facetedMinMaxValues` maps and the three global variants cached as single slots; extend the `CachedRowModels` types accordingly.) + +**Big-O:** Registered path: one factory + tableMemo construction (several closures + dev string work) saved per deps-change per faceted column. Fallback path: repeated full O(R) recomputes per call → memoized (at R=100k with a facet dropdown read per render, ~10ms/render → ~0). + +**Risk:** The cache is keyed by columnId, scaling with N (doctrine-compliant). Cache lifetime matches `_rowModels` (never reset; must not outlive `table.options.features` swaps, same invariant as filtered/sorted). The inner memo becoming long-lived makes its own memoDeps the invalidation authority; the dep set is identical to the prototype memo, so results stay coherent. +**Verification:** MERGED (B16 ≡ E13; B16 formulation kept), demoted 6 → 5: on the registered path the outer prototype memo already caches results, so the win there is construction allocations only; the O(R)-per-call fallback is real but requires a specific misconfiguration. + +--- + ## Score 4 ## 17. `row_getAllCells` / `row_getAllCellsByColumnId` use `.map`/`for...of` — Score: 4 @@ -1738,6 +1838,32 @@ if (core) return core --- +## 22. `createFacetedUniqueValues` redundant `Map.has` before `Map.set` — Score: 3 + +**Status:** `[x]` done +**Implementation note:** Replaced `has` + `get` + `set` with one `get` and one `set`, using `previousValue === undefined` as the miss sentinel because stored counts are always numbers. Added faceted unique-values coverage with an `undefined` facet key to lock in the sentinel behavior. + +**Location:** `src/features/column-faceting/createFacetedUniqueValues.ts:46–62` +**Category:** `micro` + +`set(k, (get(k) ?? 0) + 1)` works in either branch. + +**Scale impact** (Map ops saved per facet rebuild — dimension: distinct value encounters): + +| Value occurrences | Before (`has` + `get` + `set`) | After (`get` + `set`) | Saved Map ops | +| ----------------- | ------------------------------ | --------------------- | ------------- | +| 10 | 30 | 20 | 10 | +| 100 | 300 | 200 | 100 | +| 1,000 | 3,000 | 2,000 | 1,000 | +| 10,000 | 30,000 | 20,000 | 10,000 | + +**Risk:** None. + +**2026-07-01 audit (B18, score 3):** Re-verified and sharpened: `has` + `get` + `set` is 3 hashed Map ops per value on the hit path (the Map itself is correct here — R-scaling key space, not a tiny state array). Proposed form: `const prev = map.get(v); map.set(v, prev === undefined ? 1 : prev + 1)` — safe because stored counts are always numbers, so `prev === undefined` ⟺ miss. +**Verification:** Verified (2026-07-01 audit). + +--- + ## 46. `table_toggleAllRowsSelected` clones entire selection on deselect — Score: 3 **Status:** `[~]` partial diff --git a/perf-todo.md b/perf-todo.md index 73e169ab40..e218478f92 100644 --- a/perf-todo.md +++ b/perf-todo.md @@ -7,58 +7,10 @@ Entries are sorted by adjusted effectiveness score descending. ## Counts -- **Entries:** 107 -- **Source findings:** 107 +- **Entries:** 103 +- **Source findings:** 103 - **Cross-cutting sweeps:** 0 -## Score 9 - -## 62. B1: Faceted row model does a full O(R) rebuild when its filter set is empty — Score: 9 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `packages/table-core/src/features/column-faceting/createFacetedRowModel.ts:59–84` -**Category:** `big-o` (short-circuit) - -Hot path: Per state-change: every columnFilters/globalFilter keystroke, per faceted column. The extremely common case "the only active filter is on the faceted column itself" (one filter input with a facet dropdown/range on the same column) yields `filterableIds.length === 0`. `filterRowsImpl` then returns `true` for every row, yet `filterRows` still walks all R rows, pushes into a new `flatRows` array, inserts R entries into a new `rowsById` map, and calls `constructRow` clones for rows with subRows. The result is content-identical to `preRowModel`. At R=100k this is a full O(R) pass with R keyed-map insertions per keystroke, per faceted column. - -**Before** - -```ts -if (!preRowModel.rows.length || (!columnFilters?.length && !globalFilter)) { - return preRowModel -} - -const filterableIds: Array = [] -if (columnFilters) { - for (let i = 0; i < columnFilters.length; i++) { - const id = columnFilters[i]!.id - if (id !== columnId) filterableIds.push(id) - } -} -if (globalFilter) filterableIds.push('__global__') -// ... -return filterRows(preRowModel.rows, filterRowsImpl, table) -``` - -**After** - -```ts -if (globalFilter) filterableIds.push('__global__') - -if (!filterableIds.length) { - return preRowModel -} -``` - -**Big-O:** O(R) → O(1) for the excluded-own-filter-only case: ~100k iterations + ~100k object-map insertions + 2 array allocations avoided per keystroke per faceted column (~10-30ms at 100k rows). Verified bonus: `getFacetedUniqueValues`/`getFacetedMinMaxValues` memoDeps key on `.flatRows` identity, so the stable `preRowModel` reference also converts the downstream O(R) facet-map rebuild into a memo hit per keystroke. - -**Risk:** Returns `preRowModel` by reference instead of a fresh model with identical contents; same referential behavior as the existing empty-filter branch. For hierarchical data there is a flatRows-order note: the current all-pass `filterRows` emits child-before-parent flatRows while `preRowModel` is parent-first, so facet `Map` insertion order can shift (content identical; the existing no-filter branch already returns parent-first, so precedent exists). The early return never reads tags, so B17's ordering constraint does not apply. -**Verification:** CONFIRMED, raised 8 → 9; verifier added the downstream facet-map memo-hit win and the hierarchical flatRows-order note. - ---- - ## Score 8 ## 64. E2: Allocation-free compareAlphanumeric — Score: 8 @@ -476,7 +428,7 @@ if (resolvedGlobalFilters.length && !(failed && !needsAllFilterTags)) { --- -## 70. B19: constructRow allocates Object.values(table._features) and probes all features per row — Score: 7 +## 70. B19: constructRow allocates Object.values(table.\_features) and probes all features per row — Score: 7 **Status:** `[ ]` not started **Implementation note:** _(none)_ @@ -1934,57 +1886,6 @@ Also hoist the comparator itself out of `sortData` (it captures only outer-scope --- -## 89. B16+E13: Faceted factories re-invoked per call, constructing throwaway tableMemo instances — Score: 5 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `packages/table-core/src/features/column-faceting/columnFacetingFeature.utils.ts:44–56` (also 18–30, 69–81, 94–145), vs. the cached pattern at `src/core/row-models/coreRowModelsFeature.utils.ts:65–75` -**Category:** `memoization`, `allocation` - -Hot path: per state-change: every faceted deps-change per faceted column; per call in the no-prototype fallback. `table.options.features.facetedRowModel` is the factory from `createFacetedRowModel()`; each invocation builds a brand-new `tableMemo` (memo closure, debug-name parsing in dev, scheduling wiring) whose single-slot cache is used exactly once and discarded, so its memoization is structurally dead. The filtered/sorted/grouped models avoid this by caching the constructed memo in `table._rowModels`. On the registered-feature path the column prototype memo caches results, hiding the cost as "factory + tableMemo construction per deps-change per faceted column". In the fallback path (facet factories registered without `columnFacetingFeature`), every single call performs a full O(R) recompute. Same pattern for `column_getFacetedMinMaxValues`, `column_getFacetedUniqueValues`, and the three `table_getGlobalFaceted*` fns. - -**Before** - -```ts -export function column_getFacetedRowModel<...>( - column: ..., - table: Table_Internal, -): RowModel { - const facetedRowModelFn = - table.options.features.facetedRowModel?.(table, column?.id ?? '') ?? - (() => table.getPreFilteredRowModel()) - return facetedRowModelFn() -} -``` - -**After** - -(B16's table-level `_rowModels`-style cache, preferred over E13's on-column cache because it covers the three global variants and matches existing typing) - -```ts -export function column_getFacetedRowModel<...>(column, table): RowModel { - const columnId = column?.id ?? '' - const cache = (table._rowModels.facetedRowModels ??= makeObjectMap()) - let facetedRowModelFn = cache[columnId] - if (!facetedRowModelFn) { - facetedRowModelFn = cache[columnId] = - table.options.features.facetedRowModel?.(table, columnId) ?? - (() => table.getPreFilteredRowModel()) - } - return facetedRowModelFn() -} -``` - -(plus `facetedUniqueValues`/`facetedMinMaxValues` maps and the three global variants cached as single slots; extend the `CachedRowModels` types accordingly.) - -**Big-O:** Registered path: one factory + tableMemo construction (several closures + dev string work) saved per deps-change per faceted column. Fallback path: repeated full O(R) recomputes per call → memoized (at R=100k with a facet dropdown read per render, ~10ms/render → ~0). - -**Risk:** The cache is keyed by columnId, scaling with N (doctrine-compliant). Cache lifetime matches `_rowModels` (never reset; must not outlive `table.options.features` swaps, same invariant as filtered/sorted). The inner memo becoming long-lived makes its own memoDeps the invalidation authority; the dep set is identical to the prototype memo, so results stay coherent. -**Verification:** MERGED (B16 ≡ E13; B16 formulation kept), demoted 6 → 5: on the registered path the outer prototype memo already caches results, so the win there is construction allocations only; the O(R)-per-call fallback is real but requires a specific misconfiguration. - ---- - ## 90. C3: row_getVisibleCells: O(N) per-cell memoized getIsVisible calls per row instead of the table-level visible-column list — Score: 5 **Status:** `[ ]` not started @@ -2327,14 +2228,15 @@ export function cell_getIsAggregated<...>(cell: Cell) **Implementation note:** _(none)_ **Location:** - - `packages/react-table/src/useTable.ts:176–179` (per render) - - `packages/preact-table/src/useTable.ts:147–150` (per render) - - `packages/angular-table/src/injectTable.ts:128–133` (per options-effect run) - - `packages/lit-table/src/TableController.ts:177–180` (per host render) - - `packages/alpine-table/src/createTable.ts:82–85` (per effect run) - - `packages/svelte-table/src/createTable.svelte.ts:117–121` (`flatMerge(prev, mergedOptions)`, per `$effect.pre` run) - - `packages/vue-table/src/useTable.ts:89–92` (`flatMerge(prev, ...)`, per watch fire) - - `packages/solid-table/src/createTable.ts:100–104` (`mergeProps(prev, mergedOptions)`, per computed run) + +- `packages/react-table/src/useTable.ts:176–179` (per render) +- `packages/preact-table/src/useTable.ts:147–150` (per render) +- `packages/angular-table/src/injectTable.ts:128–133` (per options-effect run) +- `packages/lit-table/src/TableController.ts:177–180` (per host render) +- `packages/alpine-table/src/createTable.ts:82–85` (per effect run) +- `packages/svelte-table/src/createTable.svelte.ts:117–121` (`flatMerge(prev, mergedOptions)`, per `$effect.pre` run) +- `packages/vue-table/src/useTable.ts:89–92` (`flatMerge(prev, ...)`, per watch fire) +- `packages/solid-table/src/createTable.ts:100–104` (`mergeProps(prev, mergedOptions)`, per computed run) **Category:** `allocation` @@ -3086,32 +2988,6 @@ export function createColumnHelper<...>(): ColumnHelper { --- -## 22. `createFacetedUniqueValues` redundant `Map.has` before `Map.set` — Score: 3 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `src/features/column-faceting/createFacetedUniqueValues.ts:46–62` -**Category:** `micro` - -`set(k, (get(k) ?? 0) + 1)` works in either branch. - -**Scale impact** (Map ops saved per facet rebuild — dimension: distinct value encounters): - -| Value occurrences | Before (`has` + `get` + `set`) | After (`get` + `set`) | Saved Map ops | -| ----------------- | ------------------------------ | --------------------- | ------------- | -| 10 | 30 | 20 | 10 | -| 100 | 300 | 200 | 100 | -| 1,000 | 3,000 | 2,000 | 1,000 | -| 10,000 | 30,000 | 20,000 | 10,000 | - -**Risk:** None. - -**2026-07-01 audit (B18, score 3):** Re-verified and sharpened: `has` + `get` + `set` is 3 hashed Map ops per value on the hit path (the Map itself is correct here — R-scaling key space, not a tiny state array). Proposed form: `const prev = map.get(v); map.set(v, prev === undefined ? 1 : prev + 1)` — safe because stored counts are always numbers, so `prev === undefined` ⟺ miss. -**Verification:** Verified (2026-07-01 audit). - ---- - ## 25. `column_setFilterValue` re-searches array — Score: 3 **Status:** `[ ]` not started @@ -3331,7 +3207,7 @@ Hit path does `hasOwn` + second keyed lookup; miss path writes then re-reads; `r --- -## 127. D9: header/group ids built via [..].filter(Boolean).join('_'): 2 array allocations per header — Score: 3 +## 127. D9: header/group ids built via [..].filter(Boolean).join('\_'): 2 array allocations per header — Score: 3 **Status:** `[ ]` not started **Implementation note:** _(none)_ @@ -3634,23 +3510,6 @@ Each file has a `getXyzPrototype(table)` function with identical shape — `if ( --- -## 109. B17: Faceted model's 4th memoDep is load-bearing: DO NOT REMOVE (observation) — Score: 2 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `packages/table-core/src/features/column-faceting/createFacetedRowModel.ts:31–45` -**Category:** `observation` - -The 4th dep `table.getFilteredRowModel()` looks redundant (fn never receives it) but forces the filtered model to run FIRST, guaranteeing fresh `row.columnFilters` tags that `_createFacetedRowModel` reads as a side effect of the filtered model's tagging loop. Removing it introduces a stale-tag bug. - -**Fix:** Add a source comment (`// keep: forces filter tagging side effects before facet reads`) at the 4th memoDep slot. This dependency must NOT be removed, even though it looks redundant to a naive dep-drift audit. - -**Risk:** None noted. -**Verification:** CONFIRMED (guard-rail); verified non-interacting with B1. - ---- - ## 119. B20: getSubRows dereferenced twice per row; child row-id built via array + join — Score: 2 **Status:** `[ ]` not started