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

Batch writes #3771

Merged
merged 7 commits into from
Nov 21, 2024
Merged

Batch writes #3771

merged 7 commits into from
Nov 21, 2024

Conversation

simolus3
Copy link
Contributor

Another attempt for #3321, my previous PR for this got stale, this one is a bit simpler.

This attempts to solve the issue of build_runner and external tools watching for file system events (like the analysis server) not playing too well together. At the moment, a common issue is that, as a user runs build_runner build and the generated assets are written as they are generated, the analysis server will see many file changes and keep re-analyzing sources in an incomplete state.
By making build_runner cache pending writes in memory and then flushing them after a completed build, we'll likely get all changes to disk before they're picked up by external tools, reducing friction.

I've implemented this at a fairly low level (the raw file readers and writers making up the default IOEnvironment). They have to opt-in to batches by checking for the presence of a zone key when reading or writing assets. We support batches by default when not in low-resources mode. A batch runs in a zone wrapping a whole build run.

A todo is that I need to write integration tests for this, but it would be good to get a review for the overall approach before completing this.

Closes #3321.

Copy link

github-actions bot commented Nov 15, 2024

PR Health

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

build_runner_core/lib/src/asset/batch.dart Outdated Show resolved Hide resolved
build_runner_core/lib/src/asset/batch.dart Outdated Show resolved Hide resolved
build_runner_core/lib/src/asset/batch.dart Outdated Show resolved Hide resolved
build_runner_core/lib/src/asset/batch.dart Outdated Show resolved Hide resolved
build_runner/lib/src/generate/build.dart Outdated Show resolved Hide resolved
@jakemac53
Copy link
Contributor

@simolus3 looks like some test utilites also need to implement the new method :)

@simolus3
Copy link
Contributor Author

I'll need someone to approve the workflow again :)

@jakemac53
Copy link
Contributor

Given this is effectively tested by the regular integration tests, I am fine with landing this now if you are. Ready for me to merge?

@simolus3
Copy link
Contributor Author

I'm fine with the current state too 👍 I mainly had a "no files are visible before the build has completed" idea for integration tests, but since we have unit tests for the functionality and integration tests ensuring this didn't break anything, I think this is ready.

@jakemac53 jakemac53 merged commit f89332a into dart-lang:master Nov 21, 2024
76 checks passed
@simolus3 simolus3 deleted the delay-writes branch November 21, 2024 21:44
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.

Proposal: Add a mode where all actual file system interactions are delayed until the build completes
2 participants