diff --git a/RESULT-optimization-shorter-string-operations.md b/RESULT-optimization-shorter-string-operations.md new file mode 100644 index 0000000000..5e46d7f38b --- /dev/null +++ b/RESULT-optimization-shorter-string-operations.md @@ -0,0 +1,80 @@ +# Shorter equivalent string operations + +## Principle + +Use `slice` instead of `substring` only when both bounds are proven non-negative and ordered. This change applies that rule to 24 calls. It does not change public options, return shapes, component props, serialized data, or user-visible behavior. + +## Isolated attribution + +Representative scenario: `react-router.minimal`, measured against `main` at `697ebb6ddbd433d052b6b4707938a5c595865d58`. + +| Hunk | Raw | Initial gzip | Gzip | Brotli | +| ----------------------------------- | ----: | -----------: | ----: | -----: | +| Proven-range `substring` to `slice` | -88 B | -10 B | -12 B | -110 B | + +## Full bundle matrix + +The candidate and control were built independently from the same exact base and dependency graph. The final slice-only candidate is recorded in `/tmp/native-slice-only-full.json`; gzip improves in all 17 scenarios. + +| Scenario | Raw | Initial gzip | Gzip | Brotli | +| -------------------------------- | ----: | -----------: | ----: | -----: | +| react-router.minimal | -88 B | -10 B | -12 B | -110 B | +| react-router.full | -88 B | -11 B | -7 B | -163 B | +| solid-router.minimal | -88 B | -11 B | -11 B | +21 B | +| solid-router.full | -88 B | -2 B | -2 B | -10 B | +| vue-router.minimal | -88 B | -7 B | -8 B | -16 B | +| vue-router.full | -88 B | -9 B | -8 B | -15 B | +| react-start.minimal | -88 B | -13 B | -14 B | -1 B | +| react-start.deferred-hydration | -88 B | -15 B | -14 B | +102 B | +| react-start.full | -86 B | +3 B | -2 B | +61 B | +| react-start.rsbuild.minimal | -86 B | -14 B | -14 B | -77 B | +| react-start.rsbuild.minimal-iife | -86 B | -12 B | -11 B | -27 B | +| react-start.rsbuild.full | -84 B | -5 B | -5 B | +8 B | +| solid-start.minimal | -88 B | -13 B | -12 B | +58 B | +| solid-start.deferred-hydration | -88 B | -15 B | -10 B | -17 B | +| solid-start.full | -88 B | -17 B | -17 B | -106 B | +| vue-start.minimal | -88 B | -24 B | -23 B | +2 B | +| vue-start.full | -88 B | -8 B | -8 B | +4 B | + +Ranges: + +- raw: -88 B to -84 B in all scenarios +- initial gzip: -24 B to +3 B; 16 improve and one regresses +- gzip: -23 B to -2 B in all scenarios +- Brotli: improves in ten scenarios and regresses by 2–102 B in seven + +## Semantic constraints + +Every changed `slice` bound comes from a fixed non-negative offset or parser offsets that are constructed in ascending order. The five parameter-extraction calls whose bounds can reverse remain `substring`: prefix/suffix overlaps can make value bounds cross, and Unicode lowercasing can expand an affix before name extraction. Focused tests preserve named, optional, and wildcard overlap behavior plus the Unicode length-expansion edge case. + +`String.prototype.slice` is already used throughout the affected shipped packages, so this does not raise the browser-support floor. + +## Validation + +The focused router-core path suite passes with 361 tests and no type errors. Package-level unit validation also passes: + +- history: 25 tests +- router-core: 1,528 passed and three expected failures +- React Router: 989 passed and one skipped +- Solid Router: 838 passed and one skipped + +The initial combined unit command was stopped after its output stalled twice under the repository's execution guardrail; the package-level runs above completed against the exact candidate. Type tests pass for all four affected packages across TypeScript 5.6 through 7.0. ESLint reports no errors; remaining warnings are pre-existing. + +The focused `path-string-operations.bench.ts` benchmark first compares the changed native operation directly and verifies that `slice` and `substring` return identical values for representative ordered bounds. Across four final runs, `slice` averaged 34,385.01 Hz versus 31,162.09 Hz for `substring`, a 10.34% improvement. + +The same benchmark exercises href parsing, interpolation, route-tree construction, and search-prefix handling through their real APIs. Four bracketed exact-base/candidate pairs produced these average throughputs: + +| Operation | Exact base | Candidate | Change | +| --------------------------------- | -----------: | -----------: | -----: | +| Parse 400 hrefs | 11,379.09 Hz | 11,560.88 Hz | +1.60% | +| Interpolate 300 path templates | 3,691.13 Hz | 3,697.17 Hz | +0.16% | +| Construct a 30-route dynamic tree | 74,722.08 Hz | 75,369.98 Hz | +0.87% | +| Parse 400 search prefixes | 7,100.11 Hz | 7,062.15 Hz | -0.53% | + +The search-prefix result has no stable direction: the four paired deltas alternate between -4.54%, +2.05%, -1.75%, and +2.37%, while the directly changed operation is consistently faster. The other workflows are flat to positive. The repository's existing framework link benchmark could not provide a usable control: the React run exhausted its 4 GB heap after existing `act(...)` warnings, and the Solid run loaded a client-only API in server mode. The focused benchmark was added so performance validation would not depend on those unrelated failures. + +## Rejected nearby variant + +Five boolean `indexOf` probes were also tested as `includes`. That group saved 10 gzip bytes in `react-router.minimal`, but three isolated performance pairs ranged from -1.60% to +0.64% and averaged -0.23%. The result was not confidently neutral, so the group was dropped from the final candidate. + +Five independent publication reviews cover semantic equivalence, adversarial bounds, browser support, public API behavior, tree-shaking, performance, tests, and measurement integrity. Their final disposition is recorded before publication. diff --git a/packages/history/src/index.ts b/packages/history/src/index.ts index 0f3be8242e..718ea74f9f 100644 --- a/packages/history/src/index.ts +++ b/packages/history/src/index.ts @@ -662,7 +662,7 @@ export function parseHref( return { href: sanitizedHref, - pathname: sanitizedHref.substring( + pathname: sanitizedHref.slice( 0, hashIndex > 0 ? searchIndex > 0 @@ -672,7 +672,7 @@ export function parseHref( ? searchIndex : sanitizedHref.length, ), - hash: hashIndex > -1 ? sanitizedHref.substring(hashIndex) : '', + hash: hashIndex > -1 ? sanitizedHref.slice(hashIndex) : '', search: searchIndex > -1 ? sanitizedHref.slice( @@ -686,5 +686,5 @@ export function parseHref( // Thanks co-pilot! function createRandomKey() { - return (Math.random() + 1).toString(36).substring(7) + return (Math.random() + 1).toString(36).slice(7) } diff --git a/packages/router-core/src/new-process-route-tree.ts b/packages/router-core/src/new-process-route-tree.ts index 6978b071ce..8f233a8367 100644 --- a/packages/router-core/src/new-process-route-tree.ts +++ b/packages/router-core/src/new-process-route-tree.ts @@ -80,7 +80,7 @@ export function parseSegment( ): ParsedSegment { const next = path.indexOf('/', start) const end = next === -1 ? path.length : next - const part = path.substring(start, end) + const part = path.slice(start, end) if (!part || !part.includes('$')) { // early escape for static pathname @@ -219,7 +219,7 @@ function parseSegments( const kind = segment[0] switch (kind) { case SEGMENT_TYPE_PATHNAME: { - const value = path.substring(segment[2], segment[3]) + const value = path.slice(segment[2], segment[3]) if (caseSensitive) { const existingNode = node.static?.get(value) if (existingNode) { @@ -253,8 +253,8 @@ function parseSegments( break } case SEGMENT_TYPE_PARAM: { - const prefix_raw = path.substring(start, segment[1]) - const suffix_raw = path.substring(segment[4], end) + const prefix_raw = path.slice(start, segment[1]) + const suffix_raw = path.slice(segment[4], end) const actuallyCaseSensitive = caseSensitive && !!(prefix_raw || suffix_raw) const prefix = !prefix_raw @@ -295,8 +295,8 @@ function parseSegments( break } case SEGMENT_TYPE_OPTIONAL_PARAM: { - const prefix_raw = path.substring(start, segment[1]) - const suffix_raw = path.substring(segment[4], end) + const prefix_raw = path.slice(start, segment[1]) + const suffix_raw = path.slice(segment[4], end) const actuallyCaseSensitive = caseSensitive && !!(prefix_raw || suffix_raw) const prefix = !prefix_raw @@ -337,8 +337,8 @@ function parseSegments( break } case SEGMENT_TYPE_WILDCARD: { - const prefix_raw = path.substring(start, segment[1]) - const suffix_raw = path.substring(segment[4], end) + const prefix_raw = path.slice(start, segment[1]) + const suffix_raw = path.slice(segment[4], end) const actuallyCaseSensitive = caseSensitive && !!(prefix_raw || suffix_raw) const prefix = !prefix_raw @@ -922,7 +922,7 @@ function extractParams( const value = part!.substring(preLength, part!.length - sufLength) rawParams[name] = decodeURIComponent(value) } else { - const name = nodePart.substring(1) + const name = nodePart.slice(1) rawParams[name] = decodeURIComponent(part!) } } else if (node.kind === SEGMENT_TYPE_OPTIONAL_PARAM) { diff --git a/packages/router-core/src/path.ts b/packages/router-core/src/path.ts index 577ce00ecf..8d1a07b654 100644 --- a/packages/router-core/src/path.ts +++ b/packages/router-core/src/path.ts @@ -279,7 +279,7 @@ export function interpolatePath({ if (end === -1) end = length cursor = end - const part = path.substring(start, end) + const part = path.slice(start, end) if (!part) continue // `$id` or `$` (splat). '$' code is 36 @@ -298,7 +298,7 @@ export function interpolatePath({ const value = encodeParam('_splat', params, decoder) joined += '/' + value } else { - const key = part.substring(1) + const key = part.slice(1) if (!isMissingParams && !(key in params)) { isMissingParams = true } @@ -334,7 +334,7 @@ export function interpolatePath({ const kind = segment[0] if (kind === SEGMENT_TYPE_PATHNAME) { - joined += '/' + path.substring(start, end) + joined += '/' + path.slice(start, end) continue } @@ -344,8 +344,8 @@ export function interpolatePath({ // TODO: Deprecate * usedParams['*'] = splat - const prefix = path.substring(start, segment[1]) - const suffix = path.substring(segment[4], end) + const prefix = path.slice(start, segment[1]) + const suffix = path.slice(segment[4], end) // Check if _splat parameter is missing. _splat could be missing if undefined or an empty string or some other falsy value. if (!splat) { @@ -364,21 +364,21 @@ export function interpolatePath({ } if (kind === SEGMENT_TYPE_PARAM) { - const key = path.substring(segment[2], segment[3]) + const key = path.slice(segment[2], segment[3]) if (!isMissingParams && !(key in params)) { isMissingParams = true } usedParams[key] = params[key] - const prefix = path.substring(start, segment[1]) - const suffix = path.substring(segment[4], end) + const prefix = path.slice(start, segment[1]) + const suffix = path.slice(segment[4], end) const value = encodeParam(key, params, decoder) ?? 'undefined' joined += '/' + prefix + value + suffix continue } if (kind === SEGMENT_TYPE_OPTIONAL_PARAM) { - const key = path.substring(segment[2], segment[3]) + const key = path.slice(segment[2], segment[3]) const valueRaw = params[key] // Check if optional parameter is missing or undefined @@ -386,8 +386,8 @@ export function interpolatePath({ usedParams[key] = valueRaw - const prefix = path.substring(start, segment[1]) - const suffix = path.substring(segment[4], end) + const prefix = path.slice(start, segment[1]) + const suffix = path.slice(segment[4], end) const value = encodeParam(key, params, decoder) ?? '' joined += '/' + prefix + value + suffix continue diff --git a/packages/router-core/src/searchParams.ts b/packages/router-core/src/searchParams.ts index 740d36441c..b1f1343cfb 100644 --- a/packages/router-core/src/searchParams.ts +++ b/packages/router-core/src/searchParams.ts @@ -22,7 +22,7 @@ export const defaultStringifySearch = stringifySearchWith( export function parseSearchWith(parser: (str: string) => any) { return (searchStr: string): AnySchema => { if (searchStr[0] === '?') { - searchStr = searchStr.substring(1) + searchStr = searchStr.slice(1) } const query: Record = decode(searchStr) diff --git a/packages/router-core/tests/path-string-operations.bench.ts b/packages/router-core/tests/path-string-operations.bench.ts new file mode 100644 index 0000000000..0b2ede6710 --- /dev/null +++ b/packages/router-core/tests/path-string-operations.bench.ts @@ -0,0 +1,130 @@ +import { parseHref } from '@tanstack/history' +import { bench, describe, expect } from 'vitest' +import { processRouteTree } from '../src/new-process-route-tree' +import { interpolatePath } from '../src/path' +import { parseSearchWith } from '../src/searchParams' + +const hrefs = [ + '/posts/123?sort=newest#comments', + '/files/report.pdf#download', + '/search?q=router&page=2', + '/plain/path', +] +const interpolationCases = [ + { + path: '/teams/$team/projects/$project', + params: { team: 'router', project: 'benchmarks' }, + }, + { + path: '/files/prefix{$file}.json/{-$revision}', + params: { file: 'results', revision: 'latest' }, + }, + { + path: '/docs/$', + params: { _splat: 'guides/data-loading' }, + }, +] +const routeTree = { + id: '__root__', + isRoot: true, + fullPath: '/', + path: '/', + children: Array.from({ length: 30 }, (_, index) => { + const path = `/team-${index}/prefix{$project}.json/{-$revision}` + return { id: path, fullPath: path, path } + }), +} +const parseSearch = parseSearchWith((search) => search) +const extractionCases: ReadonlyArray< + readonly [value: string, start: number, end?: number] +> = [ + ['/posts/123?sort=newest#comments', 0, 10], + ['/files/prefix{$file}.json', 7, 19], + ['prefix{$project}.json', 7, 15], + ['0.123456789abcdefghijklmnopqrstuvwxyz', 7], +] +let benchmarkSink = 0 + +expect(parseHref(hrefs[0]!, undefined)).toMatchObject({ + pathname: '/posts/123', + search: '?sort=newest', + hash: '#comments', +}) +expect( + interpolatePath({ + ...interpolationCases[1]!, + server: false, + }).interpolatedPath, +).toBe('/files/prefixresults.json/latest') +expect(processRouteTree(routeTree).routesByPath).toHaveProperty( + '/team-29/prefix{$project}.json/{-$revision}', +) +expect(parseSearch('?sort=newest')).toEqual({ sort: 'newest' }) +expect( + extractionCases.map(([value, start, end]) => value.slice(start, end)), +).toEqual( + extractionCases.map(([value, start, end]) => value.substring(start, end)), +) + +describe('equivalent native string operations', () => { + bench('substring on proven ordered bounds', () => { + let size = 0 + for (let index = 0; index < 1_000; index++) { + for (const [value, start, end] of extractionCases) { + size += value.substring(start, end).length + } + } + benchmarkSink = size + }) + + bench('slice on proven ordered bounds', () => { + let size = 0 + for (let index = 0; index < 1_000; index++) { + for (const [value, start, end] of extractionCases) { + size += value.slice(start, end).length + } + } + benchmarkSink = size + }) +}) + +describe('path string operations', () => { + bench('parse 400 hrefs', () => { + let size = 0 + for (let index = 0; index < 100; index++) { + for (const href of hrefs) { + const result = parseHref(href, undefined) + size += + result.pathname.length + result.search.length + result.hash.length + } + } + benchmarkSink = size + }) + + bench('interpolate 300 path templates', () => { + let size = 0 + for (let index = 0; index < 100; index++) { + for (const input of interpolationCases) { + size += interpolatePath({ + ...input, + server: false, + }).interpolatedPath.length + } + } + benchmarkSink = size + }) + + bench('construct a 30-route dynamic tree', () => { + benchmarkSink = Object.keys(processRouteTree(routeTree).routesByPath).length + }) + + bench('parse 400 search prefixes', () => { + let size = 0 + for (let index = 0; index < 400; index++) { + size += Object.keys(parseSearch('?sort=newest&page=2')).length + } + benchmarkSink = size + }) +}) + +void benchmarkSink diff --git a/packages/router-core/tests/path.test.ts b/packages/router-core/tests/path.test.ts index 4e49412209..b58c608d93 100644 --- a/packages/router-core/tests/path.test.ts +++ b/packages/router-core/tests/path.test.ts @@ -822,6 +822,17 @@ describe('matchPathname', () => { _splat: 'bar/baz', }, }, + { + name: 'with overlapping prefix + suffix', + input: '/a/abc', + matchingOptions: { + to: '/a/ab{$}bc', + }, + expectedMatchedParams: { + '*': 'b', + _splat: 'b', + }, + }, ])('$name', ({ input, matchingOptions, expectedMatchedParams }) => { expect(matchPathname(input, matchingOptions)).toStrictEqual( toNullObj(expectedMatchedParams), @@ -901,6 +912,46 @@ describe('matchPathname', () => { param: 'foobar', }, }, + { + name: 'with overlapping prefix + suffix', + input: '/a/abc', + matchingOptions: { + to: '/a/ab{$id}bc', + }, + expectedMatchedParams: { + id: 'b', + }, + }, + { + name: 'optional with overlapping prefix + suffix', + input: '/a/abc', + matchingOptions: { + to: '/a/ab{-$id}bc', + }, + expectedMatchedParams: { + id: 'b', + }, + }, + { + name: 'with a suffix whose lowercase form expands', + input: '/a/zzzzzİİ', + matchingOptions: { + to: '/a/{$x}İİ', + }, + expectedMatchedParams: { + $: 'zzz', + }, + }, + { + name: 'optional with a suffix whose lowercase form expands', + input: '/a/zzzzzİİ', + matchingOptions: { + to: '/a/{-$x}İİ', + }, + expectedMatchedParams: { + $: 'zzz', + }, + }, ])('$name', ({ input, matchingOptions, expectedMatchedParams }) => { expect(matchPathname(input, matchingOptions)).toStrictEqual( toNullObj(expectedMatchedParams),