diff --git a/.changeset/soft-views-tease.md b/.changeset/soft-views-tease.md new file mode 100644 index 000000000..7889b0206 --- /dev/null +++ b/.changeset/soft-views-tease.md @@ -0,0 +1,5 @@ +--- +'@tanstack/form-core': patch +--- + +Improve performance for mounting/unmounting diff --git a/.gitignore b/.gitignore index 22096f2e0..885eed7cc 100644 --- a/.gitignore +++ b/.gitignore @@ -28,6 +28,7 @@ size-plugin.json stats-hydration.json stats.json stats.html +*.cpuprofile .vscode/settings.json *.log diff --git a/packages/form-core/package.json b/packages/form-core/package.json index 6b2fc7b27..d841674e4 100644 --- a/packages/form-core/package.json +++ b/packages/form-core/package.json @@ -25,6 +25,8 @@ "test:types:ts58": "tsc", "test:lib": "vitest", "test:lib:dev": "pnpm run test:lib --watch", + "test:bench": "vitest bench --run", + "test:bench:dev": "vitest bench --watch", "test:build": "publint --strict", "build": "vite build" }, diff --git a/packages/form-core/src/utils.ts b/packages/form-core/src/utils.ts index 7cabb65b5..f67533d78 100644 --- a/packages/form-core/src/utils.ts +++ b/packages/form-core/src/utils.ts @@ -141,16 +141,12 @@ export function deleteBy(obj: any, _path: any) { return doDelete(obj) } -const reLineOfOnlyDigits = /^(\d+)$/gm -// the second dot must be in a lookahead or the engine -// will skip subsequent numbers (like foo.0.1.) -const reDigitsBetweenDots = /\.(\d+)(?=\.)/gm -const reStartWithDigitThenDot = /^(\d+)\./gm -const reDotWithDigitsToEnd = /\.(\d+$)/gm -const reMultipleDots = /\.{2,}/gm - -const intPrefix = '__int__' -const intReplace = `${intPrefix}$1` +// Char codes used by the parser below. +const CC_DOT = 0x2e // '.' +const CC_OPEN = 0x5b // '[' +const CC_CLOSE = 0x5d // ']' +const CC_ZERO = 0x30 // '0' +const CC_NINE = 0x39 // '9' /** * @private @@ -164,31 +160,87 @@ export function makePathArray(str: string | Array) { throw new Error('Path must be a string.') } - return ( - str - // Leading `[` may lead to wrong parsing down the line - // (Example: '[0][1]' should be '0.1', not '.0.1') - .replace(/(^\[)|]/gm, '') - .replace(/\[/g, '.') - .replace(reLineOfOnlyDigits, intReplace) - .replace(reDigitsBetweenDots, `.${intReplace}.`) - .replace(reStartWithDigitThenDot, `${intReplace}.`) - .replace(reDotWithDigitsToEnd, `.${intReplace}`) - .replace(reMultipleDots, '.') - .split('.') - .map((d) => { - if (d.startsWith(intPrefix)) { - const numStr = d.substring(intPrefix.length) - const num = parseInt(numStr, 10) - - if (String(num) === numStr) { - return num + const len = str.length + const result: Array = [] + // Location of the first character of the in-progress segment in `str`. + // The segment ends at the current `i` when we hit a separator. + // + // We strip an optional leading '[' so '[0]' parses as [0], not ['', 0]. + // Doing this up front keeps the loop's backwards compatibility handling simpler. + let segStart = len > 0 && str.charCodeAt(0) === CC_OPEN ? 1 : 0 + // Whether the in-progress segment has been all ASCII digits so far. + // Used together with the leading-zero check to decide if it should be + // pushed as a number instead of a string. + let allDigits = true + // Tracks the previous character. Only necessary to preserve the + // old behavior for malformed input. + let prev = -1 + // Walk once. `i === len` is treated as a virtual final separator so the + // flush block handles both mid-string segments and the last one. + for (let i = segStart; i <= len; i++) { + const char = i < len ? str.charCodeAt(i) : -1 + + // Handle separators (including the virtual one at the end). Flush the in-progress segment. + if (i === len || char === CC_DOT || char === CC_OPEN || char === CC_CLOSE) { + const segLen = i - segStart + if (segLen > 0) { + // To treat the segment as a number... + const treatAsNumber = + // ...it must contain only digits... + allDigits && + // ...and either be a single '0' or not start with '0'. + (segLen === 1 || str.charCodeAt(segStart) !== CC_ZERO) + + const seg = str.slice(segStart, i) + if (treatAsNumber) { + const num = parseInt(seg, 10) + // Up to 15 digits, parseInt is always lossless (the max + // 15-digit decimal is below Number.MAX_SAFE_INTEGER). Beyond + // that, verify by round-trip: if parseInt lost precision + // (e.g., a 20-digit literal), fall back to the string so we + // don't silently change the value. + if (segLen <= 15 || String(num) === seg) { + result.push(num) + } else { + result.push(seg) } - return numStr + } else { + result.push(seg) } - return d - }) - ) + } else if ( + // This branch, which handles empty segments, only exists to preserve + // the old behavior for malformed input. + + // Push the empty segment unless this is a "phantom boundary" the + // old regex impl would have absorbed: + // 1. `]` was always stripped — `prev === ']'` means the real + // boundary already happened on the previous iteration. + // 2. A leading `]` was stripped too (the leading `[` strip + // above handles its counterpart for `[`). + // 3. `..` and `[[` collapse to a single boundary. + prev !== CC_CLOSE && + !(prev === -1 && char === CC_CLOSE) && + !(prev === char && (char === CC_DOT || char === CC_OPEN)) + ) { + result.push('') + } + + // Start a new segment. + segStart = i + 1 + allDigits = true + } else if (char < CC_ZERO || char > CC_NINE) { + allDigits = false + } + + prev = char + } + + // If the input was effectively all phantom chars (e.g. ']', '[]', + // '[]]'), the loop produces no segments. The old impl returned [''] + // for these because. + if (!result.length) result.push('') + + return result } /** diff --git a/packages/form-core/tests/utils.bench.ts b/packages/form-core/tests/utils.bench.ts new file mode 100644 index 000000000..33cb20111 --- /dev/null +++ b/packages/form-core/tests/utils.bench.ts @@ -0,0 +1,68 @@ +import { bench, describe } from 'vitest' +import { makePathArray } from '../src/utils' + +// Snapshot of the original implementation for side-by-side comparison. +// Remove this and the paired benches once the new implementation is merged. +const reLineOfOnlyDigits = /^(\d+)$/gm +const reDigitsBetweenDots = /\.(\d+)(?=\.)/gm +const reStartWithDigitThenDot = /^(\d+)\./gm +const reDotWithDigitsToEnd = /\.(\d+$)/gm +const reMultipleDots = /\.{2,}/gm +const intPrefix = '__int__' +const intReplace = `${intPrefix}$1` + +function makePathArrayOld( + str: string | Array, +): Array { + if (Array.isArray(str)) { + return [...str] + } + + if (typeof str !== 'string') { + throw new Error('Path must be a string.') + } + + return str + .replace(/(^\[)|]/gm, '') + .replace(/\[/g, '.') + .replace(reLineOfOnlyDigits, intReplace) + .replace(reDigitsBetweenDots, `.${intReplace}.`) + .replace(reStartWithDigitThenDot, `${intReplace}.`) + .replace(reDotWithDigitsToEnd, `.${intReplace}`) + .replace(reMultipleDots, '.') + .split('.') + .map((d) => { + if (d.startsWith(intPrefix)) { + const numStr = d.substring(intPrefix.length) + const num = parseInt(numStr, 10) + if (String(num) === numStr) { + return num + } + return numStr + } + return d + }) +} + +const cases: Array<[label: string, input: string | Array]> = [ + ['array input (fast path, no parsing)', ['a', 'b', 0, 'c']], + ['simple key (no nesting)', 'key'], + ['uuid key', '550e8400-e29b-41d4-a716-446655440000'], + ['dot notation', 'foo.bar.baz'], + ['mixed dot and bracket notation', 'a[0].b[1]'], + ['deeply nested mixed path', 'a.b[0][1].c.d[2][3].e'], + ['numeric string with leading zeros (kept as string)', '01234'], + ['numeric string (converted to number)', '12345'], +] + +for (const [label, input] of cases) { + describe(label, () => { + bench('old', () => { + makePathArrayOld(input) + }) + + bench('new', () => { + makePathArray(input) + }) + }) +} diff --git a/packages/form-core/tests/utils.spec.ts b/packages/form-core/tests/utils.spec.ts index a545fd719..e415f72b7 100644 --- a/packages/form-core/tests/utils.spec.ts +++ b/packages/form-core/tests/utils.spec.ts @@ -270,6 +270,83 @@ describe('makePathArray', () => { it('should still convert non-leading-zero numbers to number types', () => { expect(makePathArray('12345')).toEqual([12345]) }) + + it('should keep digit-only segments past Number precision as strings', () => { + expect(makePathArray('99999999999999999999')).toEqual([ + '99999999999999999999', + ]) + }) + + it('should treat lone "0" as the number 0', () => { + expect(makePathArray('0')).toEqual([0]) + expect(makePathArray('a.0.b')).toEqual(['a', 0, 'b']) + }) + + it('should preserve leading zeros mid-path in both notations', () => { + expect(makePathArray('a.01.b')).toEqual(['a', '01', 'b']) + expect(makePathArray('a[01]')).toEqual(['a', '01']) + }) + + it('should return a defensive copy when given an array', () => { + const input: Array = ['a', 0, 'b'] + const out = makePathArray(input) + expect(out).toEqual(input) + expect(out).not.toBe(input) + }) + + it('should throw on non-string non-array input', () => { + expect(() => makePathArray(null as any)).toThrow('Path must be a string.') + expect(() => makePathArray(42 as any)).toThrow('Path must be a string.') + expect(() => makePathArray({} as any)).toThrow('Path must be a string.') + }) + + it('should handle malformed input', () => { + // Backwards compatible: + + // Previous output: ['a', 'b'] + expect(makePathArray('a..b')).toEqual(['a', 'b']) + // Previous output: ['a'] + expect(makePathArray(']a')).toEqual(['a']) + // Previous output: ['a'] + expect(makePathArray('a]')).toEqual(['a']) + // Previous output: ['a', 'b', 'c'] + expect(makePathArray('a[b[c')).toEqual(['a', 'b', 'c']) + // Previous output: ['a', 'b', 'c'] + expect(makePathArray('a[b[c]')).toEqual(['a', 'b', 'c']) + // Previous output: [''] + expect(makePathArray('')).toEqual(['']) + // Previous output: ['', ''] + expect(makePathArray('.')).toEqual(['', '']) + // Previous output: [''] + expect(makePathArray('[')).toEqual(['']) + // Previous output: [''] + expect(makePathArray('[]')).toEqual(['']) + // Previous output: ['', 'a'] + expect(makePathArray('.a')).toEqual(['', 'a']) + // Previous output: ['a', ''] + expect(makePathArray('a.')).toEqual(['a', '']) + // Previous output: ['a', ''] + expect(makePathArray('a[')).toEqual(['a', '']) + // Previous output: ['', 'a'] + expect(makePathArray('..a')).toEqual(['', 'a']) + // Previous output: ['a', ''] + expect(makePathArray('a..')).toEqual(['a', '']) + // Previous output: ['a', ''] + expect(makePathArray('a[[')).toEqual(['a', '']) + // Previous output: [''] + expect(makePathArray(']')).toEqual(['']) + // Previous output: ['', ''] + expect(makePathArray('[[')).toEqual(['', '']) + // Previous output: ['', 0] + expect(makePathArray('[[0]')).toEqual(['', 0]) + + // Breaking changes: + + // This case is impossible to reproduce without allocating a new string that + // completely elides `]` or maintaining an array buffer in the loop to do so. + // Previous output: ['ab'] + expect(makePathArray('a]b')).toEqual(['a', 'b']) + }) }) describe('determineFormLevelErrorSourceAndValue', () => { diff --git a/packages/form-core/vite.config.ts b/packages/form-core/vite.config.ts index 94d53421f..92c1c4730 100644 --- a/packages/form-core/vite.config.ts +++ b/packages/form-core/vite.config.ts @@ -10,6 +10,10 @@ const config = defineConfig({ environment: 'jsdom', coverage: { enabled: true, provider: 'istanbul', include: ['src/**/*'] }, typecheck: { enabled: true }, + benchmark: { + include: ['**/*.bench.ts'], + exclude: ['node_modules'], + }, }, }) diff --git a/packages/react-form/tests/field-render-perf.test.tsx b/packages/react-form/tests/field-render-perf.test.tsx new file mode 100644 index 000000000..5a36b0cee --- /dev/null +++ b/packages/react-form/tests/field-render-perf.test.tsx @@ -0,0 +1,216 @@ +/** + * Mount + unmount cost for forms with many `` instances. + * + * Skipped by default. Two modes: + * + * # 1) Timing table across field counts + * PERF=1 pnpm --filter @tanstack/react-form test:lib field-render-perf + * + * # 2) CPU profile of a single mount + unmount, written next to this file as + * # field-render-perf.mount.N{N}.cpuprofile / .unmount.N{N}.cpuprofile. + * # Open with Chrome DevTools (Performance → Load profile) or VS Code's + * # "vscode-js-profile-flame" extension. + * PERF=1 PROFILE=1 PROFILE_N=2000 pnpm --filter @tanstack/react-form test:lib field-render-perf + * + * (No `--` separator: with pnpm 10's `--filter`, args after `--` get dropped + * before vitest sees them, so the filename filter is silently ignored and the + * full suite runs instead.) + * + * + * RESULTS BEFORE OPTIMIZATION ================================== + * iterations=5 warmup=1 env=jsdom (median of 5) + * N mount unmount total total min..max + * 100 17.9ms 18.5ms 36.3ms 35.3..45.8ms + * 500 185.1ms 419.9ms 601.5ms 599.8..612.1ms + * 1000 691.2ms 1897.7ms 2549.5ms 2457.9..2677.3ms + * 2000 2709.0ms 8351.3ms 11060.6ms 10882.0..11462.1ms + * ============================================================== + * + * NOTE: This file is intended to be removed before merge. + */ +import { writeFileSync } from 'node:fs' +import { Session } from 'node:inspector/promises' +import { join } from 'node:path' +import { describe, it } from 'vitest' +import { act, render } from '@testing-library/react' +import { useForm } from '../src/index' + +const COUNTS = [100, 500, 1000, 2000] +const WARMUP = 1 +const ITERATIONS = 5 +const PROFILE = !!process.env.PROFILE +const PROFILE_N = Number(process.env.PROFILE_N || 1000) + +function ManyFields({ ids }: { ids: string[] }) { + const form = useForm({ + defaultValues: {} as Record, + }) + + return ( +
+ {ids.map((id, i) => ( +
+ +
+ ))} +
+ ) +} + +function makeIds(n: number): string[] { + return Array.from({ length: n }, (_, i) => `f${i}`) +} + +interface Sample { + mountMs: number + unmountMs: number + totalMs: number +} + +async function measureOnce(ids: string[]): Promise { + let result: ReturnType | null = null + + const t0 = performance.now() + await act(async () => { + result = render() + }) + const t1 = performance.now() + + await act(async () => { + result!.unmount() + }) + const t2 = performance.now() + + return { mountMs: t1 - t0, unmountMs: t2 - t1, totalMs: t2 - t0 } +} + +interface Stats { + median: number + min: number + max: number +} + +function summarize(xs: number[]): Stats { + const sorted = [...xs].sort((a, b) => a - b) + return { + median: sorted[Math.floor(sorted.length / 2)]!, + min: sorted[0]!, + max: sorted[sorted.length - 1]!, + } +} + +function fmt(ms: number) { + return `${ms.toFixed(1).padStart(7)}ms` +} + +// V8 CPU profile around an arbitrary region. Writes a .cpuprofile to disk that +// Chrome DevTools / vscode-js-profile-flame can load directly. +async function withCpuProfile( + outPath: string, + fn: () => Promise, +): Promise { + const session = new Session() + session.connect() + try { + await session.post('Profiler.enable') + // 100us sample interval — ~10x finer than default; plenty of detail + // without ballooning profile size for sub-second regions. + await session.post('Profiler.setSamplingInterval', { interval: 100 }) + await session.post('Profiler.start') + const result = await fn() + const { profile } = await session.post('Profiler.stop') + writeFileSync(outPath, JSON.stringify(profile)) + return result + } finally { + session.disconnect() + } +} + +describe.skipIf(!process.env.PERF)('field render perf', () => { + it('mount + unmount cost across field counts', async () => { + const rows: Array<{ + n: number + mount: Stats + unmount: Stats + total: Stats + }> = [] + + for (const N of COUNTS) { + const ids = makeIds(N) + for (let i = 0; i < WARMUP; i++) await measureOnce(ids) + const samples: Sample[] = [] + for (let i = 0; i < ITERATIONS; i++) samples.push(await measureOnce(ids)) + rows.push({ + n: N, + mount: summarize(samples.map((s) => s.mountMs)), + unmount: summarize(samples.map((s) => s.unmountMs)), + total: summarize(samples.map((s) => s.totalMs)), + }) + } + + const lines = [ + '', + `iterations=${ITERATIONS} warmup=${WARMUP} env=jsdom (median of ${ITERATIONS})`, + `${'N'.padStart(5)} ${'mount'.padStart(9)} ${'unmount'.padStart(9)} ${'total'.padStart(9)} ${'total min..max'.padStart(20)}`, + ] + for (const r of rows) { + lines.push( + `${String(r.n).padStart(5)} ${fmt(r.mount.median)} ${fmt(r.unmount.median)} ${fmt(r.total.median)} ${`${r.total.min.toFixed(1)}..${r.total.max.toFixed(1)}ms`.padStart(20)}`, + ) + } + console.log(lines.join('\n')) + }, 600_000) + + it.skipIf(!PROFILE)( + `cpu profile: mount + unmount at N=${PROFILE_N}`, + async () => { + const ids = makeIds(PROFILE_N) + for (let i = 0; i < WARMUP; i++) await measureOnce(ids) + + let result: ReturnType | null = null + const mountPath = join( + import.meta.dirname, + `field-render-perf.mount.N${PROFILE_N}.cpuprofile`, + ) + const unmountPath = join( + import.meta.dirname, + `field-render-perf.unmount.N${PROFILE_N}.cpuprofile`, + ) + + const mountMs = await withCpuProfile(mountPath, async () => { + const t0 = performance.now() + await act(async () => { + result = render() + }) + return performance.now() - t0 + }) + + const unmountMs = await withCpuProfile(unmountPath, async () => { + const t0 = performance.now() + await act(async () => { + result!.unmount() + }) + return performance.now() - t0 + }) + + console.log( + [ + '', + `cpu profile (N=${PROFILE_N}):`, + ` mount ${mountMs.toFixed(1)}ms → ${mountPath}`, + ` unmount ${unmountMs.toFixed(1)}ms → ${unmountPath}`, + ` open in: Chrome DevTools (Performance → Load profile)`, + ` or VS Code "vscode-js-profile-flame" extension`, + ].join('\n'), + ) + }, + 600_000, + ) +})