Add option to delay file system writes #3418
Conversation
|
|
|
Oh wait nvm, the linked issue, apparently filed by me, describes the reasoning, LOL. |
|
cc @natebosch did you want to review this as well? |
There was a problem hiding this comment.
I was initially worried, but I don't think it should be possible for this cache to have the same content as the CachingAssetReader - we should always invalidate an asset before it could be possible to write it, and we'd never read an asset from disk before writing it during the build.
| }); | ||
| bool? delayAssetWrites, | ||
| }) : | ||
| // Delayed asset writes should be enabled by default if we're not in the |
There was a problem hiding this comment.
How confident are we that this is the right choice for the default?
example/pubspec.yaml
Outdated
| build_runner: | ||
| path: ../build_runner | ||
| build_runner_core: | ||
| path: ../build_runner_core |
|
new failure here looks potentially related |
|
I think that particular failure is from the Ah, already fixed in a better way by #3429 :D |
|
Does the test pass if we revert the last commit "Fix symlink test" and merge master? I'd prefer to avoid the |
|
Were you referring to b93d534? Unfortunately, I agree that the |
|
Sorry for the slow response. Yes, I think merging the interface of I'd guess few implementation won't be able to implement it, but I haven't looked closely. |
|
@simolus3 is this still WIP? Closing this for now as it is stale and the files are out of date. Feel free to reopen in case you still want to land it! |
* Use batches for builds * Add changelog entry to build runner * Review feedback aAnother attempt for #3321, my [previous PR](#3418) 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. * Bump version of build_runner_core * Make batch private, fix dependency resolution * Start fixing tests * Add lints dependency for out-of-workspace pkg
This adds a
--delay-writesoption to thebuild,watch,serveanddaemoncommands. When enabled, file system writes will only happen once at the end of a build.We can determine the impact of this option on larger builds (in terms of how the analyzer reacts, memory used by the build system, ...). If it turns out that some refinements are needed (like only doing this for Dart files or source assets), we can improve the option in subsequent releases. Once everything works, we might want to make this the default mode (at least if not using the low resource mode).
This probably needs more integration tests, but I'm not sure what the place best to add them would be.
Closes #3321