perf(ssr): skip circular import check for already-evaluated modules #21632
+7
−2
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.
Description
Hey, thanks for all the work on Vite — really appreciate it.
I was looking into #20084 and the stalled #20085, and wanted to pick up where @hanford left off. @sapphi-red asked for concrete benchmark numbers on that PR, so I focused on producing those alongside a minimal fix.
The problem
isCircularImportperforms a DFS traversal on every call tocachedRequest, including for modules that have already been fully evaluated. SincecachedRequestis called once per import edge, the total cost of these traversals grows superlinearly with module count — our profiling shows roughly quadratic scaling behavior in practice. At 25K modules this adds up to tens of millions of graph traversals, most of which are unnecessary.The fix
A 4-line early return: if a module is already evaluated (
mod.evaluated && mod.promise), skip the circular check and return the cached result directly. Resolved promises can't deadlock, so the circular dependency check is unnecessary for fully-evaluated modules.CPU profile (baseline, before fix)
CPU profiling with
--cpu-profshowsisCircularImportconsuming the majority of JavaScript execution time in the baseline. The flame graph above (via speedscope, Left Heavy view) shows the recursive DFS calls stacking up across the entire profile.Benchmark results
Synthetic module graph benchmark (configurable N, avg 3 imports/module, 5% cycle density). We instrumented
cachedRequestandisCircularImportto get exact call counts, then measured wall-clock time across multiple scales:isCircularImportcallsScaling is superlinear in both cases (doubling N gives 3-4x time), but the fix cuts absolute time by 2-2.5x across all scales. At the issue reporter's 25K module scale, this should meaningfully reduce the 60-90s overhead they reported.
Why this is safe
mod.evaluated = trueis set in thefinallyblock ofcachedRequestonly afterdirectRequestcompletesTest results
All 37 server-runtime tests pass (including all circular/cyclic import scenarios). Full unit, serve, and build test suites pass with zero failures.
Further optimization
The remaining superlinear behavior comes from the fact that each module's first evaluation still triggers a DFS traversal. Incremental cycle detection or memoized results could address this in a follow-up, but this minimal change provides meaningful improvement with very low risk.
Happy to help with anything else on this issue.
Fixes #20084