Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

perf(vite): remove vite warm up #6227

Closed
wants to merge 2 commits into from
Closed

perf(vite): remove vite warm up #6227

wants to merge 2 commits into from

Conversation

antfu
Copy link
Member

@antfu antfu commented Jul 29, 2022

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This reduces the start-up time of nuxt.com repo with viteNode: true (nuxi dev to first render on the browser)
from 60s to 40s on my M1 Max Macbook Pro.

Reasoning:

  • Vite will now do warm-up for sync deps automatically.
  • Our warm-up uses server.transformRequest(url) for both client/server. Where the server bundler actually needs server.transformRequest(url, { ssr: true }), which is actually using a different plugin pipeline and caches.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@netlify
Copy link

netlify bot commented Jul 29, 2022

βœ… Deploy Preview for nuxt3-docs canceled.

Name Link
πŸ”¨ Latest commit f0a7311
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/62fa622147e8880008a4a2e5

@pi0
Copy link
Member

pi0 commented Jul 29, 2022

Vite will now do warm-up for sync deps automatically.

Does Vite also warmups the entrypoint in background? (Same for vite-node with ssr condition)

Our warm-up uses server.transformRequest(url) for both client/server. Where the server bundler actually needs server.transformRequest(url, { ssr: true }), which is actually using a different plugin pipeline and caches.

Nice find and probably to late to fix if warmup is not relevant anymore πŸ˜…

import type { Options } from '@vitejs/plugin-vue'
import replace from '@rollup/plugin-replace'
import { sanitizeFilePath } from 'mlly'
import { buildClient } from './client'
import { buildServer } from './server'
import virtual from './plugins/virtual'
import { warmupViteServer } from './utils/warmup'
Copy link
Member

Choose a reason for hiding this comment

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

If this is not relevant anymore I think we can remove util as well. Only I want to make sure vite and vite-node does the same because in past, without entrywarmup we were simply delaying it until first request.

@antfu
Copy link
Member Author

antfu commented Jul 29, 2022

I also tried to only warm up the entry (and let Vite crawl the deps), it ends up around 50s start-up time. I guess it might be because it occupied the execution time of other parts. I could do a more detailed benchmarking, and probably try delaying the warm-up with a timeout.

@pi0
Copy link
Member

pi0 commented Jul 29, 2022

I suggest this method to benchmark:

  • Cleanup cache with nuxi cleanup each time
  • In parallel initiate a fetch to http://localhost:300 and start nuxi dev
    • Measure time to first byte for SSR perf
  • Open a new browser window ensuring no other windows are open
    • Measure time to load for entrypoint scripts

@antfu antfu marked this pull request as draft July 29, 2022 12:39
@pi0
Copy link
Member

pi0 commented Aug 15, 2022

Let's experiment this as an option that we can disable. (sorry there were merge conflicts otherwise would push to same PR) ~> #6647.

We should also track this:

Our warm-up uses server.transformRequest(url) for both client/server. Where the server bundler actually needs server.transformRequest(url, { ssr: true }), which is actually using a different plugin pipeline and caches.

~> #6649

@pi0 pi0 closed this Aug 15, 2022
@pi0 pi0 deleted the perf/vite-remove-warm-up branch August 15, 2022 15:19
@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants