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

incremental rebuilds with watch #46

Merged
merged 2 commits into from
Feb 16, 2016
Merged
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
3 changes: 3 additions & 0 deletions lib/src/asset_graph/graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ class AssetGraph {
/// Removes [node] from the graph.
AssetNode remove(AssetId id) => _nodesById.remove(id);

/// Gets all nodes in the graph.
Iterable<AssetNode> get allNodes => _nodesById.values;

@override
toString() => _nodesById.values.toList().toString();
}
5 changes: 4 additions & 1 deletion lib/src/asset_graph/node.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,11 @@ class GeneratedAssetNode extends AssetNode {
/// Whether or not this asset needs to be updated.
bool needsUpdate;

/// Whether the asset was actually output.
bool wasOutput;

GeneratedAssetNode(this.builder, this.primaryInput, this.generatingPhaseGroup,
this.needsUpdate, AssetId id)
this.needsUpdate, this.wasOutput, AssetId id)
: super(id);

@override
Expand Down
67 changes: 49 additions & 18 deletions lib/src/generate/build_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import 'package:logging/logging.dart';
import 'package:path/path.dart' as path;

import '../asset/asset.dart';
import '../asset/cache.dart';
import '../asset/exceptions.dart';
import '../asset/id.dart';
import '../asset/reader.dart';
Expand All @@ -35,8 +34,8 @@ class BuildImpl {
bool _buildRunning = false;
final _logger = new Logger('Build');

BuildImpl(this._assetGraph, this._reader, this._writer,
this._packageGraph, this._phaseGroups);
BuildImpl(this._assetGraph, this._reader, this._writer, this._packageGraph,
this._phaseGroups);

/// Runs a build
///
Expand All @@ -62,11 +61,13 @@ class BuildImpl {
_logger.info('Running build phases');
var result = await _runPhases();

// Write out the new build_outputs file.
/// Write out the new build_outputs file.
var allOuputs = _assetGraph.allNodes
.where((node) => node is GeneratedAssetNode && node.wasOutput);
var buildOutputsAsset = new Asset(
_buildOutputsId,
JSON.encode(
result.outputs.map((output) => output.id.serialize()).toList()));
allOuputs.map((output) => output.id.serialize()).toList()));
await _writer.writeAsString(buildOutputsAsset);

return result;
Expand All @@ -85,15 +86,21 @@ class BuildImpl {
/// Deletes all previous output files.
Future _deletePreviousOutputs() async {
if (await _reader.hasInput(_buildOutputsId)) {
// Cache file exists, just delete all outputs contained in it.
/// Cache file exists, delete all outputs which don't appear in the
/// [_assetGraph], or are marked as needing an update.
///
/// Removes all files from [_inputsByPackage] regardless of state.
var previousOutputs =
JSON.decode(await _reader.readAsString(_buildOutputsId));
await _writer.delete(_buildOutputsId);
_inputsByPackage[_buildOutputsId.package]?.remove(_buildOutputsId);
await Future.wait(previousOutputs.map((output) {
await Future.wait(previousOutputs.map((output) async {
var outputId = new AssetId.deserialize(output);
_inputsByPackage[outputId.package]?.remove(outputId);
return _writer.delete(outputId);
var node = _assetGraph.get(outputId);
if (node == null || (node as GeneratedAssetNode).needsUpdate) {

Choose a reason for hiding this comment

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

Will this always be a safe cast to GeneratedAssetNode? If so, why not type _assetGraph?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should always be a GeneratedAssetNode, otherwise it would be an error (all outputs are GeneratedAssetNodes).

await _writer.delete(outputId);
}
}));
return;
}
Expand Down Expand Up @@ -176,26 +183,29 @@ class BuildImpl {
final outputs = <Asset>[];
int phaseGroupNum = 0;
for (var group in _phaseGroups) {
final groupOutputs = <Asset>[];
/// Collects all the ids for files which are output by this stage. This
/// also includes files which didn't get regenerated because they weren't,
/// dirty unlike [outputs] which only gets files which were explicitly
/// generated in this build.
final groupOutputIds = new Set<AssetId>();
for (var phase in group) {
var inputs = _matchingInputs(phase.inputSets);
for (var builder in phase.builders) {
// TODO(jakemac): Optimize, we can run all the builders in a phase
// at the same time instead of sequentially.
await for (var output
in _runBuilder(builder, inputs, phaseGroupNum)) {
groupOutputs.add(output);
in _runBuilder(builder, inputs, phaseGroupNum, groupOutputIds)) {
outputs.add(output);
}
}
}

/// Once the group is done, add all outputs so they can be used in the next
/// phase.
for (var output in groupOutputs) {
for (var outputId in groupOutputIds) {
_inputsByPackage.putIfAbsent(
output.id.package, () => new Set<AssetId>());
_inputsByPackage[output.id.package].add(output.id);
outputId.package, () => new Set<AssetId>());
_inputsByPackage[outputId.package].add(outputId);
}
phaseGroupNum++;
}
Expand Down Expand Up @@ -250,7 +260,7 @@ class BuildImpl {

/// Runs [builder] with [inputs] as inputs.
Stream<Asset> _runBuilder(Builder builder, Iterable<AssetId> primaryInputs,
int phaseGroupNum) async* {
int phaseGroupNum, Set<AssetId> groupOutputs) async* {
for (var input in primaryInputs) {
var expectedOutputs = builder.declareOutputs(input);

Expand All @@ -272,31 +282,52 @@ class BuildImpl {
_assetGraph.addIfAbsent(
output,
() => new GeneratedAssetNode(
builder, input, phaseGroupNum, true, output));
builder, input, phaseGroupNum, true, false, output));
}

/// Skip the build step if none of the outputs need updating.
var skipBuild = !expectedOutputs.any((output) =>
(_assetGraph.get(output) as GeneratedAssetNode).needsUpdate);
if (skipBuild) continue;
if (skipBuild) {
/// If we skip the build, we still need to add the ids as outputs for
/// any files which were output last time, so they can be used by
/// subsequent phases.
for (var output in expectedOutputs) {
if ((_assetGraph.get(output) as GeneratedAssetNode).wasOutput) {
groupOutputs.add(output);
}
}
continue;
}

var inputAsset = new Asset(input, await _reader.readAsString(input));
var buildStep = new BuildStepImpl(inputAsset, expectedOutputs, _reader,
_writer, _packageGraph.root.name);
await builder.build(buildStep);
await buildStep.complete();

/// Mark all outputs as no longer needing an update, and mark `wasOutput`
/// as `false` for now (this will get reset to true later one).
for (var output in expectedOutputs) {
(_assetGraph.get(output) as GeneratedAssetNode)
..needsUpdate = false
..wasOutput = false;
}

/// Update the asset graph based on the dependencies discovered.
for (var dependency in buildStep.dependencies) {
var dependencyNode = _assetGraph.addIfAbsent(
dependency, () => new AssetNode(dependency));

/// We care about all [expectedOutputs], not just real outputs.
/// We care about all [expectedOutputs], not just real outputs. Updates
/// to dependencies may cause a file to be output which wasn't before.
dependencyNode.outputs.addAll(expectedOutputs);
}

/// Yield the outputs.
for (var output in buildStep.outputs) {
(_assetGraph.get(output.id) as GeneratedAssetNode).wasOutput = true;
groupOutputs.add(output.id);
yield output;
}
}
Expand Down
23 changes: 16 additions & 7 deletions lib/src/generate/watch_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class WatchImpl {
_runningWatch = true;
_resultStreamController = new StreamController<BuildResult>();
_nextBuildScheduled = false;
var updatedInputs = new Set<AssetId>();
var updatedInputs = new Map<AssetId, ChangeType>();

doBuild([bool force = false]) {
// Don't schedule more builds if we are turning down.
Expand All @@ -128,23 +128,32 @@ class WatchImpl {

/// Remove any updates that were generated outputs or otherwise not
/// interesting.
updatedInputs.removeWhere(_shouldSkipInput);
var updatesToRemove = updatedInputs.keys.where(_shouldSkipInput).toList();
updatesToRemove.forEach(updatedInputs.remove);
if (updatedInputs.isEmpty && !force) {
return;
}

_logger.info('Preparing for next build');
_logger.info('Clearing cache for invalidated assets');
void clearNodeAndDeps(AssetId id) {
void clearNodeAndDeps(AssetId id, ChangeType rootChangeType) {
var node = _assetGraph.get(id);
if (node == null) return;
if (node is GeneratedAssetNode) {
node.needsUpdate = true;
}
_assetCache.remove(id);
for (var output in node.outputs) {
clearNodeAndDeps(output);
clearNodeAndDeps(output, rootChangeType);
}

/// For deletes, prune the graph.
if (rootChangeType == ChangeType.REMOVE) {
_assetGraph.remove(id);
}
}
for (var input in updatedInputs) {
clearNodeAndDeps(input);
for (var input in updatedInputs.keys) {
clearNodeAndDeps(input, updatedInputs[input]);
}
updatedInputs.clear();

Expand Down Expand Up @@ -180,7 +189,7 @@ class WatchImpl {
_allListeners.add(watcher.events.listen((WatchEvent e) {
_logger.fine('Got WatchEvent for path ${e.path}');
var id = new AssetId(package.name, path.normalize(e.path));
updatedInputs.add(id);
updatedInputs[id] = e.type;
scheduleBuild();
}));
}
Expand Down
8 changes: 5 additions & 3 deletions test/asset_graph/graph_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,19 @@ main() {
return node;
}

test('add, contains, get', () {
test('add, contains, get, allNodes', () {
var expectedNodes = [];
for (int i = 0; i < 5; i++) {
testAddNode();
expectedNodes.add(testAddNode());
}
expect(graph.allNodes, unorderedEquals(expectedNodes));
});

test('addIfAbsent', () {
var node = makeAssetNode();
expect(graph.addIfAbsent(node.id, () => node), same(node));
expect(graph.contains(node.id), isTrue);

var otherNode = new AssetNode(node.id);
expect(graph.addIfAbsent(otherNode.id, () => otherNode), same(node));
expect(graph.contains(otherNode.id), isTrue);
Expand Down
21 changes: 19 additions & 2 deletions test/common/copy_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,33 @@ import 'dart:async';
import 'package:build/build.dart';

class CopyBuilder implements Builder {
/// If > 0, then multiple copies will be output, using the copy number as an
/// additional extension.
final int numCopies;

/// The file extension to add to files.
final String extension;

/// The package in which to output files.
final String outputPackage;

CopyBuilder({this.numCopies: 1, this.extension: 'copy', this.outputPackage});
/// Copies content from this asset into all files, instead of the primary
/// asset.
final AssetId copyFromAsset;

CopyBuilder(
{this.numCopies: 1,
this.extension: 'copy',
this.outputPackage,
this.copyFromAsset});

Future build(BuildStep buildStep) async {
var ids = declareOutputs(buildStep.input.id);
for (var id in ids) {
buildStep.writeAsString(new Asset(id, buildStep.input.stringContents));
var content = copyFromAsset == null
? buildStep.input.stringContents
: await buildStep.readAsString(copyFromAsset);
buildStep.writeAsString(new Asset(id, content));
}
}

Expand Down
Loading