perf: cache fs probes and precompute route sort keys in scan/deploy paths - #1930
Conversation
`compareRoutes` re-parsed every pattern on each comparison, so a sort called `routePrecedence` ~2·log n times per route instead of once. Replace it with `sortRoutes`, which scores each pattern once into a Map and sorts by (score, pattern.localeCompare). The localeCompare tiebreaker already guarantees a total order, so ordering is byte-identical; this just removes the redundant re-parsing on every route scan (dev re-scans on file change, plus the pages/api and app-graph builds).
`buildAppRouteGraph` walks the appDir→leaf chain separately for every route (layouts, templates, errors, boundaries, slots), each step probing up to 4 extensions per convention file via `existsSync`. Shared ancestor directories — the `app/` root above all — were re-stat'd once per descendant route, so a scan cost roughly routes × depth × conventions × extensions syscalls. Memoize `findFile` through a `WeakMap<ValidFileMatcher, Map>` keyed by a per-scan matcher clone created in `buildAppRouteGraph`. Results are immutable within one scan, so no invalidation is needed: the cache lives exactly as long as the scan and is GC'd afterward. The fresh per-scan key also keeps overlapping scans isolated, and the `null` not-found outcome is cached too. Probe results are identical to the previous direct `existsSync` calls.
….json
`analyzeProject` walked the app/ tree twice — once in `detectISR` and again
in `detectMDX` — and `detectNativeModules` re-read and re-parsed
package.json that `detectProject` had already parsed into `allDeps`.
Merge the two walkers into one `scanTreeForDetection(dir, { isr, mdx })`
that evaluates both predicates per entry with each flag short-circuiting
independently, extract the next.config MDX check into `detectMDXFromConfig`,
and add `resolveProjectDir` for the shared root/`src` precedence. Pass the
already-merged `allDeps` into `detectNativeModules`. Detection semantics are
unchanged: same files visited, same patterns tested, same booleans out.
commit: |
|
/bigbonk review for issues |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: perf optimizations in scan/deploy paths
Reviewed all three changes for behavioral parity, which is the central claim of this PR. The refactors preserve output exactly, are well-documented, and are backed by targeted tests. Ran the touched suites locally — all green:
tests/route-sorting.test.ts(47),tests/deploy.test.ts(254),tests/app-route-graph.test.ts(55),tests/routing.test.ts(112) all passvp checkclean on the three core files (format, lint, types)
1. sortRoutes (routing/utils.ts)
Decorate-sort is correct. routePrecedence is a pure function of pattern, scores are computed once, and the localeCompare tiebreaker preserves the total order, so ordering is byte-identical to the old comparator. compareRoutes has no remaining references anywhere (grepped). The decorate-sort-parity test with shuffled input is a good guard.
2. Per-scan fs-probe cache (routing/app-route-graph.ts)
Correct. findFileWithExts depends only on (dir, name, matcher.dottedExtensions); the clone shares the same dottedExtensions, and the cache key ${dir}\0${name} is deterministic. The undefined vs null distinction for cache-miss-vs-cached-miss is handled properly. Per-scan key cloning correctly isolates overlapping async scans, and the no-cache fallback path (exercised by direct helper tests) is identical to the prior findFile = findFileWithExts. The matcher is threaded consistently to every findFile/findSlotSubPages call from the entry points. Tightening findSlotSubPagesCache to the per-scan clone is a strict improvement over the prior per-build keying.
3. Single tree walk + reused package.json (deploy.ts)
Boolean outputs are equivalent: found.isr/found.mdx flip true exactly when a matching entry exists anywhere in the tree, matching the old early-return walkers. Config-first MDX precedence is preserved (mdx: !hasMDX skips the redundant fs MDX scan when config already matched, while ISR still scans). resolveProjectDir matches the old root→src/ fallback. detectNativeModules(allDeps) produces the same result as the old self-parsing version even when package.json is missing/malformed (both yield []/{}). The devDeps-only test is a nice addition.
No correctness or parity issues found. One trivial note below.
| } | ||
| } | ||
| return routes.sort((a, b) => { | ||
| const diff = (scores.get(a.pattern) ?? 0) - (scores.get(b.pattern) ?? 0); |
There was a problem hiding this comment.
Minor (non-blocking): every pattern in routes is inserted into scores in the pre-pass, so scores.get(a.pattern)/scores.get(b.pattern) can never be undefined here. The ?? 0 fallback is dead-defensive — harmless, but a reader may wonder whether a missing-score path is reachable. Fine to leave as-is.
|
Review submitted successfully. Here's a summary of my findings: Review Summary — PR #1930Verdict: Approved. This is a clean, well-tested perf refactor with behavioral parity preserved across all three changes. I verified each claim and ran the touched test suites locally (468 tests pass) plus What I verified1. 2. Per-scan fs-probe cache — 3. Single tree walk + reused package.json — Boolean outputs equivalent to the old early-return walkers; config-first MDX precedence preserved; Notes
No correctness or parity issues found. |
2 participants
Closes #1912.
The route scanners and the deploy analyzer repeat deterministic filesystem and parsing work. The route graph is rebuilt on every watcher invalidation in dev, so this lands directly on HMR rebuild latency for larger apps. All three changes preserve output exactly — identical sort order, identical probe results, identical detection booleans — so no existing expectations change.
Implemented as three reviewable commits:
1.
perf(routing): precompute route precedence before sortingcompareRoutesre-parsed every pattern on each comparison, soArray.prototype.sortcalledroutePrecedence~2·log n times per route. Replaced withsortRoutes, which scores each pattern once into aMapand sorts by(score, pattern.localeCompare). ThelocaleComparetiebreaker already guarantees a total order, so ordering is byte-identical — this just drops the redundant re-parsing on every scan (routing/utils.ts, callers inpages-router.tsandapp-route-graph.ts).2.
perf(app-router): cache convention-file probes per route-graph scanbuildAppRouteGraphwalks the appDir→leaf chain separately for every route (layouts, templates, errors, boundaries, slots), each step probing up to 4 extensions per convention file viaexistsSync. Shared ancestors — theapp/root above all — were re-stat'd once per descendant route. MemoizedfindFilethrough aWeakMap<ValidFileMatcher, Map>keyed by a per-scan matcher clone. Results are immutable within one scan, so no invalidation is needed: the cache lives exactly as long as the scan and is GC'd afterward. The fresh per-scan key keeps overlapping scans isolated, and thenullnot-found outcome is cached too.3.
perf(deploy): single tree walk for detection and reuse parsed package.jsonanalyzeProjectwalked theapp/tree twice (detectISR+detectMDX) anddetectNativeModulesre-read/re-parsedpackage.jsonthat was already parsed intoallDeps. Merged the two walkers into onescanTreeForDetection(dir, { isr, mdx })that evaluates both predicates per entry with each flag short-circuiting independently, extracteddetectMDXFromConfig, addedresolveProjectDirfor the shared root/srcprecedence, and passed the already-mergedallDepsintodetectNativeModules. Same files visited, same patterns tested, same booleans out.Tests
tests/route-sorting.test.ts:sortRoutesunit tests — documented precedence ordering, in-place/same-reference, idempotency, decorate-sort parity, empty/single-element edges.tests/app-route-graph.test.ts: sibling-route case asserting shared-ancestor layouts and nearest boundaries resolve identically (exercises the probe cache incl. thenull-miss path).tests/deploy.test.ts: combined ISR+MDX single-walk case and a devDependencies-only native-module case.All touched suites pass locally and
vp check(format, lint, types) is clean.