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(optimizer): bulk optimizer delay #12609

Merged
merged 1 commit into from Mar 27, 2023

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Mar 27, 2023

Description

runOptimizerWhenIdle was called many times.
Also it was called serially every time waiting for .done() to be completed.

This PR reduces runOptimizerWhenIdle by calling multiple .done() in a single runOptimizerWhenIdle call. Also this PR calls .done() parallelly.

I ran this PR on https://github.com/sapphi-red/performance-compare/tree/65d51a32b0282593f1b75def25075a02b28f6397.
Without this PR (420782c76733ecc8314bccc2255976347add3d2e), start up time (with --force) was 16422ms with plugin-react.
With the PR, start up time was 4784ms. This is about 3.4x improvement.

I guess this improves performance if there's many files.

More detailed result of the benchmark: https://gist.github.com/sapphi-red/d1ee341566a052703732238788cb0447

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@sapphi-red sapphi-red added the performance Performance related enhancement label Mar 27, 2023
@stackblitz
Copy link

stackblitz bot commented Mar 27, 2023

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

registeredIds.length > 0 ||
results.some((result) => result.status === 'rejected')
) {
afterLoad()
Copy link
Member

Choose a reason for hiding this comment

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

The setTimeout 0 here was added at the time because things were failing if we did a direct call. It may have been a bug that prevented us to do it. I think we can try it out without it now, let's keep in mind that we could add it later here if needed (I don't think it would affect perf)

Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

It is incredible how much of a difference this makes. Awesome catch @sapphi-red! 🔥

@patak-dev patak-dev merged commit c881971 into vitejs:main Mar 27, 2023
14 checks passed
@sapphi-red sapphi-red deleted the perf/bulk-optimizer-delay branch March 27, 2023 17:10
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.

None yet

2 participants