feat(router-core) path: parsePathname returns a readonly array#4705
Merged
feat(router-core) path: parsePathname returns a readonly array#4705
Conversation
|
View your CI Pipeline Execution ↗ for commit aea7e85
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-with-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-plugin
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-plugin
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-server-functions-client
@tanstack/start-server-functions-fetcher
@tanstack/start-server-functions-server
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
schiller-manuel
approved these changes
Jul 18, 2025
Sheraff
added a commit
that referenced
this pull request
Jul 23, 2025
We noticed that `parsePathname` is a performance bottleneck during navigation events. Here's a flamegraph from an application w/ ~300 routes around a navigation event: <img width="853" height="372" alt="Screenshot 2025-07-22 at 19 36 24" src="https://github.com/user-attachments/assets/7ed2e09a-8ea9-4170-862a-2d53c3eb4793" /> This PR proposes a basic least-recently-used cache implementation (that data structure is about 4-5x slower than a `Map` according to benchmarks, and it uses a little more memory). We can add caching to `parsePathname` now that what it returns is readonly (#4705), which should improve the performance of this bottleneck. The cache is set to a size of 1000. When / if we end up making a pre-built matcher (WIP #4714), we maybe can reduce this size. But `parsePathname` is still called w/ built pathnames so we probably shouldn't remove the cache entirely. When benchmarking `matchPathname` with and without the cache, we notice a ~9x increase in throughput with the cache. ``` ✓ @tanstack/router-core tests/cache.bench.ts > cache.bench 7191ms name hz min max mean p75 p99 p995 p999 rme samples · original 1,795.61 0.5055 0.7022 0.5569 0.5604 0.6234 0.6464 0.7022 ±0.22% 898 · cached 16,307.87 0.0562 0.2294 0.0613 0.0608 0.0767 0.1007 0.1152 ±0.17% 8154 fastest BENCH Summary @tanstack/router-core cached - tests/cache.bench.ts > cache.bench 9.08x faster than original ``` Assuming this 9x increase translates to a proportional reduction of the self-time seen in the flamegraph, it would go from 55ms to 6ms.
naoya7076
pushed a commit
to naoya7076/router
that referenced
this pull request
Feb 15, 2026
…ack#4705) `parsePathname` should return a `ReadonlyArray` instead of `Array`, and `Segment` should be `readonly` too. This is because if we want to improve perf by either caching at runtime or pre-computing routes at build time, we need to ensure those objects can't be manipulated. An unrelated change in this PR is also to move some inline functions to module-scoped functions: `segmentToString` (from `resolvePath`) and `isMatch` (from `matchByPath`). Those functions are in the hot path so it's nice if the runtime compiler can promote them to optimized functions. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
naoya7076
pushed a commit
to naoya7076/router
that referenced
this pull request
Feb 15, 2026
…ack#4752) We noticed that `parsePathname` is a performance bottleneck during navigation events. Here's a flamegraph from an application w/ ~300 routes around a navigation event: <img width="853" height="372" alt="Screenshot 2025-07-22 at 19 36 24" src="https://github.com/user-attachments/assets/7ed2e09a-8ea9-4170-862a-2d53c3eb4793" /> This PR proposes a basic least-recently-used cache implementation (that data structure is about 4-5x slower than a `Map` according to benchmarks, and it uses a little more memory). We can add caching to `parsePathname` now that what it returns is readonly (TanStack#4705), which should improve the performance of this bottleneck. The cache is set to a size of 1000. When / if we end up making a pre-built matcher (WIP TanStack#4714), we maybe can reduce this size. But `parsePathname` is still called w/ built pathnames so we probably shouldn't remove the cache entirely. When benchmarking `matchPathname` with and without the cache, we notice a ~9x increase in throughput with the cache. ``` ✓ @tanstack/router-core tests/cache.bench.ts > cache.bench 7191ms name hz min max mean p75 p99 p995 p999 rme samples · original 1,795.61 0.5055 0.7022 0.5569 0.5604 0.6234 0.6464 0.7022 ±0.22% 898 · cached 16,307.87 0.0562 0.2294 0.0613 0.0608 0.0767 0.1007 0.1152 ±0.17% 8154 fastest BENCH Summary @tanstack/router-core cached - tests/cache.bench.ts > cache.bench 9.08x faster than original ``` Assuming this 9x increase translates to a proportional reduction of the self-time seen in the flamegraph, it would go from 55ms to 6ms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
parsePathnameshould return aReadonlyArrayinstead ofArray, andSegmentshould bereadonlytoo. This is because if we want to improve perf by either caching at runtime or pre-computing routes at build time, we need to ensure those objects can't be manipulated.An unrelated change in this PR is also to move some inline functions to module-scoped functions:
segmentToString(fromresolvePath) andisMatch(frommatchByPath). Those functions are in the hot path so it's nice if the runtime compiler can promote them to optimized functions.