diff --git a/RESULT-perf-task1.md b/RESULT-perf-task1.md new file mode 100644 index 0000000000..b4517a244b --- /dev/null +++ b/RESULT-perf-task1.md @@ -0,0 +1,63 @@ +# RESULT — perf/task1-jsonstart-parse-gate + +## Change + +`packages/router-core/src/searchParams.ts` — `parseSearchWith` now captures +`const isJsonParser = parser === JSON.parse` once at factory time and skips the +try/catch `JSON.parse` attempt for string values that fail +`jsonStart = /^(?:\s|["[{\d-]|fa|nu|tr)/`, mirroring the existing optimization +in `stringifySearchWith`. Non-JSON custom parsers (public API) are completely +unaffected: the guard is gated behind `isJsonParser`. + +## Correctness + +- Old vs new implementations compared on all 11 bench input sets plus 45 + adversarial values (empty, whitespace-only, `"0"`, `"-"`, `"fa"`, `"nu"`, + `"tr"`, `"false"`, `"null"`, `"tru"`, `"{"`, `[`, digits, unicode, + leading-space strings, JSON fragments, etc.) with both `JSON.parse` and a + custom parser: **all results deep-equal**. +- Behavior for `JSON.parse` is strictly identical: regex false positives + (`"favorite"`, `"true_value"`, `"tru"` …) still enter the try/catch and fall + through unchanged; only strings that cannot begin valid JSON skip the parse. + +## Benchmark + +`packages/router-core/tests/searchParams-parse.bench.ts` (new), mirroring +`searchParams.bench.ts`; each iteration parses 1,000 search strings via +`defaultParseSearch` (`parser === JSON.parse`). Vitest bench, Node v26. + +hz = operations per second (one op = one 1,000-string batch); mean in ms; rme. + +| Scenario | hz before | mean before | rme | hz after | mean after | rme | speedup | +| ---------------------------------------------------- | --------: | ----------: | -----: | -------: | ---------: | -----: | ------: | +| ordinary string values | 67.16 | 12.68ms | ±2.29% | 1,182.63 | 0.83ms | ±0.25% | ~17.6x | +| ordinary strings outside JSON-literal initials | 63.50 | 15.36ms | ±1.19% | 1,084.15 | 0.91ms | ±0.20% | ~17.1x | +| ordinary strings with JSON-literal initials | 64.01 | 15.16ms | ±0.99% | 1,239.23 | 0.79ms | ±0.24% | ~19.4x | +| empty string values | 66.74 | 14.77ms | ±0.82% | 1,519.67 | 0.64ms | ±0.25% | ~22.8x | +| non-JSON punctuation starts | 81.58 | 12.01ms | ±1.69% | 991.53 | 0.98ms | ±0.24% | ~12.2x | +| f/n/t words outside JSON-literal prefixes | 62.90 | 15.32ms | ±4.18% | 1,153.26 | 0.83ms | ±0.28% | ~18.3x | +| application words with JSON-literal prefixes | 64.04 | 15.36ms | ±0.84% | 59.56 | 16.53ms | ±0.74% | ~0.9x* | +| words with complete JSON-literal prefixes (fa/nu/tr…) | 83.83 | 11.77ms | ±0.85% | 75.21 | 12.71ms | ±3.43% | ~0.9x* | +| JSON-literal prefixes followed by punctuation | 83.28 | 11.80ms | ±0.86% | 77.05 | 12.64ms | ±1.17% | ~0.9x* | +| JSON-compatible string values | 845.08 | 1.17ms | ±0.24% | 794.92 | 1.22ms | ±0.73% | ~0.94x | +| mixed application values | 116.46 | 8.45ms | ±0.84% | 1,226.90 | 0.80ms | ±0.26% | ~10.5x | + +\* Scenarios whose values pass the `jsonStart` regex still go through +try/catch as before; the small delta (~5–10%) is regex overhead plus run noise +and is within expected cost for keeping behavior strictly identical. The win: +typical application search params (plain words) parse **12–23x faster**. + +## Tests / lint / types + +- `@tanstack/router-core:test:unit`: **pass** (106 files, 1609 passed, + typecheck clean) +- `@tanstack/router-core:test:eslint`: **pass** (0 errors; pre-existing `_err` + unused-var warnings unchanged) +- `@tanstack/router-core:test:types`: **pass** (ts56–ts70) + +New unit tests in `packages/router-core/tests/searchParams.test.ts`: + +- `parse skips JSON.parse for strings that cannot begin valid JSON` + (asserts `JSON.parse` spy never invoked) +- `parse still parses strings that pass the jsonStart guard` +- `parse applies the guard only when the parser is JSON.parse` diff --git a/packages/router-core/src/searchParams.ts b/packages/router-core/src/searchParams.ts index b93cc23206..926e4e53dd 100644 --- a/packages/router-core/src/searchParams.ts +++ b/packages/router-core/src/searchParams.ts @@ -24,6 +24,7 @@ export const defaultStringifySearch = stringifySearchWith( * @link https://tanstack.com/router/latest/docs/framework/react/guide/custom-search-param-serialization */ export function parseSearchWith(parser: (str: string) => any) { + const isJsonParser = parser === JSON.parse return (searchStr: string): AnySchema => { if (searchStr[0] === '?') { searchStr = searchStr.substring(1) @@ -35,6 +36,10 @@ export function parseSearchWith(parser: (str: string) => any) { for (const key in query) { const value = query[key] if (typeof value === 'string') { + // Skip JSON.parse when the value cannot begin valid JSON. + if (isJsonParser && !jsonStart.test(value)) { + continue + } try { query[key] = parser(value) } catch (_err) { diff --git a/packages/router-core/tests/searchParams-parse.bench.ts b/packages/router-core/tests/searchParams-parse.bench.ts new file mode 100644 index 0000000000..aa6b966259 --- /dev/null +++ b/packages/router-core/tests/searchParams-parse.bench.ts @@ -0,0 +1,167 @@ +import { bench, describe, expect } from 'vitest' +import { defaultParseSearch } from '../src' + +const iterations = 1_000 + +const ordinaryStrings = { + tab: 'specs', + filter: 'available', + category: 'hardware', + sort: 'newest', +} +const nonLiteralInitialStrings = { + tab: 'specs', + filter: 'available', + category: 'hardware', + sort: 'descending', +} +const jsonInitialStrings = { + filter: 'foo', + notification: 'new', + tab: 'tabular', + empty: '', +} +const emptyStrings = { + first: '', + second: '', + third: '', + fourth: '', +} +const punctuationStrings = { + file: '.env', + path: '/products', + positive: '+1', + priority: '!important', +} +const nonLiteralPrefixStrings = { + first: 'future', + second: 'framework', + third: 'name', + fourth: 'table', +} +const jsonLiteralPrefixStrings = { + first: 'favorite', + second: 'number', + third: 'travel', + fourth: 'nullish', +} +const jsonLiteralWordStrings = { + truthy: 'true_value', + falsy: 'false_value', + nullable: 'null_value', +} +const jsonLiteralBoundaryStrings = { + truthy: 'true-value', + falsy: 'false/value', + nullable: 'null.value', +} +const jsonStrings = { + number: '123', + boolean: 'true', + object: '{"nested":true}', + array: '[1,2,3]', +} +const mixedValues = { + tab: 'specs', + page: '2', + filters: 'available', + exactPage: '2', +} + +function toSearchString(search: Record): string { + return Object.entries(search) + .map(([key, value]) => `${key}=${value}`) + .join('&') +} + +let benchmarkSink = 0 + +// Correctness expectations for the parse side. +expect(defaultParseSearch(toSearchString(ordinaryStrings))).toEqual( + ordinaryStrings, +) +expect(defaultParseSearch(toSearchString(nonLiteralInitialStrings))).toEqual( + nonLiteralInitialStrings, +) +expect(defaultParseSearch(toSearchString(jsonInitialStrings))).toEqual( + jsonInitialStrings, +) +expect(defaultParseSearch(toSearchString(emptyStrings))).toEqual(emptyStrings) +expect(defaultParseSearch(toSearchString(punctuationStrings))).toEqual({ + ...punctuationStrings, + positive: 1, +}) +expect(defaultParseSearch(toSearchString(nonLiteralPrefixStrings))).toEqual( + nonLiteralPrefixStrings, +) +expect(defaultParseSearch(toSearchString(jsonLiteralPrefixStrings))).toEqual( + jsonLiteralPrefixStrings, +) +expect(defaultParseSearch(toSearchString(jsonLiteralWordStrings))).toEqual( + jsonLiteralWordStrings, +) +expect(defaultParseSearch(toSearchString(jsonLiteralBoundaryStrings))).toEqual( + jsonLiteralBoundaryStrings, +) +expect(defaultParseSearch(toSearchString(jsonStrings))).toEqual({ + number: 123, + boolean: true, + object: { nested: true }, + array: [1, 2, 3], +}) + +function parseBatch(searchStr: string) { + let size = 0 + for (let index = 0; index < iterations; index++) { + size += Object.keys(defaultParseSearch(searchStr)).length + } + benchmarkSink = size +} + +describe('default search parsing', () => { + bench('ordinary string values', () => { + parseBatch(toSearchString(ordinaryStrings)) + }) + + bench('ordinary strings outside JSON-literal initials', () => { + parseBatch(toSearchString(nonLiteralInitialStrings)) + }) + + bench('ordinary strings with JSON-literal initials', () => { + parseBatch(toSearchString(jsonInitialStrings)) + }) + + bench('empty string values', () => { + parseBatch(toSearchString(emptyStrings)) + }) + + bench('ordinary strings with non-JSON punctuation starts', () => { + parseBatch(toSearchString(punctuationStrings)) + }) + + bench('ordinary f/n/t words outside JSON-literal prefixes', () => { + parseBatch(toSearchString(nonLiteralPrefixStrings)) + }) + + bench('application words with JSON-literal prefixes', () => { + parseBatch(toSearchString(jsonLiteralPrefixStrings)) + }) + + bench('application words with complete JSON-literal prefixes', () => { + parseBatch(toSearchString(jsonLiteralWordStrings)) + }) + + bench('JSON-literal prefixes followed by punctuation', () => { + parseBatch(toSearchString(jsonLiteralBoundaryStrings)) + }) + + bench('JSON-compatible string values', () => { + parseBatch(toSearchString(jsonStrings)) + }) + + bench('mixed application values', () => { + parseBatch(toSearchString(mixedValues)) + }) +}) + +void benchmarkSink diff --git a/packages/router-core/tests/searchParams.test.ts b/packages/router-core/tests/searchParams.test.ts index e56f8b7131..323dbddd5c 100644 --- a/packages/router-core/tests/searchParams.test.ts +++ b/packages/router-core/tests/searchParams.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test, vi } from 'vitest' import { defaultParseSearch, defaultStringifySearch, + parseSearchWith, stringifySearchWith, } from '../src' @@ -109,6 +110,43 @@ describe('Search Params serialization and deserialization', () => { } }) + test('parse skips JSON.parse for strings that cannot begin valid JSON', () => { + const parseSpy = vi.spyOn(JSON, 'parse') + try { + expect( + defaultParseSearch('?empty=&filter=foo&tab=specs&sort=newest'), + ).toEqual({ empty: '', filter: 'foo', tab: 'specs', sort: 'newest' }) + expect(defaultParseSearch('?file=.env&path=/products')).toEqual({ + file: '.env', + path: '/products', + }) + expect(parseSpy).not.toHaveBeenCalled() + } finally { + parseSpy.mockRestore() + } + }) + + test('parse still parses strings that pass the jsonStart guard', () => { + expect(defaultParseSearch('?n=123&flag=true&obj={"a":1}&arr=[1]')).toEqual({ + n: 123, + flag: true, + obj: { a: 1 }, + arr: [1], + }) + }) + + test('parse applies the guard only when the parser is JSON.parse', () => { + const upperCaseParser = (str: string) => str.toUpperCase() + const parse = parseSearchWith(upperCaseParser) + + // A non-JSON parser is invoked even for values the jsonStart + // regex would reject. + expect(parse('?foo=bar&filter=available')).toEqual({ + foo: 'BAR', + filter: 'AVAILABLE', + }) + }) + test('[edge case] self-reference serializes to "object Object"', () => { const obj = {} as any obj.self = obj