perf(router-core): avoid quadratic cache resource handoff - #8240
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
View your CI Pipeline Execution ↗ for commit 9be38ff
☁️ Nx Cloud last updated this comment at |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthrough
ChangesCache resource handoff
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change improves navigation performance with large preload caches while preserving cached loader-flight ownership and cleanup behavior. The covered cache and preload scenarios indicate no remaining merge-readiness risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Changeset Version Preview5 package(s) bumped directly, 18 bumped as dependents. 🟩 Patch bumps
|
Bundle Size Benchmarks
The following scenarios have bundle-size changes compared with the baseline:
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
Merging this PR will degrade performance by 7.58%
|
🎯 Changes
Committing a navigation currently checks each previous match against an array containing both active and cached matches. With many cached preloads, resource handoff repeatedly scans the cache even when its entries retain the same owners.
Use the cache's existing map to exclude retained match identities and matches without loader resources before handoff. The remaining owners are checked against active matches only, removing the quadratic cache membership scan. Filtering stays after publication and uses the captured cache so synchronous reentry preserves existing behavior. No public API changes;
commitMatchesis exported only from its internal module for tests and benchmarks.Add ownership tests for retained identities, cloned committed matches, shared flights, same-ID replacements, duplicate owners, abort ordering, synchronous cache mutation/replacement, and fresh versus expired preloads. Add focused commit and public navigation benchmarks.
Performance
Local macOS measurements using Node 25.8.1 and Vitest/jsdom, comparing the baseline with identical benchmark support against the final implementation. Each operation below performs two complete router-core navigations with preloading outside the timer; these timings exclude React rendering and browser painting.
Small-cache results do not establish a meaningful speedup. A focused commit benchmark with 5,000 retained preload resources improved from 2.35 ms to 0.253 ms (about 9.3×, both margins of error below 0.8%). The focused benchmark stubs abort controllers to isolate ownership work; unit tests cover real abort events. Benchmarks also cover empty caches and full, mixed, and zero retention. Cache reconstruction and active-match checks still run.
Tradeoff: if every cached entry expires while still owning a loader resource, the extra filtering pass can slow bulk cleanup. The focused comparison measured 38.6 → 46.8 µs for 1,000 expired resources and 208 → 225 µs for 5,000. Ordinary empty-cache checks showed only nanosecond-scale differences. The implementation keeps one filtering path without an empty-cache special case.
Bundle size
A full build of all 18 scenarios measured −3 to +9 gzip bytes versus the original baseline, with +19 raw bytes per scenario and unchanged chunk counts. React Router minimal increases by 3 gzip bytes (85,752 → 85,755); full increases by 2 gzip bytes (89,340 → 89,342). The large-cache runtime gains justify this small increase.
Validation
✅ Checklist
🚀 Release Impact
Summary by CodeRabbit
Performance
Bug Fixes
Tests