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: non-blocking write of optimized dep files #12603

Merged
merged 8 commits into from
Mar 27, 2023

Conversation

patak-dev
Copy link
Member

Description

Keep the esbuild output files in memory so we can avoid waiting for the write of all optimized deps and then read from disk.

Implements a _writing lock of the deps cache that is checked when reading the cached deps to avoid issues if the user stopped the process in the middle of writing the optimized deps. @dominikg thinks this isn't enough, but given that two processes shouldn't be working with the same cache dir in the same root I think this is enough.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@stackblitz
Copy link

stackblitz bot commented Mar 26, 2023

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

if (isDepsOptimizerEnabled(config, false)) {
depsOptimizerReady = initDepsOptimizer(config)
// start optimizer in the background, we still need to await the setup
await initDepsOptimizer(config)
Copy link
Member Author

Choose a reason for hiding this comment

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

@bluwy we need to await the optimizer init here. The setup is very lean and it starts the processing in the background so this shouldn't take more than a few ms. The issue was evident when I tried to implement the wait 500ms when there is a _writing file.

@patak-dev
Copy link
Member Author

We'll need to keep an eye on this one https://github.com/vitejs/vite/actions/runs/4526804238/jobs/7972195375#step:13:94, I don't know if this appears in this PR. It could be related to previous ones.

packages/vite/src/node/optimizer/index.ts Outdated Show resolved Hide resolved
packages/vite/src/node/optimizer/index.ts Outdated Show resolved Hide resolved
@@ -67,7 +70,7 @@ export function optimizedDepsPlugin(config: ResolvedConfig): Plugin {
// load hooks to avoid race conditions, once processing is resolved,
// we are sure that the file has been properly save to disk
try {
return await fs.readFile(file, 'utf-8')
return loadOptimizedDep(file, depsOptimizer)
Copy link
Member Author

Choose a reason for hiding this comment

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

Could we stream .contents, or use them here?

Copy link
Member

Choose a reason for hiding this comment

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

I think we need to make it a string here. For sourcemaps, since they're handled in the transform middleware, we could, but I tried that and it doesn't work since etags are generated via the Buffer instance, and Buffer.isBuffer(new Uint8Array(2)) is false.

@patak-dev
Copy link
Member Author

Thanks for the changes @bluwy! Let's merge this now so we also fix the optimizer setup before the server.

@patak-dev patak-dev merged commit 2f5f968 into main Mar 27, 2023
@patak-dev patak-dev deleted the perf/commit-optimized-deps-on-the-background branch March 27, 2023 14:03
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.

2 participants