Skip to content

perf: non-blocking write of optimized dep files#12603

Merged
patak-cat merged 8 commits intomainfrom
perf/commit-optimized-deps-on-the-background
Mar 27, 2023
Merged

perf: non-blocking write of optimized dep files#12603
patak-cat merged 8 commits intomainfrom
perf/commit-optimized-deps-on-the-background

Conversation

@patak-cat
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

@bolt-new-by-stackblitz
Copy link

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-cat
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.

// 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-cat
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-cat patak-cat merged commit 2f5f968 into main Mar 27, 2023
@patak-cat patak-cat 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