Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: lazy load rollup during dev #15621

Merged
merged 7 commits into from
Jan 23, 2024
Merged

perf: lazy load rollup during dev #15621

merged 7 commits into from
Jan 23, 2024

Conversation

patak-dev
Copy link
Member

@patak-dev patak-dev commented Jan 16, 2024

Description

Edit:
This PR now only avoids statically importing rollup before the dev server starts.
Other parts have been extracted to:

Context

We merged some PRs that moved rollup from being lazily loaded during dev (see for example a here for a properly use of rollup) to being statically imported and blocking the server init.

This PR reworks the changes done in the following PRs so rollup is lazily loaded again:

With this PR, the dev server is starting ~15% faster on my M1, seeing sub 100ms now.

For #14833, if we want to continue to expose the sync parseAst from rollup, we need to do it by also exporting an initRollupParseAst function that should be called before it. This is breaking but I don't think there should be much usage for it yet. I can break the PR if needed but it would be good if we could merge it for Vite 5.1 and not have to wait for Vite 6

cc @sapphi-red, @sheremet-va, @bluwy as we were all involved in reviewing the PRs above


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@patak-dev patak-dev added the performance Performance related enhancement label Jan 16, 2024
Copy link

stackblitz bot commented Jan 16, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@patak-dev

This comment was marked as outdated.

@vite-ecosystem-ci

This comment was marked as outdated.

@patak-dev patak-dev mentioned this pull request Jan 18, 2024
4 tasks
@patak-dev
Copy link
Member Author

Extracted the rollup/parseAst changes into #15639, so this one is easier to review and merge.

sheremet-va
sheremet-va previously approved these changes Jan 18, 2024
@bluwy
Copy link
Member

bluwy commented Jan 18, 2024

I'm not really oppose to this, but it's kinda unfortunate that we can't use the exported version now. Should we reach to Rollup if they could export it as a subpath? (though feels a bit redundant, i guess it would be nice if the utils are placed under rollup/utils perhaps)

@patak-dev
Copy link
Member Author

I'm not really oppose to this, but it's kinda unfortunate that we can't use the exported version now. Should we reach to Rollup if they could export it as a subpath? (though feels a bit redundant, i guess it would be nice if the utils are placed under rollup/utils perhaps)

Do you mean that rollup should expose a small rollup/utils entry point with the version, and maybe later things like normalizePath? We can ping Lukas to see what he thinks later, but we could move forward with the PR and improve things later when we have a better story upstream.

@bluwy
Copy link
Member

bluwy commented Jan 22, 2024

Yeah I think we can figure out the rollup/utils thing later and we can do this for now. Sorry for not responding earlier, I'd prefer the esbuild changes done separately, or even not done at all 😬 I feel like we're using esbuild a lot more frequent than Rollup, and it's not really worth optimizing the esbuild import.

@patak-dev patak-dev mentioned this pull request Jan 22, 2024
4 tasks
@patak-dev
Copy link
Member Author

Yeah I think we can figure out the rollup/utils thing later and we can do this for now. Sorry for not responding earlier, I'd prefer the esbuild changes done separately, or even not done at all 😬 I feel like we're using esbuild a lot more frequent than Rollup, and it's not really worth optimizing the esbuild import.

I extracted the esbuild changes as a draft to

This PR only avoids statically loading rollup before the server starts now. I need to run proper benchmarks with the other extracted PRs so we can discuss them further 👍🏼

@patak-dev patak-dev merged commit 6f88a90 into main Jan 23, 2024
16 checks passed
@patak-dev patak-dev deleted the perf/lazy-rollup-on-dev branch January 23, 2024 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance related enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants