Skip to content

perf: lazy load rollup during dev#15621

Merged
patak-cat merged 7 commits into
mainfrom
perf/lazy-rollup-on-dev
Jan 23, 2024
Merged

perf: lazy load rollup during dev#15621
patak-cat merged 7 commits into
mainfrom
perf/lazy-rollup-on-dev

Conversation

@patak-cat
Copy link
Copy Markdown
Member

@patak-cat patak-cat 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-cat patak-cat added the performance Performance related enhancement label Jan 16, 2024
@bolt-new-by-stackblitz
Copy link
Copy Markdown

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

@patak-cat

This comment was marked as outdated.

@vite-ecosystem-ci

This comment was marked as outdated.

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

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

Comment thread packages/vite/src/node/utils.ts
sheremet-va
sheremet-va previously approved these changes Jan 18, 2024
Comment thread packages/vite/src/node/publicUtils.ts
@bluwy
Copy link
Copy Markdown
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-cat
Copy link
Copy Markdown
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.

Comment thread packages/vite/src/node/plugins/esbuild.ts Outdated
@bluwy
Copy link
Copy Markdown
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-cat patak-cat mentioned this pull request Jan 22, 2024
4 tasks
This reverts commit 02f4bd5.
This reverts commit af14ea0.
@patak-cat
Copy link
Copy Markdown
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-cat patak-cat merged commit 6f88a90 into main Jan 23, 2024
@patak-cat patak-cat 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