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: Persistent cache #4120

Closed
wants to merge 8 commits into from
Closed

feat: Persistent cache #4120

wants to merge 8 commits into from

Conversation

ArnaudBarre
Copy link
Member

@ArnaudBarre ArnaudBarre commented Jul 5, 2021

Description

Fixes #1309

For medium/big React apps, Babel become a non negligible cost to run each time you start the dev server. Caching the transformations can have a significant impact for SPA.

First run: No cache (server startup: 591ms, total time: 20s)
Second run: With cache (server startup: 1243ms, total time: 5.2s)

cache.mp4

As suggested by @AlonMiz the start up time could be improved by using parallel fs requests. I will do it at the end when the overall design is validated.

@Shinigami92 Shinigami92 added the p2-nice-to-have Not breaking anything but nice to have (priority) label Jul 5, 2021
@ArnaudBarre
Copy link
Member Author

@yyx990803 I would appreciate your take on this. I see on twitter that you are often worried when some people don't experience "fast startup" with Vite. Adding a cache layer would have a big impact for apps that are not optimised for FCP.

@Shinigami92 Shinigami92 added the YAO Yet another option... label Nov 15, 2021
@ArnaudBarre ArnaudBarre changed the title feat: cache module graph to speed up first page load feat: Persistent cache Jan 6, 2022
@Niputi
Copy link
Contributor

Niputi commented Jan 6, 2022

please fix error

[Error: ENOENT: no such file or directory, open 'D:\a\vite\vite\packages\temp\preserve-symlinks\node_modules\.vite\cache.json'

@ArnaudBarre
Copy link
Member Author

@Niputi Sorry for the trouble, I fixed the issue with open handles.

@Niputi
Copy link
Contributor

Niputi commented Jan 6, 2022

@ArnaudBarre thank you

@davidwallacejackson would you be able to try out this pull request on your big project and report back if it helps, it looks like author have had good results?

@Niputi Niputi added enhancement New feature or request and removed YAO Yet another option... labels Jan 6, 2022
Shinigami92
Shinigami92 previously approved these changes Jan 6, 2022
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Code LGTM
Have not actually test anything

@patak-dev
Copy link
Member

@ArnaudBarre we talked about this PR in our last team meeting. At this point, we aren't sure that the complexity of a persistent cache is justified. There are edge cases that appear because of it, that we will need to deal with. We think that there is a lot of value in continue exploring this option though, and this PR is a great way to do so. If you or others would like to add test cases for both React and Vue apps, and show performance metrics for real apps that could help to make the case for the feature. And as you have done with the cache PR, we could at least continue to implement smaller PRs that will make adding a persistent cache in the future a smaller jump. Thanks again for your work here, that so far is the further I've seen in this direction.

@ArnaudBarre
Copy link
Member Author

Thanks for your feedback! It's true that there will be a lot a config files that could impact the value of transformation and which are not taken into account here. I agree it's safe to not merge it.
I'm currently working on a dev server inspire by Vite which take caching as an important part of the design. There are for now two ideas I'm trying:

  • Caching at the "plugin" level, not the graph one
  • Pre-transform all the requests on the server, and have an equivalent of import-analysis which uses a hash of the transform request as a query parameter and uses strong browser caching. This requires a recursive invalidation of this plugin cache (like ssrTransformation in Vite) on file change (via watcher and at restart like in this PR)

For people using React and looking for some development perf improvements, I made this plugin to use swc just in dev.

@aleclarson
Copy link
Member

A need for caching in a dev server seems to indicate you're not lazy-loading enough of your application code. Is that a bad assumption?

@ArnaudBarre
Copy link
Member Author

In this case there is no lazy loading at all. It's a professional app where most users use 70% of the app. So downloading all the app code up front (with a vendor chunk stable across most updates) is actually a better UX IMO. Would you prefer to have a spinner when opening the settings of your editor to save 100kb on updates? Even if we are still far from offline support, lazy loading would make it more complex. And this adds some engineering complexity (keeping multiple versions available at the same time, choosing the right chunk split, managing more loading state...) which (in our case) would be better invested in terms of UX in improving our chat performances or features.

Aside from the lazy loading part, the time to run the transform functions on most/all files is still something that I will pay for when running the full e2e tests suite locally or navigating in the app across a long development session.

@ncknuna
Copy link

ncknuna commented Feb 14, 2022

I'm in a similar boat as @ArnaudBarre , where I have an enterprise app where the code splitting makes less sense. Even if we did want to move towards more lazy-loading, it would be nice for the development experience to not be so painful while the codebase is refactored to support that.

@bluwy
Copy link
Member

bluwy commented Feb 14, 2022

FWIW I have a fairly large SPA as well, and every route is not lazy-loaded for reasons above. However, the filesystem router we use has the option to "toggle" between dynamic imports or direct imports. So in dev, dynamic imports is used for all routes, while in prod, direct imports is used instead. This allows us to cut down the initial load time from around 20+s to 4s

@patak-dev
Copy link
Member

@bluwy is this something you do through a plugin that is publicly available? It is quite an interesting scheme that we could recommend in some cases

@ArnaudBarre @ncknuna thanks again for explaining your use cases. I think there is more to do in Vite, at least in the long run, to help here.

@bluwy
Copy link
Member

bluwy commented Feb 14, 2022

The import switching is a feature of https://www.routify.dev (Svelte filesystem router), which dynamically generates a routing manifest JS file that alternates between dynamic imports and direct imports (docs -- dynamicImports). Not sure if this can be done router-agnostic with a Vite plugin though, but with some grease it could be possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Persistent Cache
9 participants