From bc0f71a4ed531749411289431828dd733286e240 Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Tue, 7 Jul 2026 09:43:42 -0500 Subject: [PATCH] fix: performance improvements related to cell order --- .../src/core/rows/coreRowsFeature.types.ts | 5 + .../src/core/rows/coreRowsFeature.utils.ts | 14 +- .../column-ordering/columnOrderingFeature.ts | 20 +- .../columnOrderingFeature.types.ts | 26 + .../columnOrderingFeature.utils.ts | 61 ++- .../columnPinningFeature.utils.ts | 34 +- packages/table-core/src/utils.ts | 5 +- .../core/rows/coreRowsFeature.utils.test.ts | 64 +++ .../columnOrderingFeature.utils.test.ts | 84 ++- .../columnPinningFeature.utils.test.ts | 71 ++- perf-done.md | 288 +++++++++- perf-skipped.md | 70 ++- perf-todo.md | 493 +----------------- 13 files changed, 715 insertions(+), 520 deletions(-) create mode 100644 packages/table-core/tests/unit/core/rows/coreRowsFeature.utils.test.ts diff --git a/packages/table-core/src/core/rows/coreRowsFeature.types.ts b/packages/table-core/src/core/rows/coreRowsFeature.types.ts index 4345405577..993775a9fb 100644 --- a/packages/table-core/src/core/rows/coreRowsFeature.types.ts +++ b/packages/table-core/src/core/rows/coreRowsFeature.types.ts @@ -3,11 +3,16 @@ import type { RowData } from '../../types/type-utils' import type { TableFeatures } from '../../types/TableFeatures' import type { Row } from '../../types/Row' import type { Cell } from '../../types/Cell' +import type { Column } from '../../types/Column' export interface Row_CoreProperties< in out TFeatures extends TableFeatures, in out TData extends RowData, > { + _cellsCache?: WeakMap< + Column, + Cell + > _uniqueValuesCache: Record _valuesCache: Record /** diff --git a/packages/table-core/src/core/rows/coreRowsFeature.utils.ts b/packages/table-core/src/core/rows/coreRowsFeature.utils.ts index a4318bf15c..2e8b74927c 100644 --- a/packages/table-core/src/core/rows/coreRowsFeature.utils.ts +++ b/packages/table-core/src/core/rows/coreRowsFeature.utils.ts @@ -169,11 +169,23 @@ export function row_getAllCells< TData extends RowData, >(row: Row): Array> { const columns = row.table.getAllLeafColumns() + // WeakMap so cells keyed by replaced column instances can be collected; + // rows are memoized on data only and outlive column generations + let cache = row._cellsCache + if (!cache) { + cache = row._cellsCache = new WeakMap() + } const cells: Array> = new Array( columns.length, ) for (let i = 0; i < columns.length; i++) { - cells[i] = constructCell(columns[i]!, row, row.table) + const column = columns[i]! + let cell = cache.get(column) + if (!cell) { + cell = constructCell(column, row, row.table) + cache.set(column, cell) + } + cells[i] = cell } return cells } diff --git a/packages/table-core/src/features/column-ordering/columnOrderingFeature.ts b/packages/table-core/src/features/column-ordering/columnOrderingFeature.ts index fb33cefb29..a279a44fc7 100644 --- a/packages/table-core/src/features/column-ordering/columnOrderingFeature.ts +++ b/packages/table-core/src/features/column-ordering/columnOrderingFeature.ts @@ -8,6 +8,7 @@ import { column_getIsFirstColumn, column_getIsLastColumn, getDefaultColumnOrderState, + table_getColumnIndexes, table_getOrderColumnsFn, table_resetColumnOrder, table_setColumnOrder, @@ -35,14 +36,6 @@ export const columnOrderingFeature: TableFeature = { assignPrototypeAPIs('columnOrderingFeature', prototype, table, { column_getIndex: { fn: (column, position) => column_getIndex(column, position), - memoDeps: (column, position) => [ - position, - column.table.atoms.columnOrder?.get(), - column.table.atoms.columnPinning?.get(), - column.table.atoms.grouping?.get(), - column.table.atoms.columnVisibility?.get(), - column.table.options.groupedColumnMode, - ], }, column_getIsFirstColumn: { fn: (column, position) => column_getIsFirstColumn(column, position), @@ -55,6 +48,17 @@ export const columnOrderingFeature: TableFeature = { constructTableAPIs: (table) => { assignTableAPIs('columnOrderingFeature', table, { + table_getColumnIndexes: { + fn: () => table_getColumnIndexes(table), + memoDeps: () => [ + table.options.columns, + table.atoms.columnOrder?.get(), + table.atoms.columnPinning?.get(), + table.atoms.columnVisibility?.get(), + table.atoms.grouping?.get(), + table.options.groupedColumnMode, + ], + }, table_setColumnOrder: { fn: (updater) => table_setColumnOrder(table, updater), }, diff --git a/packages/table-core/src/features/column-ordering/columnOrderingFeature.types.ts b/packages/table-core/src/features/column-ordering/columnOrderingFeature.types.ts index be7be63052..9141761d5a 100644 --- a/packages/table-core/src/features/column-ordering/columnOrderingFeature.types.ts +++ b/packages/table-core/src/features/column-ordering/columnOrderingFeature.types.ts @@ -4,6 +4,25 @@ import type { ColumnPinningPosition } from '../column-pinning/columnPinningFeatu export type ColumnOrderState = Array +export interface ColumnIndexes { + /** + * Maps each visible leaf column id to its index in the full visible column list. + */ + all: Record + /** + * Maps each unpinned visible leaf column id to its index within the center region. + */ + center: Record + /** + * Maps each left-pinned visible leaf column id to its index within the left region. + */ + left: Record + /** + * Maps each right-pinned visible leaf column id to its index within the right region. + */ + right: Record +} + export interface TableState_ColumnOrdering { columnOrder: ColumnOrderState } @@ -47,6 +66,13 @@ export interface Table_ColumnOrdering< in out TFeatures extends TableFeatures, in out TData extends RowData, > { + /** + * Builds column-id to index records for each visible pinning region. + * + * This is the memoized source that `column.getIndex` reads from; most apps + * will not need to call it directly. + */ + getColumnIndexes: () => ColumnIndexes /** * Resets `columnOrder` to `initialState.columnOrder`. * diff --git a/packages/table-core/src/features/column-ordering/columnOrderingFeature.utils.ts b/packages/table-core/src/features/column-ordering/columnOrderingFeature.utils.ts index 49a2654aef..25701fe2c2 100644 --- a/packages/table-core/src/features/column-ordering/columnOrderingFeature.utils.ts +++ b/packages/table-core/src/features/column-ordering/columnOrderingFeature.utils.ts @@ -1,12 +1,15 @@ import { table_getPinnedVisibleLeafColumns } from '../column-pinning/columnPinningFeature.utils' -import { cloneState } from '../../utils' +import { callMemoOrStaticFn, cloneState, makeObjectMap } from '../../utils' import type { GroupingState } from '../column-grouping/columnGroupingFeature.types' import type { CellData, RowData, Updater } from '../../types/type-utils' import type { TableFeatures } from '../../types/TableFeatures' import type { Table_Internal } from '../../types/Table' -import type { Column_Internal } from '../../types/Column' +import type { Column, Column_Internal } from '../../types/Column' import type { ColumnPinningPosition } from '../column-pinning/columnPinningFeature.types' -import type { ColumnOrderState } from './columnOrderingFeature.types' +import type { + ColumnIndexes, + ColumnOrderState, +} from './columnOrderingFeature.types' /** * Creates the default column order state. @@ -23,6 +26,42 @@ export function getDefaultColumnOrderState(): ColumnOrderState { return [] } +/** + * Builds column-id to index records for each visible pinning region. + * + * All four regions are built in one pass so a single memo entry serves every + * `column_getIndex` lookup without per-column scans. + * + * @example + * ```ts + * const indexes = table_getColumnIndexes(table) + * ``` + */ +export function table_getColumnIndexes< + TFeatures extends TableFeatures, + TData extends RowData, +>(table: Table_Internal): ColumnIndexes { + const buildIndexes = ( + columns: ReadonlyArray< + | Column + | Column_Internal + >, + ): Record => { + const indexes = makeObjectMap() + for (let i = 0; i < columns.length; i++) { + indexes[columns[i]!.id] = i + } + return indexes + } + + return { + all: buildIndexes(table_getPinnedVisibleLeafColumns(table)), + center: buildIndexes(table_getPinnedVisibleLeafColumns(table, 'center')), + left: buildIndexes(table_getPinnedVisibleLeafColumns(table, 'left')), + right: buildIndexes(table_getPinnedVisibleLeafColumns(table, 'right')), + } +} + /** * Finds this column's index within a visible pinning region. * @@ -42,8 +81,20 @@ export function column_getIndex< column: Column_Internal, position?: ColumnPinningPosition | 'center', ) { - const columns = table_getPinnedVisibleLeafColumns(column.table, position) - return columns.findIndex((d) => d.id === column.id) + const indexes = callMemoOrStaticFn( + column.table, + 'getColumnIndexes', + table_getColumnIndexes, + ) + const key = + position === 'left' + ? 'left' + : position === 'right' + ? 'right' + : position === 'center' + ? 'center' + : 'all' + return indexes[key][column.id] ?? -1 } /** diff --git a/packages/table-core/src/features/column-pinning/columnPinningFeature.utils.ts b/packages/table-core/src/features/column-pinning/columnPinningFeature.utils.ts index 4bb3f4305f..03d92287ab 100644 --- a/packages/table-core/src/features/column-pinning/columnPinningFeature.utils.ts +++ b/packages/table-core/src/features/column-pinning/columnPinningFeature.utils.ts @@ -139,15 +139,22 @@ export function column_getIsPinned< >( column: Column_Internal, ): ColumnPinningPosition | false { - const leafColumnIds = column.getLeafColumns().map((d) => d.id) + const leafColumns = column.getLeafColumns() const { left, right } = column.table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() - const isLeft = leafColumnIds.some((d) => left.includes(d)) - const isRight = leafColumnIds.some((d) => right.includes(d)) - - return isLeft ? 'left' : isRight ? 'right' : false + for (let i = 0; i < leafColumns.length; i++) { + if (left.includes(leafColumns[i]!.id)) { + return 'left' + } + } + for (let i = 0; i < leafColumns.length; i++) { + if (right.includes(leafColumns[i]!.id)) { + return 'right' + } + } + return false } /** @@ -197,6 +204,9 @@ export function row_getCenterVisibleCells< ) const { left, right } = row.table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() + if (!left.length && !right.length) { + return allCells + } const leftAndRight: Array = [...left, ...right] return allCells.filter((d) => !leftAndRight.includes(d.column.id)) } @@ -441,11 +451,12 @@ export function table_getCenterHeaderGroups< ) const { left, right } = table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() - const leftAndRight: Array = [...left, ...right] - - leafColumns = leafColumns.filter( - (column) => !leftAndRight.includes(column.id), - ) + if (left.length || right.length) { + const leftAndRight: Array = [...left, ...right] + leafColumns = leafColumns.filter( + (column) => !leftAndRight.includes(column.id), + ) + } return buildHeaderGroups(allColumns, leafColumns, table, 'center') } @@ -741,6 +752,9 @@ export function table_getCenterLeafColumns< >(table: Table_Internal) { const { left, right } = table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() + if (!left.length && !right.length) { + return table.getAllLeafColumns() + } const leftAndRight: Array = [...left, ...right] return table.getAllLeafColumns().filter((d) => !leftAndRight.includes(d.id)) } diff --git a/packages/table-core/src/utils.ts b/packages/table-core/src/utils.ts index 2b843820b3..ee4dd3ae2f 100755 --- a/packages/table-core/src/utils.ts +++ b/packages/table-core/src/utils.ts @@ -52,7 +52,8 @@ export function cloneState(value: T): T { /** * Copies prototype-instance own properties without carrying over lazy memo - * closures that were bound to the source instance. + * closures or the per-row cell cache, both of which are bound to the source + * instance (cached cells reference the source row). */ export function copyInstancePropertiesWithoutMemos< TTarget extends Record, @@ -63,7 +64,7 @@ export function copyInstancePropertiesWithoutMemos< for (let i = 0; i < keys.length; i++) { const key = keys[i]! - if (!key.startsWith('_memo_')) { + if (!key.startsWith('_memo_') && key !== '_cellsCache') { targetRecord[key] = source[key] } } diff --git a/packages/table-core/tests/unit/core/rows/coreRowsFeature.utils.test.ts b/packages/table-core/tests/unit/core/rows/coreRowsFeature.utils.test.ts new file mode 100644 index 0000000000..410d65547f --- /dev/null +++ b/packages/table-core/tests/unit/core/rows/coreRowsFeature.utils.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from 'vitest' +import { stockFeatures } from '../../../../src' +import { row_getAllCells } from '../../../../src/core/rows/coreRowsFeature.utils' +import { + generateTestTableWithData, + generateTestTableWithDataAndState, +} from '../../../helpers/generateTestTable' +import type { Person } from '../../../fixtures/data/types' +import type { + StockFeatures, + Table_ColumnOrdering, + Table_Internal, +} from '../../../../src' + +describe('row_getAllCells', () => { + it('should build one cell per leaf column in leaf column order', () => { + const table = generateTestTableWithData(1) + const row = table.getRowModel().rows[0]! + + const cells = row_getAllCells(row) + + expect(cells.map((cell) => cell.column.id)).toEqual( + table.getAllLeafColumns().map((col) => col.id), + ) + expect(cells.every((cell) => cell.row === row)).toBe(true) + }) + + it('should reuse cell instances across calls', () => { + const table = generateTestTableWithData(1) + const row = table.getRowModel().rows[0]! + + const firstCells = row_getAllCells(row) + const secondCells = row_getAllCells(row) + + expect(secondCells).not.toBe(firstCells) + for (let i = 0; i < firstCells.length; i++) { + expect(secondCells[i]).toBe(firstCells[i]) + } + }) + + it('should preserve cell identity across column order changes', () => { + const table = generateTestTableWithDataAndState(1, { + features: stockFeatures, + }) as Table_Internal & + Table_ColumnOrdering + const row = table.getRowModel().rows[0]! + + const cellsBefore = row.getAllCells() + const cellsByColumnIdBefore = new Map( + cellsBefore.map((cell) => [cell.column.id, cell]), + ) + + table.setColumnOrder(['lastName', 'firstName']) + + const cellsAfter = row.getAllCells() + + expect(cellsAfter).not.toBe(cellsBefore) + expect(cellsAfter[0]?.column.id).toBe('lastName') + expect(cellsAfter[1]?.column.id).toBe('firstName') + for (const cell of cellsAfter) { + expect(cell).toBe(cellsByColumnIdBefore.get(cell.column.id)) + } + }) +}) diff --git a/packages/table-core/tests/unit/features/column-ordering/columnOrderingFeature.utils.test.ts b/packages/table-core/tests/unit/features/column-ordering/columnOrderingFeature.utils.test.ts index c4fddab1be..32f5a55d87 100644 --- a/packages/table-core/tests/unit/features/column-ordering/columnOrderingFeature.utils.test.ts +++ b/packages/table-core/tests/unit/features/column-ordering/columnOrderingFeature.utils.test.ts @@ -1,16 +1,28 @@ import { describe, expect, it, vi } from 'vitest' -import { generateTestTableWithData } from '../../../helpers/generateTestTable' +import { stockFeatures } from '../../../../src' +import { + generateTestTableWithData, + generateTestTableWithDataAndState, +} from '../../../helpers/generateTestTable' import { column_getIndex, column_getIsFirstColumn, column_getIsLastColumn, getDefaultColumnOrderState, orderColumns, + table_getColumnIndexes, table_getOrderColumnsFn, + table_getPinnedVisibleLeafColumns, table_resetColumnOrder, table_setColumnOrder, } from '../../../../src/static-functions' -import type { TableFeatures } from '../../../../src' +import type { Person } from '../../../fixtures/data/types' +import type { + StockFeatures, + TableFeatures, + Table_ColumnOrdering, + Table_Internal, +} from '../../../../src' describe('getDefaultColumnOrderState', () => { it('should return an empty array', () => { @@ -18,6 +30,36 @@ describe('getDefaultColumnOrderState', () => { }) }) +describe('table_getColumnIndexes', () => { + it('should map each visible column id to its index within each region', () => { + const table = generateTestTableWithData(3, { + initialState: { + columnPinning: { + left: ['lastName'], + right: ['age'], + }, + columnVisibility: { + firstName: false, + }, + }, + }) + + const indexes = table_getColumnIndexes(table) + + for (const [key, position] of [ + ['all', undefined], + ['left', 'left'], + ['right', 'right'], + ['center', 'center'], + ] as const) { + const columns = table_getPinnedVisibleLeafColumns(table, position) + expect(indexes[key]).toEqual( + Object.fromEntries(columns.map((col, i) => [col.id, i])), + ) + } + }) +}) + describe('column_getIndex', () => { it('should return correct index for a column', () => { const table = generateTestTableWithData(3) @@ -36,6 +78,44 @@ describe('column_getIndex', () => { expect(column_getIndex(column as any)).toBe(-1) }) + + it('should return the index within the requested pinning region', () => { + const table = generateTestTableWithData(3, { + initialState: { + columnPinning: { + left: ['lastName', 'firstName'], + right: ['age'], + }, + }, + }) + const columns = table.getAllLeafColumns() + const firstName = columns.find((col) => col.id === 'firstName')! + const age = columns.find((col) => col.id === 'age')! + const visits = columns.find((col) => col.id === 'visits')! + + expect(column_getIndex(firstName, 'left')).toBe(1) + expect(column_getIndex(age, 'right')).toBe(0) + expect(column_getIndex(visits, 'center')).toBeGreaterThanOrEqual(0) + expect(column_getIndex(firstName, 'right')).toBe(-1) + expect(column_getIndex(firstName, 'center')).toBe(-1) + }) + + it('should recompute the instance API after column order changes', () => { + const table = generateTestTableWithDataAndState(1, { + features: stockFeatures, + }) as Table_Internal & + Table_ColumnOrdering + + const lastName = table.getColumn('lastName')! + + expect(lastName.getIndex()).toBe(2) + expect(table.getColumnIndexes().all['lastName']).toBe(2) + + table.setColumnOrder(['lastName', 'firstName']) + + expect(lastName.getIndex()).toBe(0) + expect(table.getColumnIndexes().all['lastName']).toBe(0) + }) }) describe('column_getIsFirstColumn', () => { diff --git a/packages/table-core/tests/unit/features/column-pinning/columnPinningFeature.utils.test.ts b/packages/table-core/tests/unit/features/column-pinning/columnPinningFeature.utils.test.ts index e50fd05ebd..b2fcf5bf19 100644 --- a/packages/table-core/tests/unit/features/column-pinning/columnPinningFeature.utils.test.ts +++ b/packages/table-core/tests/unit/features/column-pinning/columnPinningFeature.utils.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' -import { stockFeatures } from '../../../../src' +import { constructTable, coreFeatures, stockFeatures } from '../../../../src' +import { storeReactivityBindings } from '../../../../src/store-reactivity-bindings' import { column_getCanPin, column_getIsPinned, @@ -221,6 +222,49 @@ describe('column_getIsPinned', () => { expect(result).toBe(false) }) + + it('should prefer left when column is pinned in both regions', () => { + const table = generateTestTableWithData(1, { + initialState: { + columnPinning: { + left: ['firstName'], + right: ['firstName'], + }, + }, + }) + const column = table.getColumn('firstName')! + + expect(column_getIsPinned(column)).toBe('left') + }) + + it('should report the pinned region of a group column from its leaf columns', () => { + const table = constructTable({ + features: { + ...coreFeatures, + coreReactivityFeature: storeReactivityBindings(), + }, + columns: [ + { + id: 'name', + header: 'Name', + columns: [ + { accessorKey: 'firstName', id: 'firstName' }, + { accessorKey: 'lastName', id: 'lastName' }, + ], + }, + ], + data: [], + initialState: { + columnPinning: { + left: [], + right: ['lastName'], + }, + }, + } as any) + const groupColumn = table.getAllColumns()[0]! + + expect(column_getIsPinned(groupColumn as any)).toBe('right') + }) }) describe('table_setColumnPinning', () => { @@ -378,6 +422,15 @@ describe('row_getCenterVisibleCells', () => { expect(centerCells.map((cell) => cell.column.id)).not.toContain('lastName') expect(centerCells.length).toBeGreaterThan(0) }) + + it('should return the shared visible cells array when nothing is pinned', () => { + const table = generateTestTableWithData(1, { + features: stockFeatures, + }) as any + const row = table.getRowModel().rows[0]! + + expect(row.getCenterVisibleCells()).toBe(row.getVisibleCells()) + }) }) describe('row_getLeftVisibleCells', () => { @@ -490,6 +543,16 @@ describe('table_getCenterHeaderGroups', () => { expect(centerColumnIds).not.toContain('lastName') expect(headerGroups[0]?.headers.length).toBeGreaterThan(0) }) + + it('should include all visible columns when nothing is pinned', () => { + const table = generateTestTableWithData(1) + + const headerGroups = table_getCenterHeaderGroups(table) + + expect(headerGroups[0]?.headers.map((header) => header.column.id)).toEqual( + table_getVisibleLeafColumns(table).map((col) => col.id), + ) + }) }) describe('table_getLeftLeafColumns', () => { @@ -546,6 +609,12 @@ describe('table_getCenterLeafColumns', () => { expect(centerColumnIds).not.toContain('lastName') expect(leafColumns.length).toBeGreaterThan(0) }) + + it('should return the shared leaf columns array when nothing is pinned', () => { + const table = generateTestTableWithData(1) + + expect(table_getCenterLeafColumns(table)).toBe(table.getAllLeafColumns()) + }) }) describe('table_getPinnedLeafColumns', () => { diff --git a/perf-done.md b/perf-done.md index 441a08fde2..9729c45602 100644 --- a/perf-done.md +++ b/perf-done.md @@ -7,10 +7,14 @@ Entries are sorted by adjusted effectiveness score descending. ## Counts -- **Entries:** 54 -- **Source findings:** 52 +- **Entries:** 58 +- **Source findings:** 56 - **Cross-cutting sweeps:** 2 - 2026-07-03: #102 (C9) moved here from perf-todo.md after the row-model benchmark confirmed and the fix landed. +- 2026-07-07: #68 (A4) moved here from perf-todo.md after implementation. +- 2026-07-07: #76 (A5) moved here from perf-todo.md after implementation. +- 2026-07-07: #85 (A6) moved here from perf-todo.md after implementation. +- 2026-07-07: #92 (C11) moved here from perf-todo.md after implementation. ## Score 9 @@ -800,6 +804,116 @@ Eliminated back-to-back Array method chains across `packages/table-core/src/**` ## Score 7 +## 68. A4: column_getIndex: O(N) findIndex per column, O(N²) cascade per ordering change → table-level index records — Score: 7 + +**Status:** `[x]` done +**Implementation note:** Implemented as proposed: added `table_getColumnIndexes` (single memo building `all`/`center`/`left`/`right` id-to-index records via `makeObjectMap`), registered in `columnOrderingFeature.constructTableAPIs` with the full deps chain (`options.columns`, `columnOrder`, `columnPinning`, `columnVisibility`, `grouping`, `groupedColumnMode`); `column_getIndex` is now a plain prototype fn doing an O(1) record lookup through `callMemoOrStaticFn(column.table, 'getColumnIndexes', ...)`, with `?? -1` preserving miss semantics. One deviation: `buildIndexes` accepts `Column | Column_Internal` (union element type) because `table_getPinnedVisibleLeafColumns` returns a union of the two array types. Also closes the pre-existing `groupedColumnMode` staleness gap flagged in the A1/A2 sweep notes. Regression tests added: region records vs `table_getPinnedVisibleLeafColumns`, per-region `getIndex` lookups (including -1 misses), and instance-API invalidation after `setColumnOrder`. Full table-core unit tests, type checks, build, and size-limit (16.86 kB / 20 kB) pass. + +**Location:** `packages/table-core/src/features/column-ordering/columnOrderingFeature.utils.ts:37–47`; registration at `columnOrderingFeature.ts:36–45` +**Category:** `big-o` + +Hot path: Per state-change (columnOrder/columnPinning/grouping/columnVisibility): all N per-column memos invalidate; each recompute is an O(N) scan → O(N²). Also on first render. (Not per-tick: `columnSizing` is correctly absent from its deps.) `findIndex` with a fresh closure per recompute, O(N) per column. After any ordering-affecting state change, every column recomputes → O(N²) id comparisons (N=500 → 250k). With A1 applied the internal callers disappear, but `getIndex` remains public API. Since the iterated dimension scales with N, a keyed record is warranted. + +**Before** + +```ts +export function column_getIndex< + TFeatures extends TableFeatures, + TData extends RowData, + TValue extends CellData = CellData, +>( + column: Column_Internal, + position?: ColumnPinningPosition | 'center', +) { + const columns = table_getPinnedVisibleLeafColumns(column.table, position) + return columns.findIndex((d) => d.id === column.id) +} +``` + +**After** + +(Table-level index records, all four position keys in ONE memo so there is no single-slot thrash; unmemoized per-column lookup.) + +```ts +// columnOrderingFeature.utils.ts +export function table_getColumnIndexes< + TFeatures extends TableFeatures, + TData extends RowData, +>(table: Table_Internal) { + const buildIndexes = ( + columns: Array>, + ): Record => { + const indexes = makeObjectMap() + for (let i = 0; i < columns.length; i++) { + indexes[columns[i]!.id] = i + } + return indexes + } + + return { + all: buildIndexes(table_getPinnedVisibleLeafColumns(table)), + center: buildIndexes(table_getPinnedVisibleLeafColumns(table, 'center')), + left: buildIndexes(table_getPinnedVisibleLeafColumns(table, 'left')), + right: buildIndexes(table_getPinnedVisibleLeafColumns(table, 'right')), + } +} + +export function column_getIndex< + TFeatures extends TableFeatures, + TData extends RowData, + TValue extends CellData = CellData, +>( + column: Column_Internal, + position?: ColumnPinningPosition | 'center', +) { + const indexes = callMemoOrStaticFn( + column.table, + 'getColumnIndexes', + table_getColumnIndexes, + ) + const key = + position === 'left' + ? 'left' + : position === 'right' + ? 'right' + : position === 'center' + ? 'center' + : 'all' + return indexes[key][column.id] ?? -1 +} +``` + +```ts +// columnOrderingFeature.ts + assignTableAPIs('columnOrderingFeature', table, { + table_getColumnIndexes: { + fn: () => table_getColumnIndexes(table), + memoDeps: () => [ + table.options.columns, + table.atoms.columnOrder?.get(), + table.atoms.columnPinning?.get(), + table.atoms.columnVisibility?.get(), + table.atoms.grouping?.get(), + table.options.groupedColumnMode, + ], + }, + // ... + }) + // column_getIndex: drop memoDeps, register as a plain prototype fn: + column_getIndex: { + fn: (column, position) => column_getIndex(column, position), + }, +``` + +Deps coverage: same transitive chain as A1 (columns, columnOrder, columnPinning, columnVisibility, grouping, groupedColumnMode); no `columnSizing`, so resize ticks never invalidate index records. Keep this memo SEPARATE from A1's offsets memo. `?? -1` preserves miss semantics. + +**Big-O:** O(N²) → O(N) per ordering state-change (N=500: 250k comparisons + N closures → 500 loop iterations, 4 record allocs). Removes N per-instance `_memo_getIndex` closures. + +**Risk:** Low. The proposed deps are a strict superset of the current ones (adds `options.columns` and `groupedColumnMode`, closing a small `groupedColumnMode` staleness gap). Duplicate column ids would flip first-wins to last-wins; ids are unique by construction, or guard with `if (indexes[id] === undefined)`. +**Verification:** CONFIRMED (deps superset verified; no import cycle; keep offsets/index memos separate exactly as proposed). + +--- + ## 1. `memo()` deps equality uses `.some()` callback per call — Score: 7 **Status:** `[x]` done @@ -1431,6 +1545,76 @@ Lowercasing an already-lowercased string hits the V8 no-change fast path (no all ## Score 6 +## 76. A5: column_getIsPinned: unmemoized per-call .map allocation on a per-cell render path — Score: 6 + +**Status:** `[x]` done +**Implementation note:** Implemented exactly as proposed: replaced the `.map` + two `.some` closures with two classic indexed loops over `column.getLeafColumns()`, returning `'left'` on first left hit (preserving left-before-right precedence) and skipping the right scan entirely on a left hit. Zero allocations per call. Added regression tests for the both-regions precedence case and the group-column multi-leaf case (group column reports `'right'` when a leaf is pinned right). Full table-core unit tests (438), type checks, build, and size-limit (16.85 kB / 20 kB) pass. + +**Location:** `packages/table-core/src/features/column-pinning/columnPinningFeature.utils.ts:135–151`, registered without memo at `columnPinningFeature.ts:74–76` +**Category:** `allocation`, `render-path` + +Hot path: Per render: sticky-pinning layouts call `column.getIsPinned()` per visible cell and header (R_vis × N per render; see `examples/react/column-pinning-sticky`), including during resize-driven re-renders. Every call allocates an ids array via `.map` plus two `.some` closures; for leaf columns (`getLeafColumns()` returns `[column]`) that is 3 allocations to do 2 tiny `.includes` checks, and both sides always run. At 50 visible rows × 100 columns = 5,000 calls per render → ~15k transient allocations per render pass, per tick when resize re-renders. The `left`/`right` arrays themselves are tiny (1-3), so `.includes` is optimal; the waste is the closures/array, not the scan. + +**Before** + +```ts +export function column_getIsPinned< + TFeatures extends TableFeatures, + TData extends RowData, + TValue extends CellData = CellData, +>( + column: Column_Internal, +): ColumnPinningPosition | false { + const leafColumnIds = column.getLeafColumns().map((d) => d.id) + + const { left, right } = + column.table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() + + const isLeft = leafColumnIds.some((d) => left.includes(d)) + const isRight = leafColumnIds.some((d) => right.includes(d)) + + return isLeft ? 'left' : isRight ? 'right' : false +} +``` + +**After** + +(classic loops, no closures, right side skipped after a left hit) + +```ts +export function column_getIsPinned< + TFeatures extends TableFeatures, + TData extends RowData, + TValue extends CellData = CellData, +>( + column: Column_Internal, +): ColumnPinningPosition | false { + const leafColumns = column.getLeafColumns() + + const { left, right } = + column.table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() + + for (let i = 0; i < leafColumns.length; i++) { + if (left.includes(leafColumns[i]!.id)) { + return 'left' + } + } + for (let i = 0; i < leafColumns.length; i++) { + if (right.includes(leafColumns[i]!.id)) { + return 'right' + } + } + return false +} +``` + +**Big-O:** Same O(leaf × pin) worst case, but 0 allocations per call vs 3, and early return on first hit. ~15k allocations per render eliminated at 50×100 scale. `column_getPinnedIndex` calls this and benefits too. + +**Risk:** Very low. Return values and left-before-right precedence provably identical, including the group-column multi-leaf case. +**Verification:** CONFIRMED. + +--- + ## 102. C9: getFilteredSelectedRowModel / getGroupedSelectedRowModel read the CORE row model while memoDeps declare filtered/sorted models (bug) — Score: 6 (bug) **Status:** `[x]` done @@ -1837,6 +2021,106 @@ Typecheck verified clean after the sweep (`pnpm tsc --noEmit` passes). ## Score 5 +## 92. C11: row_getAllCells reconstructs all cell instances whenever leaf-column array identity changes: per-row cell cache keyed by column instance (WeakMap REQUIRED) — Score: 5 + +**Status:** `[x]` done +**Implementation note:** Implemented as proposed with the mandated `WeakMap`, lazily created on first `row_getAllCells` call (unrendered rows never allocate one). `_cellsCache` added to `Row_CoreProperties` as an optional field. All three premises re-verified in code before implementing: rows memoized on `[options.data]` only (survive columns swaps), column instances keyed on `[options.columns]` only (stable across reorders), cells hold only `column`/`id`/`row` on a shared prototype. One required follow-on fix the entry did not anticipate: `copyInstancePropertiesWithoutMemos` (used by sorted-model branch clones and selection parent clones) copied `_cellsCache` onto clone rows, making clones return cells whose `.row` pointed at the source row AND sharing one WeakMap between two rows; `_cellsCache` is now excluded there alongside `_memo_*` keys (caught by the two existing clone regression tests). New tests: cell-instance reuse across calls, cell identity preserved across `setColumnOrder` (array rebuilt, instances reused, new order). Full table-core unit tests (444), type checks, build, and size-limit (16.94 kB / 20 kB) pass. + +**Location:** `packages/table-core/src/core/rows/coreRowsFeature.utils.ts:167–179` + registration `coreRowsFeature.ts:26–29` +**Category:** `allocation` + +Hot path: per rendered row after any `columnOrder`, `grouping`, `groupedColumnMode`, or `columns` change: `getAllLeafColumns` memoDeps invalidate on all four, cascading into `row_getAllCells` (dep `[row.table.getAllLeafColumns()]`), then `row_getVisibleCells`, `row_getAllCellsByColumnId`, and `cell_getContext` (new instances → new context objects and closures). Reordering columns (drag) or toggling grouping does not change any (row, column) pair, yet every rendered row rebuilds N cell objects; each cell later rebuilds its context (1 object + 2 closures + a lazy tableMemo per instance). At 100 rendered rows × 100 columns per drag step: ~10k cells + ~10k contexts + ~20k closures churned, and all cell identities change, defeating adapter-level cell memoization (full DOM-level cell re-render). Column instances are stable across order/grouping changes (`getAllColumns` deps are `[options.columns]` only; ordering reorders existing instances), so instance-keyed reuse works. + +**Cross-refs / gating:** WeakMap is REQUIRED, not optional (see risk). Legacy entry #20 (perf-todo.md) records the underlying observation this rationale depends on: `createCoreRowModel` memoDeps are `[table.options.data]` only, so rows survive `columns` swaps. + +**Before** + +```ts +const columns = row.table.getAllLeafColumns() +const cells: Array> = new Array(columns.length) +for (let i = 0; i < columns.length; i++) { + cells[i] = constructCell(columns[i]!, row, row.table) +} +return cells +``` + +**After** (verifier-corrected: MUST be a `WeakMap`, not a `Map`) + +```ts +const columns = row.table.getAllLeafColumns() +let cache = row._cellsCache +if (!cache) { + cache = row._cellsCache = new WeakMap< + Column, + Cell + >() +} +const cells: Array> = new Array(columns.length) +for (let i = 0; i < columns.length; i++) { + const column = columns[i]! + let cell = cache.get(column) + if (!cell) { + cell = constructCell(column, row, row.table) + cache.set(column, cell) + } + cells[i] = cell +} +return cells +``` + +**Big-O:** Column reorder / grouping toggle: rendered-rows × N cell constructions + context/memo churn → 0 reconstructions (array reorder only); preserves cell identity so adapter cell memoization survives reorders. Steady-state renders unchanged (the outer memo still short-circuits). + +**Risk (WeakMap rationale):** The finder's claim "rows are rebuilt on data/columns change" is FALSE for columns: `createCoreRowModel` memoDeps are `[table.options.data]` ONLY, so rows survive a `columns` swap. A plain `Map` would accumulate one generation of stale column→cell entries per columns-identity change; with the common user error of unstable `columns` per render, that is an unbounded R×N leak. `WeakMap` ephemeron semantics collect the circular key→cell→column entries once old column instances are unreachable. Cells hold only `column`/`id`/`row` + shared prototype (verified); no fresh-cell assumption found in scope. Behavior delta to flag: cell identity now survives leaf-array changes (an adapter memoization win). +**Verification:** AMENDED: WeakMap made mandatory, with the createCoreRowModel-deps rationale (`[data]` only; rows survive columns swaps). + +--- + +## 85. A6: Missing empty-pinning short-circuits in center-partition functions — Score: 5 + +**Status:** `[x]` done +**Implementation note:** Implemented as proposed at all three sites. `row_getCenterVisibleCells` returns the shared `getVisibleCells` array when both pinning sides are empty; `table_getCenterLeafColumns` returns `table.getAllLeafColumns()` directly; `table_getCenterHeaderGroups` skips the spread + filter and passes the visible leaf columns straight to `buildHeaderGroups`. The identity-return behavior delta is locked in by regression tests (`getCenterVisibleCells` === `getVisibleCells` via instance memos with stockFeatures; `table_getCenterLeafColumns` === `getAllLeafColumns`; center header groups cover all visible columns when unpinned). Full table-core unit tests (441), type checks, build, and size-limit (16.91 kB / 20 kB) pass. + +**Location:** `packages/table-core/src/features/column-pinning/columnPinningFeature.utils.ts:189–202` (row_getCenterVisibleCells), `:738–746` (table_getCenterLeafColumns), `:430–450` (table_getCenterHeaderGroups) +**Category:** `big-o` (short-circuit), `allocation` + +Hot path: per state-change recompute (pinning/visibility/order/columns) × R rows for the row variant; the vast majority of tables have EMPTY pinning yet still pay the copy. The siblings `row_getLeftVisibleCells`/`row_getRightVisibleCells` already have the empty fast path (`if (!left.length) return []`, lines 221/257); the center variants do not. With no pinning (the default), every recompute still allocates the spread array plus a filtered COPY of all N cells per row, and returns a new array identity where returning `allCells` directly would preserve reference equality for downstream consumers. `.includes` over the tiny `leftAndRight` is fine per doctrine; the issue is only the missing short-circuit and the copies. + +**Cross-refs / gating:** Skipped entry #36 (perf-skipped.md) deliberately rejected the Set conversion for the tiny `[...left, ...right]` arrays; that decision stands. This entry is the doctrine-compliant alternative: the empty-pinning short-circuit plus identity preservation, which needs no data-structure change. + +**Before** + +```ts +const allCells = callMemoOrStaticFn(row, 'getVisibleCells', row_getVisibleCells) +const { left, right } = + row.table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() +const leftAndRight: Array = [...left, ...right] +return allCells.filter((d) => !leftAndRight.includes(d.column.id)) +``` + +(the other two use the identical `[...left, ...right]` + `.filter` shape) + +**After** + +```ts +const { left, right } = + row.table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() +const allCells = callMemoOrStaticFn(row, 'getVisibleCells', row_getVisibleCells) +if (!left.length && !right.length) { + return allCells +} +const leftAndRight: Array = [...left, ...right] +return allCells.filter((d) => !leftAndRight.includes(d.column.id)) +``` + +For `table_getCenterLeafColumns`: `if (!left.length && !right.length) return table.getAllLeafColumns()`. For `table_getCenterHeaderGroups`: skip the spread+filter when both are empty and pass `leafColumns` straight to `buildHeaderGroups`. + +**Big-O:** In the unpinned case: O(N) filter copy + spread per recompute → O(1). Row variant: R × (1 array alloc + N includes-checks + N-element copy) per pinning-adjacent state change eliminated. Referential identity of `allCells` preserved, so downstream dep tuples keyed on the cells array stop invalidating spuriously. + +**Risk:** Very low. Returning the shared array matches the `table_getPinnedVisibleLeafColumns(table, undefined)` precedent; `buildHeaderGroups` does not mutate `columnsToGroup` (verified: it only `.map`s it); no in-repo mutation of the returned arrays. +**Verification:** CONFIRMED, demoted 6 → 5 (fires only for pinning-layout tables that happen to have empty pinning; per state-change, not per tick). + +--- + ## 13. `buildHeaderGroups.findMaxDepth` allocates intermediate filtered arrays — Score: 5 **Status:** `[x]` done diff --git a/perf-skipped.md b/perf-skipped.md index ec22754523..4b6f907ee9 100644 --- a/perf-skipped.md +++ b/perf-skipped.md @@ -7,8 +7,8 @@ Entries are sorted by adjusted effectiveness score descending. ## Counts -- **Entries:** 12 -- **Source findings:** 12 +- **Entries:** 16 +- **Source findings:** 16 - **Cross-cutting sweeps:** 0 ## Score 1 @@ -314,6 +314,72 @@ Both walk the `sorting` array; called for every visible sortable column on every ## Score 0 +## 28. Row filter state reset allocates even when already reset (superseded by B2: rowModel-marker skip of the O(R) tag-map reset) — Score: 7 + +**Status:** `[-]` skipped + +**Adjusted score:** 0 +**Original score:** 7 +**Score note:** Implementation discarded after review; the required row-model dirty-marker bookkeeping was judged too subtle for the maintenance cost. +**Implementation note:** Attempted and then discarded. The performance claim is legitimate on large unfiltered data updates: fresh rows already receive empty `columnFilters`/`columnFiltersMeta` maps from `columnFilteringFeature.initRowInstanceData`, so the empty-filter branch's O(R) reset and 2R map allocations are waste. However, preserving correctness for active-filters → no-filters over the same row objects requires tracking exactly which pre-filtered row model objects contain stale tags. Both own-property markers and `WeakSet`/closure marker variants made the code harder to reason about than the local win justified. Leave the straightforward reset in place unless a simpler design emerges. + +**Location:** `src/features/column-filtering/createFilteredRowModel.ts:57–67` (plus `columnFilteringFeature.ts:72–75`) +**Category:** `big-o` (short-circuit), `allocation` + +**Verification:** AMENDED: verifier identified the rowModel-marker variant as the safe shape, but implementation review rejected carrying that marker complexity. + +--- + +## 69. B5: Per-row early exit across filters once the row has failed, gated on faceting absence — Score: 7 + +**Status:** `[-]` skipped + +**Adjusted score:** 0 +**Original score:** 7 +**Score note:** Implementation discarded after review; partial per-row filter tags and faceting-gated control flow were judged too complex for the hot path. +**Implementation note:** Attempted and then discarded. The optimization can save filterFn calls after a strict `false` when no faceted row model is registered, but the implementation makes `row.columnFilters`/`columnFiltersMeta` intentionally partial on failed rows and adds a faceting-specific branch inside the tagging loop. Although the verifier proved the in-repo readers short-circuit safely and the faceting gate is necessary, the resulting behavior surface was not accepted. Keep the simpler full-tagging pass for now. + +**Location:** `packages/table-core/src/features/column-filtering/createFilteredRowModel.ts:121–167` +**Category:** `big-o` (short-circuit) + +**Verification:** CONFIRMED by audit, but skipped by implementation review due to maintainability/observable partial-tag concerns. + +--- + +## 77. B4: Per-row-per-filter addMeta closures → one cursor closure — Score: 6 + +**Status:** `[-]` skipped + +**Adjusted score:** 0 +**Original score:** 6 +**Score note:** Implementation discarded after review; the cursor callback narrows public `addMeta` behavior and is easy to misuse. +**Implementation note:** Attempted and then discarded. Reusing one cursor-style `addMeta` callback removes R×F closure allocations and the dead `columnFiltersMeta` guard, but it relies on `addMeta` being called synchronously during the filterFn invocation. That sync-only contract is defensible, but it changes the practical behavior of a public callback if user filterFns store `addMeta` and call it later. The resulting mutable-cursor code was not accepted for this API surface. Keep per-call closures unless the public contract is explicitly redesigned. + +**Location:** `packages/table-core/src/features/column-filtering/createFilteredRowModel.ts:116–168` +**Category:** `allocation` + +**Verification:** CONFIRMED by audit, but skipped by implementation review due to public callback contract/maintainability concerns. + +--- + +## 59. `table.getAllLeafColumns()` is called many places per row-model build — Score: 4 + +**Status:** `[-]` skipped + +**Adjusted score:** 0 +**Original score:** 4 +**Score note:** Audit-only entry; current source already memoizes `getAllLeafColumns()` on the complete leaf-column identity inputs. +**Implementation note:** Audited current source and found no row-model lifecycle dep churn to optimize. `coreColumnsFeature` registers `table_getAllLeafColumns` with deps `[columnOrder, grouping, options.columns, groupedColumnMode]`, exactly the inputs that can change the leaf-column list or order. Row-model rebuild inputs such as `data`, `columnFilters`, `globalFilter`, grouping row-model state, and faceting row-model state do not invalidate this memo. Hot-path consumers (`row_getAllCells`, `createFilteredRowModel` global-filter column discovery, faceted min/max/unique values, grouped row models, and pinning/visibility derived lists) either consume the table-level memo directly or depend on its array identity through their own memos. The known stale derived-column dependency class referenced by this entry was fixed separately in #67/A2 and #16, where visibility/header derived memos were given the missing grouping/groupedColumnMode coverage. No standalone implementation remains for #59. + +**Location:** filterFns, faceting, grouping, pinning, global filtering +**Category:** `memoization` + +`getAllLeafColumns()` is memoized at the table level, but its deps are sometimes computed inline (see #16 type defects). Verify the memo holds across the row-model rebuild lifecycle. If it doesn't, this is the most-leveraged optimization in the package. + +**Risk:** Already memoized in `coreColumnsFeature`; just audit for accidental dep churn. + +--- + ## 108. B22: Row-model memoDeps systematically omit options they read (observation) — Score: 4 **Status:** `[-]` skipped diff --git a/perf-todo.md b/perf-todo.md index dd76a7cab5..9d096497fd 100644 --- a/perf-todo.md +++ b/perf-todo.md @@ -7,10 +7,14 @@ Entries are sorted by adjusted effectiveness score descending. ## Counts -- **Entries:** 83 -- **Source findings:** 83 +- **Entries:** 75 +- **Source findings:** 75 - **Cross-cutting sweeps:** 0 - 2026-07-03: #102 (C9) completed and moved to perf-done.md. +- 2026-07-07: #68 (A4) completed and moved to perf-done.md. +- 2026-07-07: #76 (A5) completed and moved to perf-done.md. +- 2026-07-07: #85 (A6) completed and moved to perf-done.md. +- 2026-07-07: #92 (C11) completed and moved to perf-done.md. ## Score 8 @@ -48,240 +52,6 @@ const stateStore = useSelector(table.store, selector, { compare: shallow }) ## Score 7 -## 28. Row filter state reset allocates even when already reset (superseded by B2: rowModel-marker skip of the O(R) tag-map reset) — Score: 7 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `src/features/column-filtering/createFilteredRowModel.ts:57–67` (plus `columnFilteringFeature.ts:72–75`) -**Category:** `big-o` (short-circuit), `allocation` - -Original #28 (score 1) proposed skipping the `row.columnFilters = {}` write when already an empty object. The 2026-07-01 audit (B2, score 7) supersedes it with the full analysis. Hot path: every `data` change (and every transition to empty filters) on tables with the filtered model registered but no active filters. `columnFilteringFeature.initRowInstanceData` already assigns fresh `columnFilters`/`columnFiltersMeta` maps in `constructRow`, so whenever the recompute is triggered by a data change (fresh rows), the empty-filter reset loop is pure waste: O(R) iterations + 2R `Object.create(null)` allocations (200k allocs at R=100k). The loop is only needed to clear stale tags when transitioning active-filters → no-filters over the SAME row objects. - -**Before** - -```ts -if (!rowModel.rows.length || (!columnFilters?.length && !globalFilter)) { - const flatRows = rowModel.flatRows as Array< - Row & Partial> - > - for (let i = 0; i < flatRows.length; i++) { - const row = flatRows[i]! - row.columnFilters = makeObjectMap() - row.columnFiltersMeta = makeObjectMap() - } - return rowModel -} -``` - -**After** (verifier's rowModel-marker variant, which replaces the finder's closure-flag: mark the exact model object whose flatRows were tagged, and clear only when the marker is present — the marker travels with the tagged rowModel object) - -```ts -// after the tagging loop completes in the filtered branch: -;(rowModel as any)._filterTagsDirty = true -// (equivalently: a module-level WeakSet of tagged models) - -// in the empty-filter branch: -if (!rowModel.rows.length || (!columnFilters?.length && !globalFilter)) { - if ((rowModel as any)._filterTagsDirty) { - const flatRows = rowModel.flatRows as Array< - Row & Partial> - > - for (let i = 0; i < flatRows.length; i++) { - const row = flatRows[i]! - row.columnFilters = makeObjectMap() - row.columnFiltersMeta = makeObjectMap() - } - delete (rowModel as any)._filterTagsDirty - } - return rowModel -} -``` - -memoDeps unchanged (`[preFilteredRowModel, columnFilters, globalFilter]`). - -**Big-O:** O(R) → O(1) on the unfiltered data-change path: ~100k iterations + 200k null-proto object allocations saved per data update at R=100k (~5–10ms + GC pressure). - -**Risk:** The marker travels with the exact object whose flatRows were tagged, so it stays correct under every sequence, including custom `features.coreRowModel` implementations with multi-slot caching (where the finder's closure flag would desync when an old tagged rowModel object resurfaces) and the filters-active-over-empty-rowModel case. No signature threading needed. -**Verification:** AMENDED: the verifier's rowModel-marker variant replaces the finder's closure-flag proposal (closure flag desyncs under custom core row models and goes conservatively wrong over empty models). - ---- - -## 68. A4: column_getIndex: O(N) findIndex per column, O(N²) cascade per ordering change → table-level index records — Score: 7 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `packages/table-core/src/features/column-ordering/columnOrderingFeature.utils.ts:37–47`; registration at `columnOrderingFeature.ts:36–45` -**Category:** `big-o` - -Hot path: Per state-change (columnOrder/columnPinning/grouping/columnVisibility): all N per-column memos invalidate; each recompute is an O(N) scan → O(N²). Also on first render. (Not per-tick: `columnSizing` is correctly absent from its deps.) `findIndex` with a fresh closure per recompute, O(N) per column. After any ordering-affecting state change, every column recomputes → O(N²) id comparisons (N=500 → 250k). With A1 applied the internal callers disappear, but `getIndex` remains public API. Since the iterated dimension scales with N, a keyed record is warranted. - -**Before** - -```ts -export function column_getIndex< - TFeatures extends TableFeatures, - TData extends RowData, - TValue extends CellData = CellData, ->( - column: Column_Internal, - position?: ColumnPinningPosition | 'center', -) { - const columns = table_getPinnedVisibleLeafColumns(column.table, position) - return columns.findIndex((d) => d.id === column.id) -} -``` - -**After** - -(Table-level index records, all four position keys in ONE memo so there is no single-slot thrash; unmemoized per-column lookup.) - -```ts -// columnOrderingFeature.utils.ts -export function table_getColumnIndexes< - TFeatures extends TableFeatures, - TData extends RowData, ->(table: Table_Internal) { - const buildIndexes = ( - columns: Array>, - ): Record => { - const indexes = makeObjectMap() - for (let i = 0; i < columns.length; i++) { - indexes[columns[i]!.id] = i - } - return indexes - } - - return { - all: buildIndexes(table_getPinnedVisibleLeafColumns(table)), - center: buildIndexes(table_getPinnedVisibleLeafColumns(table, 'center')), - left: buildIndexes(table_getPinnedVisibleLeafColumns(table, 'left')), - right: buildIndexes(table_getPinnedVisibleLeafColumns(table, 'right')), - } -} - -export function column_getIndex< - TFeatures extends TableFeatures, - TData extends RowData, - TValue extends CellData = CellData, ->( - column: Column_Internal, - position?: ColumnPinningPosition | 'center', -) { - const indexes = callMemoOrStaticFn( - column.table, - 'getColumnIndexes', - table_getColumnIndexes, - ) - const key = - position === 'left' - ? 'left' - : position === 'right' - ? 'right' - : position === 'center' - ? 'center' - : 'all' - return indexes[key][column.id] ?? -1 -} -``` - -```ts -// columnOrderingFeature.ts - assignTableAPIs('columnOrderingFeature', table, { - table_getColumnIndexes: { - fn: () => table_getColumnIndexes(table), - memoDeps: () => [ - table.options.columns, - table.atoms.columnOrder?.get(), - table.atoms.columnPinning?.get(), - table.atoms.columnVisibility?.get(), - table.atoms.grouping?.get(), - table.options.groupedColumnMode, - ], - }, - // ... - }) - // column_getIndex: drop memoDeps, register as a plain prototype fn: - column_getIndex: { - fn: (column, position) => column_getIndex(column, position), - }, -``` - -Deps coverage: same transitive chain as A1 (columns, columnOrder, columnPinning, columnVisibility, grouping, groupedColumnMode); no `columnSizing`, so resize ticks never invalidate index records. Keep this memo SEPARATE from A1's offsets memo. `?? -1` preserves miss semantics. - -**Big-O:** O(N²) → O(N) per ordering state-change (N=500: 250k comparisons + N closures → 500 loop iterations, 4 record allocs). Removes N per-instance `_memo_getIndex` closures. - -**Risk:** Low. The proposed deps are a strict superset of the current ones (adds `options.columns` and `groupedColumnMode`, closing a small `groupedColumnMode` staleness gap). Duplicate column ids would flip first-wins to last-wins; ids are unique by construction, or guard with `if (indexes[id] === undefined)`. -**Verification:** CONFIRMED (deps superset verified; no import cycle; keep offsets/index memos separate exactly as proposed). - ---- - -## 69. B5: Per-row early exit across filters once the row has failed, gated on faceting absence — Score: 7 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `packages/table-core/src/features/column-filtering/createFilteredRowModel.ts:121–167` (decision loop at 170–180) -**Category:** `big-o` (short-circuit) - -Hot path: Per state-change: tagging pass over all R flatRows, every filter keystroke. Every row is evaluated against ALL column filters, and the global loop runs regardless of column-filter outcomes. The full tag matrix is only consumed by `createFacetedRowModel` (each faceted column excludes its own filter and needs every OTHER filter's verdict). The final decision (`filterRowsImpl`) checks ids in tagging order, so it hits the first `false` before any missing later tag: evaluation after the first failure is pure waste whenever faceting is not registered. With a selective first filter (90% fail) and F=3+global, that is ~R×(F-1) wasted filterFn invocations plus whole global-filter scans for already-failed rows. - -**Before** - -```ts - if (resolvedColumnFilters.length) { - for (let j = 0; j < resolvedColumnFilters.length; j++) { - const currentColumnFilter = resolvedColumnFilters[j]! - const id = currentColumnFilter.id - - // Tag the row with the column filter state - row.columnFilters[id] = currentColumnFilter.filterFn( - row, - id, - // ... -``` - -**After** - -```ts -const needsAllFilterTags = !!table.options.features.facetedRowModel - -// inside the flatRows loop: -let failed = false -if (resolvedColumnFilters.length) { - for (let j = 0; j < resolvedColumnFilters.length; j++) { - const currentColumnFilter = resolvedColumnFilters[j]! - const id = currentColumnFilter.id - metaId = id - const pass = currentColumnFilter.filterFn( - row, - id, - currentColumnFilter.resolvedValue, - addMeta, - ) - row.columnFilters[id] = pass - if (pass === false) { - failed = true - if (!needsAllFilterTags) break - } - } -} - -if (resolvedGlobalFilters.length && !(failed && !needsAllFilterTags)) { - // ...existing global loop... -} -``` - -`filterRowsImpl` is untouched: id order in `filterableIds` matches `resolvedColumnFilters` order, so the first stored `false` short-circuits before any untagged id is read. - -**Big-O:** Worst case unchanged O(R×F); typical selective-filter case drops to ~O(R×1) filterFn calls. At R=100k, F=3, 90% fail-on-first: ~180k filterFn calls saved per keystroke (~10-30ms), plus skipped global-filter scans. - -**Risk:** Verifier completed the consumer sweep: tags are read only by the two `filterRowsImpl`s and the faceted model; `columnFiltersMeta` has zero in-repo readers; the read-order proof holds even with missing-column filter ids; strict `pass === false` matches `filterRowsImpl` semantics exactly; `!!table.options.features.facetedRowModel` is the correct gate (the fallback faceted path reads no tags). Residual documented delta: public `row.columnFilters`/`columnFiltersMeta` become partial on failed rows. Do NOT apply the skip when faceting is registered. -**Verification:** CONFIRMED (consumer sweep and read-order proof completed by verifier). - ---- - ## 70. B19: constructRow allocates Object.values(table.\_features) and probes all features per row — Score: 7 **Status:** `[ ]` not started @@ -605,143 +375,6 @@ Deps verified complete against transitive reads (`row_getIsExpanded` → expande --- -## 76. A5: column_getIsPinned: unmemoized per-call .map allocation on a per-cell render path — Score: 6 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `packages/table-core/src/features/column-pinning/columnPinningFeature.utils.ts:135–151`, registered without memo at `columnPinningFeature.ts:74–76` -**Category:** `allocation`, `render-path` - -Hot path: Per render: sticky-pinning layouts call `column.getIsPinned()` per visible cell and header (R_vis × N per render; see `examples/react/column-pinning-sticky`), including during resize-driven re-renders. Every call allocates an ids array via `.map` plus two `.some` closures; for leaf columns (`getLeafColumns()` returns `[column]`) that is 3 allocations to do 2 tiny `.includes` checks, and both sides always run. At 50 visible rows × 100 columns = 5,000 calls per render → ~15k transient allocations per render pass, per tick when resize re-renders. The `left`/`right` arrays themselves are tiny (1-3), so `.includes` is optimal; the waste is the closures/array, not the scan. - -**Before** - -```ts -export function column_getIsPinned< - TFeatures extends TableFeatures, - TData extends RowData, - TValue extends CellData = CellData, ->( - column: Column_Internal, -): ColumnPinningPosition | false { - const leafColumnIds = column.getLeafColumns().map((d) => d.id) - - const { left, right } = - column.table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() - - const isLeft = leafColumnIds.some((d) => left.includes(d)) - const isRight = leafColumnIds.some((d) => right.includes(d)) - - return isLeft ? 'left' : isRight ? 'right' : false -} -``` - -**After** - -(classic loops, no closures, right side skipped after a left hit) - -```ts -export function column_getIsPinned< - TFeatures extends TableFeatures, - TData extends RowData, - TValue extends CellData = CellData, ->( - column: Column_Internal, -): ColumnPinningPosition | false { - const leafColumns = column.getLeafColumns() - - const { left, right } = - column.table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() - - for (let i = 0; i < leafColumns.length; i++) { - if (left.includes(leafColumns[i]!.id)) { - return 'left' - } - } - for (let i = 0; i < leafColumns.length; i++) { - if (right.includes(leafColumns[i]!.id)) { - return 'right' - } - } - return false -} -``` - -**Big-O:** Same O(leaf × pin) worst case, but 0 allocations per call vs 3, and early return on first hit. ~15k allocations per render eliminated at 50×100 scale. `column_getPinnedIndex` calls this and benefits too. - -**Risk:** Very low. Return values and left-before-right precedence provably identical, including the group-column multi-leaf case. -**Verification:** CONFIRMED. - ---- - -## 77. B4: Per-row-per-filter addMeta closures → one cursor closure — Score: 6 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `packages/table-core/src/features/column-filtering/createFilteredRowModel.ts:116–168` (closures at 131–137 and 151–157) -**Category:** `allocation` - -Hot path: Per state-change: full tagging pass over all flatRows on every filter keystroke. A fresh `addMeta` closure is allocated for every (row × filter) pair: R×F closures per recompute (200k at R=100k, F=2), plus the same pattern per (row × globally-filterable-column). The `if (!row.columnFiltersMeta)` guard inside both closures is provably dead (`row.columnFiltersMeta` is unconditionally assigned before any filterFn runs). - -**Before** - -```ts -row.columnFilters[id] = currentColumnFilter.filterFn( - row, - id, - currentColumnFilter.resolvedValue, - (filterMeta) => { - if (!row.columnFiltersMeta) { - row.columnFiltersMeta = makeObjectMap() - } - row.columnFiltersMeta[id] = filterMeta - }, -) -``` - -**After** - -(one reusable closure over mutable cursor variables; the meta contract is synchronous) - -```ts -let metaRow: (typeof flatRows)[number] -let metaId: string -const addMeta = (filterMeta: any) => { - metaRow.columnFiltersMeta![metaId] = filterMeta -} - -for (let i = 0; i < flatRows.length; i++) { - const row = flatRows[i]! - row.columnFilters = makeObjectMap() - row.columnFiltersMeta = makeObjectMap() - metaRow = row - - if (resolvedColumnFilters.length) { - for (let j = 0; j < resolvedColumnFilters.length; j++) { - const currentColumnFilter = resolvedColumnFilters[j]! - const id = currentColumnFilter.id - metaId = id - row.columnFilters[id] = currentColumnFilter.filterFn( - row, - id, - currentColumnFilter.resolvedValue, - addMeta, - ) - } - } - // same treatment for the resolvedGlobalFilters loop -} -``` - -**Big-O:** Same Big-O; ~R×F + R×(global cols evaluated) closure allocations removed per filter state change (200k+ at scale, several ms + GC pressure). Dead-branch removal saves a load+test per meta call. - -**Risk:** Breaks only if a user filterFn stores `addMeta` and invokes it after the loop advances (asynchronous meta). Built-in filterFns never even declare the addMeta param. Must be applied to BOTH loops (131–137 and 151–157), and the sync-only addMeta contract documented, since it narrows a technically public FilterFn API behavior. -**Verification:** CONFIRMED (dead guard proven; both-loops + contract-documentation requirements added). - ---- - ## 79. B12: leafRows eagerly flattened for every group at depth ≥ 1 even if never aggregated — Score: 6 **Status:** `[ ]` not started @@ -1284,52 +917,6 @@ return { --- -## 85. A6: Missing empty-pinning short-circuits in center-partition functions — Score: 5 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `packages/table-core/src/features/column-pinning/columnPinningFeature.utils.ts:189–202` (row_getCenterVisibleCells), `:738–746` (table_getCenterLeafColumns), `:430–450` (table_getCenterHeaderGroups) -**Category:** `big-o` (short-circuit), `allocation` - -Hot path: per state-change recompute (pinning/visibility/order/columns) × R rows for the row variant; the vast majority of tables have EMPTY pinning yet still pay the copy. The siblings `row_getLeftVisibleCells`/`row_getRightVisibleCells` already have the empty fast path (`if (!left.length) return []`, lines 221/257); the center variants do not. With no pinning (the default), every recompute still allocates the spread array plus a filtered COPY of all N cells per row, and returns a new array identity where returning `allCells` directly would preserve reference equality for downstream consumers. `.includes` over the tiny `leftAndRight` is fine per doctrine; the issue is only the missing short-circuit and the copies. - -**Cross-refs / gating:** Skipped entry #36 (perf-skipped.md) deliberately rejected the Set conversion for the tiny `[...left, ...right]` arrays; that decision stands. This entry is the doctrine-compliant alternative: the empty-pinning short-circuit plus identity preservation, which needs no data-structure change. - -**Before** - -```ts -const allCells = callMemoOrStaticFn(row, 'getVisibleCells', row_getVisibleCells) -const { left, right } = - row.table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() -const leftAndRight: Array = [...left, ...right] -return allCells.filter((d) => !leftAndRight.includes(d.column.id)) -``` - -(the other two use the identical `[...left, ...right]` + `.filter` shape) - -**After** - -```ts -const { left, right } = - row.table.atoms.columnPinning?.get() ?? getDefaultColumnPinningState() -const allCells = callMemoOrStaticFn(row, 'getVisibleCells', row_getVisibleCells) -if (!left.length && !right.length) { - return allCells -} -const leftAndRight: Array = [...left, ...right] -return allCells.filter((d) => !leftAndRight.includes(d.column.id)) -``` - -For `table_getCenterLeafColumns`: `if (!left.length && !right.length) return table.getAllLeafColumns()`. For `table_getCenterHeaderGroups`: skip the spread+filter when both are empty and pass `leafColumns` straight to `buildHeaderGroups`. - -**Big-O:** In the unpinned case: O(N) filter copy + spread per recompute → O(1). Row variant: R × (1 array alloc + N includes-checks + N-element copy) per pinning-adjacent state change eliminated. Referential identity of `allCells` preserved, so downstream dep tuples keyed on the cells array stop invalidating spuriously. - -**Risk:** Very low. Returning the shared array matches the `table_getPinnedVisibleLeafColumns(table, undefined)` precedent; `buildHeaderGroups` does not mutate `columnsToGroup` (verified: it only `.map`s it); no in-repo mutation of the returned arrays. -**Verification:** CONFIRMED, demoted 6 → 5 (fires only for pinning-layout tables that happen to have empty pinning; per state-change, not per tick). - ---- - ## 86. A11: buildHeaderGroups findMaxDepth: skip getIsVisible on leaf columns — Score: 5 **Status:** `[ ]` not started @@ -1534,60 +1121,6 @@ Hot path: per rendered parent row per render after any selection change (indeter --- -## 92. C11: row_getAllCells reconstructs all cell instances whenever leaf-column array identity changes: per-row cell cache keyed by column instance (WeakMap REQUIRED) — Score: 5 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `packages/table-core/src/core/rows/coreRowsFeature.utils.ts:167–179` + registration `coreRowsFeature.ts:26–29` -**Category:** `allocation` - -Hot path: per rendered row after any `columnOrder`, `grouping`, `groupedColumnMode`, or `columns` change: `getAllLeafColumns` memoDeps invalidate on all four, cascading into `row_getAllCells` (dep `[row.table.getAllLeafColumns()]`), then `row_getVisibleCells`, `row_getAllCellsByColumnId`, and `cell_getContext` (new instances → new context objects and closures). Reordering columns (drag) or toggling grouping does not change any (row, column) pair, yet every rendered row rebuilds N cell objects; each cell later rebuilds its context (1 object + 2 closures + a lazy tableMemo per instance). At 100 rendered rows × 100 columns per drag step: ~10k cells + ~10k contexts + ~20k closures churned, and all cell identities change, defeating adapter-level cell memoization (full DOM-level cell re-render). Column instances are stable across order/grouping changes (`getAllColumns` deps are `[options.columns]` only; ordering reorders existing instances), so instance-keyed reuse works. - -**Cross-refs / gating:** WeakMap is REQUIRED, not optional (see risk). Legacy entry #20 (perf-todo.md) records the underlying observation this rationale depends on: `createCoreRowModel` memoDeps are `[table.options.data]` only, so rows survive `columns` swaps. - -**Before** - -```ts -const columns = row.table.getAllLeafColumns() -const cells: Array> = new Array(columns.length) -for (let i = 0; i < columns.length; i++) { - cells[i] = constructCell(columns[i]!, row, row.table) -} -return cells -``` - -**After** (verifier-corrected: MUST be a `WeakMap`, not a `Map`) - -```ts -const columns = row.table.getAllLeafColumns() -let cache = row._cellsCache -if (!cache) { - cache = row._cellsCache = new WeakMap< - Column, - Cell - >() -} -const cells: Array> = new Array(columns.length) -for (let i = 0; i < columns.length; i++) { - const column = columns[i]! - let cell = cache.get(column) - if (!cell) { - cell = constructCell(column, row, row.table) - cache.set(column, cell) - } - cells[i] = cell -} -return cells -``` - -**Big-O:** Column reorder / grouping toggle: rendered-rows × N cell constructions + context/memo churn → 0 reconstructions (array reorder only); preserves cell identity so adapter cell memoization survives reorders. Steady-state renders unchanged (the outer memo still short-circuits). - -**Risk (WeakMap rationale):** The finder's claim "rows are rebuilt on data/columns change" is FALSE for columns: `createCoreRowModel` memoDeps are `[table.options.data]` ONLY, so rows survive a `columns` swap. A plain `Map` would accumulate one generation of stale column→cell entries per columns-identity change; with the common user error of unstable `columns` per render, that is an unbounded R×N leak. `WeakMap` ephemeron semantics collect the circular key→cell→column entries once old column instances are unreachable. Cells hold only `column`/`id`/`row` + shared prototype (verified); no fresh-cell assumption found in scope. Behavior delta to flag: cell identity now survives leaf-array changes (an adapter memoization win). -**Verification:** AMENDED: WeakMap made mandatory, with the createCoreRowModel-deps rationale (`[data]` only; rows survive columns swaps). - ---- - ## 93. D8: buildHeaderGroups placeholderId computes an O(P) allocating filter per header, O(N²) worst case per level — Score: 5 **Status:** `[ ]` not started @@ -1994,20 +1527,6 @@ Trivial `for` loop replacement of `.reduce`. --- -## 59. `table.getAllLeafColumns()` is called many places per row-model build — Score: 4 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** filterFns, faceting, grouping, pinning, global filtering -**Category:** `memoization` - -`getAllLeafColumns()` is memoized at the table level, but its deps are sometimes computed inline (see #16 type defects). Verify the memo holds across the row-model rebuild lifecycle. If it doesn't, this is the most-leveraged optimization in the package. - -**Risk:** Already memoized in `coreColumnsFeature`; just audit for accidental dep churn. - ---- - ## 105. A10: contextDocument operator precedence: parameter always ignored; SSR ReferenceError path (bug) — Score: 4 (bug) **Status:** `[ ]` not started