diff --git a/RESULT-perf-task2.md b/RESULT-perf-task2.md new file mode 100644 index 0000000000..a6eafefaec --- /dev/null +++ b/RESULT-perf-task2.md @@ -0,0 +1,154 @@ +# RESULT — perf/task2-search-reserialization + +Benchmark-first investigation of search re-serialization costs in `router-core` per navigation. +Branch: `perf/task2-search-reserialization` (worktree `/tmp/opencode/router-perf-task2`). + +## 1. Call graph: search work in ONE navigation + +### Same-search navigation (e.g. hash-only nav, param-change nav, re-click same link) + +``` +navigate() / buildAndCommitLocation() +└─ buildLocation(opts) router.ts:1838 + ├─ matchRoutesLightweight(currentLocation) router.ts:1758 + │ └─ fromSearch = { ...location.search } (shallow copy, cached per location + │ in lightweightCache, WeakMap) + ├─ applySearchMiddleware(...) router.ts:2739 + ├─ nullReplaceEqualDeep(fromSearch, nextSearch) router.ts:2016 [DEEP COMPARE] + └─ this.options.stringifySearch(nextSearch) router.ts:2019 [ALWAYS STRINGIFY] + → defaultStringifySearch: + URLSearchParams encode + JSON.stringify + per object value + jsonStart regex / + JSON.parse probe per string value + +load() router.ts:2343 +└─ updateLatestLocation() router.ts:1309 + └─ parseLocation(history.location, prevLocation) router.ts:1387 + ├─ parseSearch(search) router.ts:1402 [PARSE] + ├─ stringifySearch(parsedSearch) router.ts:1403 [STRINGIFY AGAIN] + └─ nullReplaceEqualDeep(prevSearch, parsedSearch) router.ts:1411 [DEEP COMPARE AGAIN] +``` + +Counts per navigation: + +| Nav type | deep compares | stringify | parse | notes | +|---------------------|--------------|-----------|-------|-------| +| same-search nav | 2 | 2 | 1 | both stringifies are pure recomputation of an unchanged string | +| hash-only nav | 2 | 2 | 1 | identical search work to same-search | +| search-changing nav | 2 | 2 | 1 | all necessary except possibly the second stringify | + +Key observation (router.ts:2016-2019): `nullReplaceEqualDeep` returns **the previous reference** +when deeply equal (structural sharing). The subsequent unconditional `stringifySearch` therefore +recomputes a string that is, by definition, identical to one already produced for that very +object reference earlier. The identity of the returned object is a free memo key. + +Also noted: `replaceEqualDeep` has an O(1) fast path when `prev === _next` (utils.ts:240), and +`getEnumerableOwnKeys`/`isPlainObject` make unequal-object traversal comparatively cheap. + +## 2. Benchmarks + +Files: `packages/router-core/tests/searchReserialization.bench.ts` (micro + flow), +`packages/router-core/tests/buildLocationSearchStr.bench.ts` (end-to-end `buildLocation`). +Style follows `searchParams.bench.ts`: batched iterations (1000/bench op; 2000 for e2e). +Correctness asserted before timing (reference reuse, structural sharing, string equality). + +Machine-local vitest bench (hz = batches/s, mean in ms per batched op): + +### nullReplaceEqualDeep alone (mean ms) + +| Case | hz | mean | rme | +|-------------------------------------------|-----------|--------|--------| +| small equal `{page:1}` | 6,902 | 0.145 | ±0.32% | +| small unequal | 7,344 | 0.136 | ±0.44% | +| medium equal (~10 keys mixed) | 2,039 | 0.490 | ±0.73% | +| medium unequal (one leaf changed) | 2,031 | 0.492 | ±1.17% | +| large equal (~50 keys nested) | 94.4 | 10.59 | ±0.84% | +| large unequal (one nested leaf changed) | 126.3 | 7.92 | ±0.98% | +| large unequal (new key) | 126.8 | 7.89 | ±0.31% | +| identical reference (fast path) | 65,688 | 0.015 | ±0.05% | + +### defaultStringifySearch alone (mean ms) + +| Shape | hz | mean | rme | +|--------------------------------|----------|-------|--------| +| small (`{page:1}`) | 10,135 | 0.099 | ±0.56% | +| medium (~10 keys mixed) | 1,141 | 0.876 | ±0.46% | +| large (~50 keys nested) | 152.5 | 6.56 | ±0.36% | + +Stringify is ~2x the cost of the deep compare at every size — it dominates the flow. + +### Flow: current (deep-equal + always stringify) vs identity-memoized (mean ms) + +| Scenario | current hz | memoized hz | speedup | delta mean | +|--------------------------------|------------|-------------|---------|------------| +| small equal (same-search) | 3,501 | 6,802 | **1.94x** | −49% | +| medium equal (same-search) | 637 | 2,038 | **3.20x** | −69% | +| large equal (same-search) | 58.5 | 94.9 | **1.62x** | −38% | +| medium unequal (changed) | 566 | 563 | 1.00x | ±0% | +| large unequal (leaf change) | 69.1 | 67.9 | 0.98x | +1.7% (within rme) | + +### End-to-end real `buildLocation` (route with validateSearch, repeated same-search navs) + +| Build | hz | mean (ms / 2000 calls) | rme | per-call | +|--------------------------------|--------|------------------------|--------|----------| +| baseline (main) | 342.08 | 2.923 | ±0.45% | ~1.46 µs | +| with prototype | 694.49 | 1.440 | ±0.40% | ~0.72 µs | + +**2.03x throughput on the hot path** (measured by stashing/unstashing only the router.ts change). + +## 3. Verdict + +**IMPLEMENT** — data clears the >20% bar decisively: + +- Same-search navigations (the most common navigation type: hash changes, path-param-only + changes, redundant clicks, link re-renders): 1.6x–3.2x on the search-resolution step, + 2.03x end-to-end on real `buildLocation`. +- Changed-search navigations: no regression (±1%, within measurement error) — the memo + simply misses and falls through to the normal stringify. +- Memory cost: one `WeakMap` entry per distinct committed search object; + entries are GC-eligible as soon as the structurally-shared search object dies. + +## 4. Prototype (implemented, buildLocation only) + +`packages/router-core/src/router.ts`: + +- New private field `searchStrMemo = new WeakMap()` (router.ts:1114). +- In `build()` (router.ts:2016+): after `nullReplaceEqualDeep`, look up the merged search + object's identity in the memo; on hit, reuse the cached `searchStr`; on miss, call + `this.options.stringifySearch(nextSearch)` and store it. + +Correctness argument: + +- The emitted `searchStr` always corresponds to the emitted `search` object: the memo is + keyed on object *identity*, and `stringifySearch` is deterministic per snapshot. Since + search objects are treated immutably throughout the codebase (all mutation paths create + copies via spread/middleware), identity ⇒ same content ⇒ same string. This was verified + explicitly by round-trip assertions (`stringifySearch(loc.search) === loc.searchStr`) in + `tests/searchStrMemo.test.ts`. +- Hash-only navigations still produce correct hrefs: hash resolution (router.ts:2022+) is + independent of the memoized string; href = `pathname + searchStr + hashStr` with the + memoized searchStr being byte-identical to what stringify would have produced. Verified + in tests (`/?page=1#section`) and by the full suite. +- Scope deliberately limited to `buildLocation`; `parseLocation`'s duplicate + parse→stringify round-trip (router.ts:1402-1403) is a separate follow-up opportunity. + +## 5. Verification + +- `pnpm nx run @tanstack/router-core:test:unit --skipNxCache --skipRemoteCache` + → **107 files passed, 1610 tests passed (+1 new regression test)**, no type errors. +- `pnpm nx run @tanstack/router-core:test:eslint ...` + → 26 warnings, 0 errors — **identical to clean main** (pre-existing), no new issues. + +New files: +- `packages/router-core/tests/searchReserialization.bench.ts` — micro + flow benchmarks +- `packages/router-core/tests/buildLocationSearchStr.bench.ts` — end-to-end benchmark +- `packages/router-core/tests/searchStrMemo.test.ts` — correctness regression test + +Production change: 13 lines in `packages/router-core/src/router.ts`. + +## Recommendation + +Ship the `searchStrMemo` prototype. Follow-ups worth benchmarking separately: +(1) apply the same memo in `parseLocation` to kill the parse→stringify round-trip on +URL-normalization paths, and (2) consider skipping the second `nullReplaceEqualDeep` +when `parsedSearch` can be proven freshly created. diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 900609098e..3c58cdeb25 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -1111,6 +1111,7 @@ export class RouterCore< ParsedLocation, LightweightRouteMatchCacheEntry >() + private searchStrMemo = new WeakMap() isServer!: boolean pathParamsDecoder?: (encoded: string) => string protocolAllowlist!: Set @@ -2015,8 +2016,18 @@ export class RouterCore< // Replace the equal deep nextSearch = nullReplaceEqualDeep(fromSearch, nextSearch) - // Stringify the next search - const searchStr = this.options.stringifySearch(nextSearch) + // Stringify the next search, reusing a previously computed string when + // the (structurally shared) search object was serialized before + let searchStr: string | undefined + if (nextSearch !== null && typeof nextSearch === 'object') { + searchStr = this.searchStrMemo.get(nextSearch) + } + if (searchStr === undefined) { + searchStr = this.options.stringifySearch(nextSearch) + if (nextSearch !== null && typeof nextSearch === 'object') { + this.searchStrMemo.set(nextSearch, searchStr) + } + } // Resolve the next hash const hash = diff --git a/packages/router-core/tests/buildLocationSearchStr.bench.ts b/packages/router-core/tests/buildLocationSearchStr.bench.ts new file mode 100644 index 0000000000..43c9253998 --- /dev/null +++ b/packages/router-core/tests/buildLocationSearchStr.bench.ts @@ -0,0 +1,55 @@ +import { bench, describe, expect } from 'vitest' +import { createMemoryHistory } from '@tanstack/history' +import { BaseRootRoute, BaseRoute } from '../src' +import { createTestRouter } from './routerTestUtils' + +const rootRoute = new BaseRootRoute({}) +const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + validateSearch: (search: { page?: number }) => { + return { + page: typeof search.page === 'number' ? search.page : 1, + } + }, +}) +const routeTree = rootRoute.addChildren([indexRoute]) + +function makeRouter() { + return createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/'] }), + }) +} + +let benchmarkSink = 0 +const iterations = 2_000 + +// Warm up and correctness check +const warm = makeRouter() +const loc = warm.buildLocation({ to: '/', search: true }) +expect(loc.searchStr).toBe('?page=1') +const hashLoc = warm.buildLocation({ + to: '/', + search: true, + hash: (prev: string) => prev, +}) +expect(hashLoc.href.startsWith('/?page=1')).toBe(true) + +describe('router.buildLocation - repeated same-search navigations', () => { + bench( + 'validateSearch route, same-search nav', + () => { + const router = makeRouter() + let size = 0 + for (let i = 0; i < iterations; i++) { + size += router.buildLocation({ to: '/', search: true }).searchStr + .length + } + benchmarkSink = size + }, + { time: 2000 }, + ) +}) + +void benchmarkSink diff --git a/packages/router-core/tests/searchReserialization.bench.ts b/packages/router-core/tests/searchReserialization.bench.ts new file mode 100644 index 0000000000..d541f45fc9 --- /dev/null +++ b/packages/router-core/tests/searchReserialization.bench.ts @@ -0,0 +1,260 @@ +import { bench, describe, expect } from 'vitest' +import { defaultStringifySearch } from '../src' +import { nullReplaceEqualDeep } from '../src/utils' + +const iterations = 1_000 + +// --------------------------------------------------------------------------- +// Search shapes: small / medium / large-nested +// --------------------------------------------------------------------------- + +const small = { page: 1 } + +const medium: Record = { + page: 1, + tab: 'specs', + filter: 'available', + sort: 'newest', + desc: true, + limit: 25, + offset: 0, + tags: ['hardware', 'featured'], + range: 'last-30-days', + view: 'grid', +} + +function makeLarge(): Record { + const large: Record = {} + for (let i = 0; i < 40; i++) { + large[`field${i}`] = i % 3 === 0 ? `value-${i}` : i + } + // Nested objects/arrays to exercise deep traversal + large.filters = { + category: ['a', 'b', 'c'], + price: { min: 10, max: 500 }, + rating: { min: 3 }, + } + large.sort = { by: 'name', dir: 'asc' } + large.meta = { source: 'ui', nested: { deep: true, count: [1, 2, 3] } } + // 40 scalars + 3 nested top-level keys = 43... add a few more flat ones + large.extraA = 'x' + large.extraB = false + large.extraC = 9.5 + large.extraD = null + large.extraE = '' + large.extraF = 'y' + large.extraG = 1 + return large +} + +const large = makeLarge() +const largeUnequalChangedLeaf = makeLarge() +largeUnequalChangedLeaf.field7 = 'different' +const largeUnequalNewKey = makeLarge() +largeUnequalNewKey.brandNew = true + +const mediumUnequal = { ...medium, page: 2 } +const smallUnequal = { page: 2 } + +let benchmarkSink = 0 +let benchmarkSinkObj: unknown + +// --------------------------------------------------------------------------- +// Correctness verification before timing +// --------------------------------------------------------------------------- + +// nullReplaceEqualDeep must return the previous reference for equal inputs +// and a fresh value for unequal ones. +expect(nullReplaceEqualDeep(small, { ...small })).toBe(small) +expect(nullReplaceEqualDeep(medium, { ...medium })).toBe(medium) +expect(nullReplaceEqualDeep(large, makeLarge())).toBe(large) +expect(nullReplaceEqualDeep(small, smallUnequal)).not.toBe(small) +expect(nullReplaceEqualDeep(medium, mediumUnequal)).not.toBe(medium) +expect(nullReplaceEqualDeep(large, largeUnequalChangedLeaf)).not.toBe(large) +expect(nullReplaceEqualDeep(large, largeUnequalNewKey)).not.toBe(large) + +// Structural sharing: unchanged children are reused +const shared = nullReplaceEqualDeep( + large, + largeUnequalChangedLeaf, +) as typeof large +expect(shared.filters).toBe(large.filters) +expect(shared.sort).toBe(large.sort) +expect(shared.meta).toBe(large.meta) +expect(shared.field7).toBe('different') + +// The hypothetical memoized flow must emit an identical searchStr to the +// current always-stringify flow. +const cachedSmallStr = defaultStringifySearch(small) +const cachedMediumStr = defaultStringifySearch(medium) +const cachedLargeStr = defaultStringifySearch(large) +expect(cachedSmallStr).toBe(defaultStringifySearch({ ...small })) +expect(cachedMediumStr).toBe(defaultStringifySearch({ ...medium })) +expect(cachedLargeStr).toBe(defaultStringifySearch(makeLarge())) + +// --------------------------------------------------------------------------- +// Helpers modeling the two flows +// --------------------------------------------------------------------------- + +/** Current production flow: deep structural compare + unconditional stringify. */ +function currentFlow( + prev: Record, + next: Record, +) { + const merged = nullReplaceEqualDeep(prev, next) + const searchStr = defaultStringifySearch(merged) + benchmarkSink = searchStr.length + return merged +} + +/** + * Hypothetical flow: when the deep compare returns the previous reference, + * reuse the previously computed searchStr instead of re-stringifying. + */ +function memoizedFlow( + prev: Record, + next: Record, + prevSearchStr: string, +) { + const merged = nullReplaceEqualDeep(prev, next) + const searchStr = merged === prev ? prevSearchStr : defaultStringifySearch(merged) + benchmarkSink = searchStr.length + return merged +} + +function batch(flow: () => void) { + for (let index = 0; index < iterations; index++) { + flow() + } +} + +// --------------------------------------------------------------------------- +// Benchmarks: nullReplaceEqualDeep alone +// --------------------------------------------------------------------------- + +describe('nullReplaceEqualDeep', () => { + bench('small equal ({page:1})', () => { + batch(() => { + benchmarkSinkObj = nullReplaceEqualDeep(small, { ...small }) + }) + }) + + bench('small unequal', () => { + batch(() => { + benchmarkSinkObj = nullReplaceEqualDeep(small, smallUnequal) + }) + }) + + bench('medium equal (~10 keys)', () => { + batch(() => { + benchmarkSinkObj = nullReplaceEqualDeep(medium, { ...medium }) + }) + }) + + bench('medium unequal (one leaf changed)', () => { + batch(() => { + benchmarkSinkObj = nullReplaceEqualDeep(medium, mediumUnequal) + }) + }) + + bench('large equal (~50 keys, nested)', () => { + batch(() => { + benchmarkSinkObj = nullReplaceEqualDeep(large, makeLarge()) + }) + }) + + bench('large unequal (one nested leaf changed)', () => { + batch(() => { + benchmarkSinkObj = nullReplaceEqualDeep(large, largeUnequalChangedLeaf) + }) + }) + + bench('large unequal (new key)', () => { + batch(() => { + benchmarkSinkObj = nullReplaceEqualDeep(large, largeUnequalNewKey) + }) + }) + + bench('identical reference (fast path)', () => { + batch(() => { + benchmarkSinkObj = nullReplaceEqualDeep(large, large) + }) + }) +}) + +// --------------------------------------------------------------------------- +// Benchmarks: defaultStringifySearch alone +// --------------------------------------------------------------------------- + +describe('defaultStringifySearch', () => { + bench('small ({page:1})', () => { + batch(() => { + benchmarkSink = defaultStringifySearch(small).length + }) + }) + + bench('medium (~10 keys mixed types)', () => { + batch(() => { + benchmarkSink = defaultStringifySearch(medium).length + }) + }) + + bench('large (~50 keys nested)', () => { + batch(() => { + benchmarkSink = defaultStringifySearch(large).length + }) + }) +}) + +// --------------------------------------------------------------------------- +// Benchmarks: full navigation-search flow, current vs memoized +// --------------------------------------------------------------------------- + +describe('flow comparison - same-search navigation', () => { + bench('current: small equal', () => { + batch(() => currentFlow(small, { ...small })) + }) + + bench('memoized: small equal', () => { + batch(() => memoizedFlow(small, { ...small }, cachedSmallStr)) + }) + + bench('current: medium equal', () => { + batch(() => currentFlow(medium, { ...medium })) + }) + + bench('memoized: medium equal', () => { + batch(() => memoizedFlow(medium, { ...medium }, cachedMediumStr)) + }) + + bench('current: large equal', () => { + batch(() => currentFlow(large, makeLarge())) + }) + + bench('memoized: large equal', () => { + batch(() => memoizedFlow(large, makeLarge(), cachedLargeStr)) + }) +}) + +describe('flow comparison - changed-search navigation', () => { + bench('current: medium unequal', () => { + batch(() => currentFlow(medium, mediumUnequal)) + }) + + bench('memoized: medium unequal', () => { + batch(() => memoizedFlow(medium, mediumUnequal, cachedMediumStr)) + }) + + bench('current: large unequal (leaf change)', () => { + batch(() => currentFlow(large, largeUnequalChangedLeaf)) + }) + + bench('memoized: large unequal (leaf change)', () => { + batch(() => + memoizedFlow(large, largeUnequalChangedLeaf, cachedLargeStr), + ) + }) +}) + +void benchmarkSink +void benchmarkSinkObj diff --git a/packages/router-core/tests/searchStrMemo.test.ts b/packages/router-core/tests/searchStrMemo.test.ts new file mode 100644 index 0000000000..3c2c5dfe23 --- /dev/null +++ b/packages/router-core/tests/searchStrMemo.test.ts @@ -0,0 +1,77 @@ +import { expect, test } from 'vitest' +import { createMemoryHistory } from '@tanstack/history' +import { BaseRootRoute, BaseRoute } from '../src' +import { createTestRouter } from './routerTestUtils' + +const rootRoute = new BaseRootRoute({}) +const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + validateSearch: (search: { page?: number; q?: string }) => { + return { + page: typeof search.page === 'number' ? search.page : 1, + q: typeof search.q === 'string' ? search.q : undefined, + } + }, +}) +const routeTree = rootRoute.addChildren([indexRoute]) + +function makeRouter() { + return createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/'] }), + }) +} + +test('same-search nav: searchStr matches emitted search object', () => { + const router = makeRouter() + router.buildLocation({ to: '/', search: true }) + const a = router.buildLocation({ to: '/', search: true }) + expect(a.searchStr).toBe('?page=1') + // round-trip: re-stringifying the emitted search yields the emitted string + expect(router.options.stringifySearch(a.search)).toBe(a.searchStr) +}) + +test('hash-only nav: href contains unchanged search plus hash', () => { + const router = makeRouter() + router.buildLocation({ to: '/', search: true }) + const h1 = router.buildLocation({ to: '/', search: true, hash: 'section' }) + expect(h1.href).toBe('/?page=1#section') + expect(h1.searchStr).toBe('?page=1') + const h2 = router.buildLocation({ + to: '/', + search: true, + hash: (prev: string) => `x-${prev.length}`, + }) + expect(h2.href).toBe('/?page=1#x-0') +}) + +test('changed-search nav: fresh string computed', () => { + const router = makeRouter() + router.buildLocation({ to: '/', search: true }) + const b = router.buildLocation({ + to: '/', + search: (prev: any) => ({ ...prev, page: 5 }), + } as any) + expect(b.searchStr).toBe('?page=5') + expect(router.options.stringifySearch(b.search)).toBe(b.searchStr) + // back to original search still produces the correct string + const c = router.buildLocation({ to: '/', search: true }) + expect(c.searchStr).toBe('?page=1') +}) + +test('empty search serializes consistently', () => { + const aboutRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/about', + }) + const tree2 = new BaseRootRoute({}).addChildren([aboutRoute]) + const router = createTestRouter({ + routeTree: tree2, + history: createMemoryHistory({ initialEntries: ['/about'] }), + }) + router.buildLocation({ to: '/about', search: true }) + const loc = router.buildLocation({ to: '/about', search: true }) + expect(loc.searchStr).toBe('') + expect(loc.href).toBe('/about') +})