Skip to content

perf(ssr): optimize isCircularImport algorithm for large codebases#20085

Closed
hanford wants to merge 7 commits intovitejs:mainfrom
hanford:patch-1
Closed

perf(ssr): optimize isCircularImport algorithm for large codebases#20085
hanford wants to merge 7 commits intovitejs:mainfrom
hanford:patch-1

Conversation

@hanford
Copy link
Copy Markdown

@hanford hanford commented May 21, 2025

Description

I swapped out the old recursive “visited” set for a simple 3-state map (unvisited, visiting, done) that walks each module only once. As soon as we hit a module marked “visiting,” we know there’s a cycle and bail out.

Why It Matters

  1. Old code would re-scan big dependency trees many times
  2. The single integer map tracks both “on the stack” and “already safe” status
  3. Early exit on cycle means less wasted time/work
  4. Startup time dropped noticeably in our big repo

fixes #20084

@hanford hanford changed the title Implement disjoint set in isCircularImport refactor: Implement faster algorithm for isCircularImport check May 21, 2025
@hanford hanford changed the title refactor: Implement faster algorithm for isCircularImport check refactor(isCircularImport): implement faster isCircularImport algorithm May 21, 2025
@hanford hanford changed the title refactor(isCircularImport): implement faster isCircularImport algorithm refactor(isCircularImport): Optimize isCircularImport algorithm for large codebases May 21, 2025
@hanford hanford changed the title refactor(isCircularImport): Optimize isCircularImport algorithm for large codebases refactor(isCircularImport): optimize isCircularImport algorithm for large codebases May 21, 2025
@sapphi-red sapphi-red added the p2-nice-to-have Not breaking anything but nice to have (priority) label May 22, 2025
@sapphi-red sapphi-red changed the title refactor(isCircularImport): optimize isCircularImport algorithm for large codebases perf(ssr): optimize isCircularImport algorithm for large codebases May 22, 2025
sapphi-red
sapphi-red previously approved these changes May 22, 2025
Copy link
Copy Markdown
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This is nice.

Startup time dropped noticeably in our big repo

Do you have a concrete value of this? I understand that the performance should improve theoretically due to the reduced algorithmic complexity, but I’m curious about the actual numbers.

@hanford
Copy link
Copy Markdown
Author

hanford commented May 22, 2025

I can share concrete numbers tomorrow morning!

@hanford
Copy link
Copy Markdown
Author

hanford commented May 22, 2025

I think I found some issues with this approach, will report back after taking a deeper look

@sapphi-red sapphi-red dismissed their stale review May 23, 2025 00:48

see comment above

@sapphi-red
Copy link
Copy Markdown
Member

sapphi-red commented May 23, 2025

I'll convert this to draft for now.

@sapphi-red sapphi-red marked this pull request as draft May 23, 2025 00:56
@sapphi-red
Copy link
Copy Markdown
Member

/ecosystem-ci run

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Aug 8, 2025

Open in StackBlitz

npm i https://pkg.pr.new/vite@20085

commit: 5912c0b

@vite-ecosystem-ci
Copy link
Copy Markdown

@sapphi-red
Copy link
Copy Markdown
Member

@hanford This PR seems to work fine for the projects in ecosystem-ci. What was the issue you found?

@hanford
Copy link
Copy Markdown
Author

hanford commented Aug 11, 2025

I don't recall if there was a significant performance impact or not.

@sapphi-red
Copy link
Copy Markdown
Member

closing as #21632 is merged

@sapphi-red sapphi-red closed this Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat: ssr p2-nice-to-have Not breaking anything but nice to have (priority) trigger: preview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isCircularImport complexity impacting Vite performance

2 participants