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

feat(cli): build --profile #10719

Merged
merged 2 commits into from Nov 10, 2022
Merged

feat(cli): build --profile #10719

merged 2 commits into from Nov 10, 2022

Conversation

ArnaudBarre
Copy link
Member

@ArnaudBarre ArnaudBarre commented Oct 30, 2022

This will allow to profile vite build to detect performance issues in plugins only applied at build time.

Usage example:
Screenshot 2022-11-09 at 04 14 30
Screenshot 2022-11-09 at 04 14 56

Note: There is a small regression on the initial build message in Vite 4

@ArnaudBarre ArnaudBarre changed the title feat(build): enable --profile feat(cli): build --profile Nov 9, 2022
patak-dev
patak-dev previously approved these changes Nov 9, 2022
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.

Thanks for the PR @ArnaudBarre, I think this feature makes now a lot more sense for build than its current state for dev.

Looks like it is currently only measuring the config and buildStart hooks for plugins and the HTTP server start during dev. At one point it was also measuring pre-bundling of dependencies but that was now moved after the server start. Maybe we should stop the profiler during dev once every statically imported module has been processed, deps prebundled, and the server is idle. We have that signal now. But this could be discussed in another PR.

@ArnaudBarre
Copy link
Member Author

I though of that and was thinking of a dumb "setTimeout 10_000" but that not ideal. What is this signal?

Or maybe we should only write on server exit so that people can profile HMR?

@patak-dev
Copy link
Member

The signal is currently tied to the optimizer here. We use it during dev to delay sending to the browser optimized dependencies that may be invalidated if there is a missing dependency. We could extract the onCrawlEnd signal out of the optimizer and use it to have a new onServerIdle (after crawl end + the optimizer was re-run if needed). This would measure the real work done by the server to get the full static module graph to the browser. Another way is to let the browser send us a message through websockets when has loaded, maybe that is even more reliable.

But interesting idea about profiling HMR. Maybe by default having the profiler stop when the server exits is more useful. Another idea is to have something like #9673 and a shortcut to start and stop the profiler at any point during the server run. Or maybe having Vite output several profiles, like:

  • vite-profile:start.cpuprofile
  • vite-profile:hmr-1.cpuprofile
  • vite-profile:hmr-2.cpuprofile

@ArnaudBarre
Copy link
Member Author

ArnaudBarre commented Nov 9, 2022

Thanks pour the pointers. I will update the PR this evening.

Interesting idea for using the current shortcuts PR pour profiling HMR. Do you want to go forward with this PR? I did a review. I like some of the possibilities, even if it could mean a yet another cli shortcut GitHub label in the future 😁

@patak-dev
Copy link
Member

Thanks for reviewing #9673 @ArnaudBarre. I think we shouldn't block this PR waiting for it. Let's address Blue comment to stop at finally in both dev and build, and we can merge this one. We can then work the details of #9673 without additional shortcuts, it would be good to get it in Vite 4. And once that one is merged, we can propose additional shortcuts (like start/stop profiling). I hope we don't need that new label though 😅

@ArnaudBarre
Copy link
Member Author

ArnaudBarre commented Nov 10, 2022

I added also preview because that's cheap

I looked at the onCrawlEnd but it would be too hacky to put it there I think.

Maybe exposing something like:

server.onInitialLoadEnded(cb: () => void)

Not the best name, but I would like both something that is not tied to the optimiser and only call once.

@patak-dev patak-dev requested a review from bluwy November 10, 2022 06:32
@patak-dev patak-dev merged commit 9c808cd into vitejs:main Nov 10, 2022
@ArnaudBarre ArnaudBarre deleted the profile-build branch November 10, 2022 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants