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

Add option to delay file system writes #3418

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions _test_common/lib/in_memory_reader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,20 @@ class InMemoryRunnerAssetReader extends InMemoryAssetReader
final _onCanReadController = StreamController<AssetId>.broadcast();
Stream<AssetId> get onCanRead => _onCanReadController.stream;

@override
bool get supportsFindingAssetPaths => false;

@override
Future<bool> canRead(AssetId id) {
_onCanReadController.add(id);
return super.canRead(id);
}

@override
String pathTo(AssetId id) {
throw UnsupportedError('Path for asset id $id');
}

InMemoryRunnerAssetReader(
[Map<AssetId, dynamic>? sourceAssets, String? rootPackage])
: super(sourceAssets: sourceAssets, rootPackage: rootPackage);
Expand Down
3 changes: 3 additions & 0 deletions _test_common/lib/in_memory_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,7 @@ class InMemoryRunnerAssetWriter extends InMemoryAssetWriter
FakeWatcher.notifyWatchers(WatchEvent(
ChangeType.REMOVE, p.absolute(id.package, p.fromUri(id.path))));
}

@override
void onBuildComplete() {}
}
3 changes: 3 additions & 0 deletions _test_common/lib/runner_asset_writer_spy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,7 @@ class RunnerAssetWriterSpy extends AssetWriterSpy implements RunnerAssetWriter {
_assetsDeleted.add(id);
return _delegate.delete(id);
}

@override
FutureOr<void> onBuildComplete() => _delegate.onBuildComplete();
}
2 changes: 2 additions & 0 deletions build_modules/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,5 @@ dev_dependencies:
dependency_overrides:
build_runner:
path: ../build_runner
build_runner_core:
path: ../build_runner_core
3 changes: 3 additions & 0 deletions build_runner/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

## 2.4.0

- Add `--delay-writes` option to delay all writes to the end of a build. As no
intermediate build state is visible on the file system, this may improve
performance of third-party tools like the analysis server.
- Warn if a `package:` builder import cannot be resolved and skip it,
instead of creating an invalid build script or failing in other obscure ways.
- Require Dart 3.0, drop support for unsound build scripts.
Expand Down
19 changes: 12 additions & 7 deletions build_runner/lib/src/daemon/daemon_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'package:build_daemon/data/build_status.dart';
import 'package:build_daemon/data/build_target.dart' hide OutputLocation;
import 'package:build_daemon/data/server_log.dart';
import 'package:build_runner/src/entrypoint/options.dart';
import 'package:build_runner/src/generate/environment.dart';
import 'package:build_runner/src/package_graph/build_config_overrides.dart';
import 'package:build_runner/src/watcher/asset_change.dart';
import 'package:build_runner/src/watcher/change_filter.dart';
Expand Down Expand Up @@ -199,13 +200,17 @@ class BuildRunnerDaemonBuilder implements DaemonBuilder {
var expectedDeletes = <AssetId>{};
var outputStreamController = StreamController<ServerLog>();

var environment = OverrideableEnvironment(
IOEnvironment(packageGraph,
outputSymlinksOnly: daemonOptions.outputSymlinksOnly),
onLog: (record) {
outputStreamController.add(ServerLog.fromLogRecord(record));
});

var environment = createDefaultEnvironment(
packageGraph: packageGraph,
assumeTty: null,
outputSymlinksOnly: daemonOptions.outputSymlinksOnly,
reader: null,
writer: null,
delayAssetWrites: daemonOptions.delayAssetWrites,
onLog: (record) =>
outputStreamController.add(ServerLog.fromLogRecord(record)),
verbose: true,
);
var daemonEnvironment = OverrideableEnvironment(environment,
writer: OnDeleteWriter(environment.writer, expectedDeletes.add));

Expand Down
8 changes: 7 additions & 1 deletion build_runner/lib/src/entrypoint/base_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,13 @@ abstract class BuildRunnerCommand extends Command<int> {
'If multiple filters are applied then outputs matching any filter '
'will be built (they do not need to match all filters).')
..addMultiOption(enableExperimentOption,
help: 'A list of dart language experiments to enable.');
help: 'A list of dart language experiments to enable.')
..addFlag(
jakemac53 marked this conversation as resolved.
Show resolved Hide resolved
delayWritesOption,
help: 'Delays all file system interactions until the end of a build, '
'potentially reducing load on tools like the analysis server.',
defaultsTo: null,
jakemac53 marked this conversation as resolved.
Show resolved Hide resolved
);
}

/// Must be called inside [run] so that [argResults] is non-null.
Expand Down
1 change: 1 addition & 0 deletions build_runner/lib/src/entrypoint/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class BuildCommand extends BuildRunnerCommand {
trackPerformance: options.trackPerformance,
skipBuildScriptCheck: options.skipBuildScriptCheck,
logPerformanceDir: options.logPerformanceDir,
delayAssetWrites: options.delayAssetWrites,
);
if (result.status == BuildStatus.success) {
return ExitCode.success.code;
Expand Down
17 changes: 16 additions & 1 deletion build_runner/lib/src/entrypoint/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const skipBuildScriptCheckOption = 'skip-build-script-check';
const symlinkOption = 'symlink';
const usePollingWatcherOption = 'use-polling-watcher';
const verboseOption = 'verbose';
const delayWritesOption = 'delay-writes';

enum BuildUpdatesOption { none, liveReload }

Expand Down Expand Up @@ -67,6 +68,9 @@ class SharedOptions {
/// Enables performance tracking and the `/$perf` page.
final bool trackPerformance;

/// Delay asset writes until the end of a build.
final bool delayAssetWrites;

/// A directory to log performance information to.
String? logPerformanceDir;

Expand Down Expand Up @@ -100,7 +104,11 @@ class SharedOptions {
required this.isReleaseBuild,
required this.logPerformanceDir,
required this.enableExperiments,
});
bool? delayAssetWrites,
}) :
// Delayed asset writes should be enabled by default if we're not in the
Copy link
Member

Choose a reason for hiding this comment

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

How confident are we that this is the right choice for the default?

// low-resources mode.
delayAssetWrites = delayAssetWrites ?? !enableLowResourcesMode;

SharedOptions.fromParsedArgs(ArgResults argResults,
Iterable<String> positionalArgs, String rootPackage, Command command)
Expand All @@ -122,6 +130,7 @@ class SharedOptions {
isReleaseBuild: argResults[releaseOption] as bool,
logPerformanceDir: argResults[logPerformanceOption] as String?,
enableExperiments: argResults[enableExperimentOption] as List<String>,
delayAssetWrites: argResults[delayWritesOption] as bool?,
);
}

Expand All @@ -147,6 +156,7 @@ class DaemonOptions extends WatchOptions {
required String? logPerformanceDir,
required bool usePollingWatcher,
required List<String> enableExperiments,
required super.delayAssetWrites,
}) : super._(
buildFilters: buildFilters,
deleteFilesByDefault: deleteFilesByDefault,
Expand Down Expand Up @@ -202,6 +212,7 @@ class DaemonOptions extends WatchOptions {
logPerformanceDir: argResults[logPerformanceOption] as String?,
usePollingWatcher: argResults[usePollingWatcherOption] as bool,
enableExperiments: argResults[enableExperimentOption] as List<String>,
delayAssetWrites: argResults[delayWritesOption] as bool?,
);
}
}
Expand Down Expand Up @@ -229,6 +240,7 @@ class WatchOptions extends SharedOptions {
required bool isReleaseBuild,
required String? logPerformanceDir,
required List<String> enableExperiments,
required super.delayAssetWrites,
}) : super._(
buildFilters: buildFilters,
deleteFilesByDefault: deleteFilesByDefault,
Expand Down Expand Up @@ -266,6 +278,7 @@ class WatchOptions extends SharedOptions {
logPerformanceDir: argResults[logPerformanceOption] as String?,
usePollingWatcher: argResults[usePollingWatcherOption] as bool,
enableExperiments: argResults[enableExperimentOption] as List<String>,
delayAssetWrites: argResults[delayWritesOption] as bool?,
);
}

Expand Down Expand Up @@ -295,6 +308,7 @@ class ServeOptions extends WatchOptions {
required String? logPerformanceDir,
required bool usePollingWatcher,
required List<String> enableExperiments,
required super.delayAssetWrites,
}) : super._(
buildFilters: buildFilters,
deleteFilesByDefault: deleteFilesByDefault,
Expand Down Expand Up @@ -381,6 +395,7 @@ class ServeOptions extends WatchOptions {
logPerformanceDir: argResults[logPerformanceOption] as String?,
usePollingWatcher: argResults[usePollingWatcherOption] as bool,
enableExperiments: argResults[enableExperimentOption] as List<String>,
delayAssetWrites: argResults[delayWritesOption] as bool?,
);
}
}
Expand Down
29 changes: 17 additions & 12 deletions build_runner/lib/src/generate/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import 'package:logging/logging.dart';
import 'package:shelf/shelf.dart';
import 'package:watcher/watcher.dart';

import '../logging/std_io_logging.dart';
import '../package_graph/build_config_overrides.dart';
import '../server/server.dart';
import 'environment.dart';
import 'watch_impl.dart' as watch_impl;

/// Runs all of the BuilderApplications in [builders] once.
Expand Down Expand Up @@ -68,7 +68,8 @@ Future<BuildResult> build(List<BuilderApplication> builders,
bool? isReleaseBuild,
Map<String, Map<String, dynamic>>? builderConfigOverrides,
String? logPerformanceDir,
Set<BuildFilter>? buildFilters}) async {
Set<BuildFilter>? buildFilters,
bool? delayAssetWrites}) async {
builderConfigOverrides ??= const {};
buildDirs ??= <BuildDirectory>{};
buildFilters ??= <BuildFilter>{};
Expand All @@ -80,15 +81,18 @@ Future<BuildResult> build(List<BuilderApplication> builders,
skipBuildScriptCheck ??= false;
trackPerformance ??= false;
verbose ??= false;
var environment = OverrideableEnvironment(
IOEnvironment(
packageGraph,
assumeTty: assumeTty,
outputSymlinksOnly: outputSymlinksOnly,
),
reader: reader,
writer: writer,
onLog: onLog ?? stdIOLogListener(assumeTty: assumeTty, verbose: verbose));

final environment = createDefaultEnvironment(
packageGraph: packageGraph,
assumeTty: assumeTty,
outputSymlinksOnly: outputSymlinksOnly,
reader: reader,
writer: writer,
delayAssetWrites: delayAssetWrites,
onLog: onLog,
verbose: verbose,
);

var logSubscription =
LogSubscription(environment, verbose: verbose, logLevel: logLevel);
var options = await BuildOptions.create(
Expand Down Expand Up @@ -165,7 +169,8 @@ Future<ServeHandler> watch(List<BuilderApplication> builders,
bool? isReleaseBuild,
Map<String, Map<String, dynamic>>? builderConfigOverrides,
String? logPerformanceDir,
Set<BuildFilter>? buildFilters}) =>
Set<BuildFilter>? buildFilters,
bool? delayAssetWrites}) =>
watch_impl.watch(
builders,
assumeTty: assumeTty,
Expand Down
36 changes: 36 additions & 0 deletions build_runner/lib/src/generate/environment.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import 'package:build_runner_core/build_runner_core.dart';
import 'package:logging/logging.dart';

import '../logging/std_io_logging.dart';

BuildEnvironment createDefaultEnvironment({
required PackageGraph packageGraph,
required bool? assumeTty,
required bool outputSymlinksOnly,
required RunnerAssetReader? reader,
required RunnerAssetWriter? writer,
required bool? delayAssetWrites,
required void Function(LogRecord)? onLog,
required bool verbose,
}) {
BuildEnvironment environment = IOEnvironment(
packageGraph,
assumeTty: assumeTty,
outputSymlinksOnly: outputSymlinksOnly,
);
reader ??= environment.reader;
writer ??= environment.writer;

if (delayAssetWrites == true) {
final delayed = writer = DelayedAssetWriter(writer);
reader = delayed.reader(reader, packageGraph.root.name);
}

environment = environment.change(
reader: reader,
writer: writer,
onLog: onLog ?? stdIOLogListener(assumeTty: assumeTty, verbose: verbose),
);

return environment;
}
19 changes: 12 additions & 7 deletions build_runner/lib/src/generate/watch_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import 'package:logging/logging.dart';
import 'package:stream_transform/stream_transform.dart';
import 'package:watcher/watcher.dart';

import '../logging/std_io_logging.dart';
import '../server/server.dart';
import 'environment.dart';
import 'terminator.dart';

final _logger = Logger('Watch');
Expand Down Expand Up @@ -55,6 +55,7 @@ Future<ServeHandler> watch(
bool? isReleaseBuild,
String? logPerformanceDir,
Set<BuildFilter>? buildFilters,
bool? delayAssetWrites,
}) async {
builderConfigOverrides ??= const {};
buildDirs ??= <BuildDirectory>{};
Expand All @@ -68,12 +69,16 @@ Future<ServeHandler> watch(
trackPerformance ??= false;
verbose ??= false;

var environment = OverrideableEnvironment(
IOEnvironment(packageGraph,
assumeTty: assumeTty, outputSymlinksOnly: outputSymlinksOnly),
reader: reader,
writer: writer,
onLog: onLog ?? stdIOLogListener(assumeTty: assumeTty, verbose: verbose));
final environment = createDefaultEnvironment(
packageGraph: packageGraph,
assumeTty: assumeTty,
outputSymlinksOnly: outputSymlinksOnly,
reader: reader,
writer: writer,
delayAssetWrites: delayAssetWrites,
onLog: onLog,
verbose: verbose,
);
var logSubscription =
LogSubscription(environment, verbose: verbose, logLevel: logLevel);
overrideBuildConfig ??= await findBuildConfigOverrides(
Expand Down
3 changes: 3 additions & 0 deletions build_runner/lib/src/watcher/delete_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,7 @@ class OnDeleteWriter implements RunnerAssetWriter {
Future writeAsString(AssetId id, String contents,
{Encoding encoding = utf8}) =>
_writer.writeAsString(id, contents, encoding: encoding);

@override
FutureOr<void> onBuildComplete() => _writer.onBuildComplete();
}
4 changes: 3 additions & 1 deletion build_runner/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ dependencies:
build_config: ">=1.1.0 <1.2.0"
build_daemon: ^3.1.0
build_resolvers: ^2.0.0
build_runner_core: ^7.2.0
build_runner_core: ^8.0.0-dev
code_builder: ^4.2.0
collection: ^1.15.0
crypto: ^3.0.0
Expand Down Expand Up @@ -63,3 +63,5 @@ dependency_overrides:
path: ../build_modules
build_web_compilers:
path: ../build_web_compilers
build_runner_core:
path: ../build_runner_core
5 changes: 4 additions & 1 deletion build_runner_core/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## 7.2.8-dev
## 8.0.0-dev

- __Breaking__: Add `onBuildComplete` to `RunnerAssetWriter`.
- Add `DelayedAssetWriter`, a writer implementation delaying all writes until
`onBuildComplete` is called.
- Raise the minimum SDK constraint to 2.18.

## 7.2.7
Expand Down
Loading