Don't make lazy transformers eager when an asset is requested.

pkg/barback/lib/src/transform_node.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library barback.transform_node;
 
 import 'dart:async';
 
 import 'asset.dart';
 import 'asset_id.dart';
@@ skipping 27 matching lines @@
   /// A string describing the location of [this] in the transformer graph.
   final String _location;
 
   /// The subscription to [primary]'s [AssetNode.onStateChange] stream.
   StreamSubscription _primarySubscription;
 
   /// The subscription to [phase]'s [Phase.onAsset] stream.
   StreamSubscription<AssetNode> _phaseSubscription;
 
   /// Whether [this] is dirty and still has more processing to do.
-  bool get isDirty => _state != _State.NOT_PRIMARY && _state != _State.APPLIED;
+  bool get isDirty => _state != _State.NOT_PRIMARY &&
+      _state != _State.APPLIED && _state != _State.DECLARED;
 
-  /// Whether this transform is lazy and this transform has yet to be forced.
+  /// Whether this transform is deferred.
   ///
-  /// A transform being lazy is distinct from a transformer being lazy. A
-  /// transformer that's declaring but not lazy will have lazy transforms for
-  /// primary inputs that are themselves lazy.
-  bool _isLazy;
+  /// A transform is deferred if either its transformer is lazy or if its
+  /// transformer is declaring and its primary input comes from a deferred
+  /// transformer.
+  final bool deferred;
+
+  /// Whether this is a deferred transform waiting for [force] to be called to
+  /// generate inputs.
+  ///
+  /// This defaults to `true` for deferred transforms and `false` otherwise.
+  /// During or after running `isPrimary` or `declareOutputs`, this may become
+  /// `false`, indicating that the transform has been forced and should generate
+  /// outputs as soon as possible. It will only be set back to `true` if an
+  /// input changes *after* `apply` has completed.
+  bool _awaitingForce;
 
   /// The subscriptions to each input's [AssetNode.onStateChange] stream.
   final _inputSubscriptions = new Map<AssetId, StreamSubscription>();
 
   /// The controllers for the asset nodes emitted by this node.
   final _outputControllers = new Map<AssetId, AssetNodeController>();
 
   /// The ids of inputs the transformer tried and failed to read last time it
   /// ran.
   final _missingInputs = new Set<AssetId>();
@@ skipping 27 matching lines @@
   /// This is synchronous because error logs can cause the transform to fail, so
   /// we need to ensure that their processing isn't delayed until after the
   /// transform or build has finished.
   Stream<LogEntry> get onLog => _onLogPool.stream;
   final _onLogPool = new StreamPool<LogEntry>.broadcast();
 
   /// A controller for log entries emitted by this node.
   final _onLogController = new StreamController<LogEntry>.broadcast(sync: true);
 
   /// The current state of [this].
-  var _state = _State.COMPUTING_IS_PRIMARY;
+  var _state = _State.DECLARING;
 
   /// Whether [this] has been marked as removed.
   bool get _isRemoved => _onAssetController.isClosed;
 
   /// Whether the most recent run of this transform has declared that it
   /// consumes the primary input.
   ///
   /// Defaults to `false`. This is not meaningful unless [_state] is
-  /// [_State.APPLIED].
+  /// [_State.APPLIED] or [_State.DECLARED].
   bool _consumePrimary = false;
 
   /// The set of output ids that [transformer] declared it would emit.
   ///
   /// This is only non-null if [transformer] is a [DeclaringTransformer] and its
   /// [declareOutputs] has been run successfully.
   Set<AssetId> _declaredOutputs;
 
   TransformNode(this.phase, Transformer transformer, AssetNode primary,
       this._location)
       : transformer = transformer,
         primary = primary,
-        _isLazy = transformer is LazyTransformer ||
-            (transformer is DeclaringTransformer && primary.isLazy) {
+        deferred = transformer is LazyTransformer ||
+            (transformer is DeclaringTransformer &&
+             primary.transform != null &&
+             primary.transform.deferred) {

[Bob Nystrom, 2014/04/14 19:20:25]

Instead of checking for a null transform here, what do you think of adding
.deferred to AssetNode that encapsulates that check?

[nweiz, 2014/04/14 21:55:07]

Done.

+    _awaitingForce = deferred;
+
     _onLogPool.add(_onLogController.stream);
 
-    if (!_isLazy) primary.force();
+    if (!deferred) primary.force();
 
     _primarySubscription = primary.onStateChange.listen((state) {
       if (state.isRemoved) {
         remove();
       } else {
+        if (state.isDirty && !deferred) primary.force();
         _dirty();
       }
     });
 
     _phaseSubscription = phase.previous.onAsset.listen((node) {
-      if (_missingInputs.contains(node.id)) _dirty();
+      if (!_missingInputs.contains(node.id)) return;
+      if (!deferred) node.force();
+      _dirty();
     });
 
     _isPrimary();
   }
 
   /// The [TransformInfo] describing this node.
   ///
   /// [TransformInfo] is the publicly-visible representation of a transform
   /// node.
   TransformInfo get info => new TransformInfo(transformer, primary.id);
@@ skipping 11 matching lines @@
     _primarySubscription.cancel();
     _phaseSubscription.cancel();
     _clearInputSubscriptions();
     _clearOutputs();
     if (_passThroughController != null) {
       _passThroughController.setRemoved();
       _passThroughController = null;
     }
   }
 
-  /// If [transformer] is lazy, ensures that its concrete outputs will be
+  /// If [this] is deferred, ensures that its concrete outputs will be
   /// generated.
   void force() {
-    // TODO(nweiz): we might want to have a timeout after which, if the
-    // transform's outputs have gone unused, we switch it back to lazy mode.
-    if (!_isLazy) return;
+    if (!_awaitingForce) return;
     primary.force();
-    _isLazy = false;
+    _awaitingForce = false;
     _dirty();
   }
 
   /// Marks this transform as dirty.
   ///
   /// This causes all of the transform's outputs to be marked as dirty as well.
   void _dirty() {
     if (_state == _State.NOT_PRIMARY) {
       _emitPassThrough();
       return;
     }
-    if (_state == _State.COMPUTING_IS_PRIMARY || _isLazy) return;
+
+    // If we're in the process of running [isPrimary] or [declareOutputs], we're

[Bob Nystrom, 2014/04/14 19:20:25]

"we're" -> "we"

[nweiz, 2014/04/14 21:55:07]

Done.

+    // already know that [apply] needs to be run so there's nothing we need to
+    // mark as dirty.
+    if (_state == _State.DECLARING) return;
+
+    // If we're waiting until [force] is called to run [apply], we don't to run

[Bob Nystrom, 2014/04/14 19:20:25]

"don't to" -> "don't want to".

[nweiz, 2014/04/14 21:55:07]

Done.

+    // [apply] too early.
+    if (_awaitingForce) return;
+
+    if (_state == _State.APPLIED && deferred) {
+      // Transition to DECLARED, indicating that we know what outputs [apply]
+      // will emit but we're waiting to emit them concretely until [force] is
+      // called.
+      _state = _State.DECLARED;
+      _awaitingForce = true;
+      for (var controller in _outputControllers.values) {
+        controller.setLazy(force);
+      }
+      return;
+    }
 
     if (_passThroughController != null) _passThroughController.setDirty();
     for (var controller in _outputControllers.values) {
-      controller.setDirty();
+      // Don't re-mark a controller as dirty to avoid cases where we try to
+      // dispatch an event while handling another event (e.g. an output is
+      // marked lazy, which causes it to be forced, which causes it to be marked
+      // dirty).
+      if (!controller.node.state.isDirty) controller.setDirty();

[Bob Nystrom, 2014/04/14 19:20:25]

Can this check be moved into .setDirty()?

[nweiz, 2014/04/14 21:55:07]

Done.

     }
 
-    if (_state == _State.APPLIED) {
+    if (_state == _State.APPLIED || _state == _State.DECLARED) {
       _apply();
     } else {
       _state = _State.NEEDS_APPLY;
     }
   }
 
   /// Runs [transformer.isPrimary] and adjusts [this]'s state according to the
   /// result.
   ///
   /// This will also run [_declareOutputs] and/or [_apply] as appropriate.
   void _isPrimary() {
     syncFuture(() => transformer.isPrimary(primary.id))
         .catchError((error, stackTrace) {
       if (_isRemoved) return false;
 
       // Catch all transformer errors and pipe them to the results stream. This
       // is so a broken transformer doesn't take down the whole graph.
       phase.cascade.reportError(_wrapException(error, stackTrace));
 
       return false;
     }).then((isPrimary) {
       if (_isRemoved) return null;
       if (isPrimary) {
         return _declareOutputs().then((_) {
           if (_isRemoved) return;
-          if (_isLazy) {
-            _state = _State.APPLIED;
+          if (_awaitingForce) {
+            _state = _State.DECLARED;
             _onDoneController.add(null);
           } else {
             _apply();
           }
         });
       }
 
       _emitPassThrough();
       _state = _State.NOT_PRIMARY;
       _onDoneController.add(null);
@@ skipping 19 matching lines @@
           .where((id) => id.package != phase.cascade.package).toSet();
       for (var id in invalidIds) {
         _declaredOutputs.remove(id);
         // TODO(nweiz): report this as a warning rather than a failing error.
         phase.cascade.reportError(new InvalidOutputException(info, id));
       }
 
       if (!_declaredOutputs.contains(primary.id)) _emitPassThrough();
 
       for (var id in _declaredOutputs) {
-        var controller = _isLazy
+        var controller = _awaitingForce
             ? new AssetNodeController.lazy(id, force, this)
             : new AssetNodeController(id, this);
         _outputControllers[id] = controller;
         _onAssetController.add(controller.node);
       }
     }).catchError((error, stackTrace) {
       if (_isRemoved) return;
       phase.cascade.reportError(_wrapException(error, stackTrace));
     });
   }
 
   /// Applies this transform.
   void _apply() {
-    assert(!_isRemoved && !_isLazy);
+    assert(!_isRemoved && !_awaitingForce);
 
     // Clear input subscriptions here as well as in [_process] because [_apply]
     // may be restarted independently if only a secondary input changes.
     _clearInputSubscriptions();
     _state = _State.APPLYING;
     _runApply().then((hadError) {
       if (_isRemoved) return;
 
       if (_state == _State.NEEDS_APPLY) {
         _apply();
@@ skipping 27 matching lines @@
     return phase.previous.getOutput(id).then((node) {
       // Throw if the input isn't found. This ensures the transformer's apply
       // is exited. We'll then catch this and report it through the proper
       // results stream.
       if (node == null) {
         _missingInputs.add(id);
         throw new AssetNotFoundException(id);
       }
 
       _inputSubscriptions.putIfAbsent(node.id, () {
-        return node.onStateChange.listen((_) => _dirty());
+        return node.onStateChange.listen((state) => _dirty());
       });
 
       return node.asset;
     });
   }
 
   /// Run [Transformer.apply] as soon as [primary] is available.
   ///
   /// Returns whether or not an error occurred while running the transformer.
   Future<bool> _runApply() {
@@ skipping 130 matching lines @@
     _onLogController.add(
         new LogEntry(info, primary.id, LogLevel.WARNING, message, null));
   }
 
   String toString() =>
     "transform node in $_location for $transformer on $primary";
 }
 
 /// The enum of states that [TransformNode] can be in.
 class _State {
-  /// The transform is running [Transformer.isPrimary].
+  /// The transform is running [Transformer.isPrimary] followed by
+  /// [DeclaringTransformer.declareOutputs] (for a [DeclaringTransformer]).
   ///
-  /// This is the initial state of the transformer. Once [Transformer.isPrimary]
-  /// finishes running, this will transition to [APPLYING] if the input is
-  /// primary, or [NOT_PRIMARY] if it's not.
-  static final COMPUTING_IS_PRIMARY = const _State._("computing isPrimary");
+  /// This is the initial state of the transformer, and it will only occur once
+  /// since [Transformer.isPrimary] and [DeclaringTransformer.declareOutputs]
+  /// are independent of the contents of the primary input. Once the two methods
+  /// finish running, this will transition to [NOT_PRIMARY] if the input isn't
+  /// primary, [DECLARED] if the transform is deferred, and [APPLYING] otherwise.

[Bob Nystrom, 2014/04/14 19:20:25]

Long line.

[nweiz, 2014/04/14 21:55:07]

Done.

+  static final DECLARING = const _State._("computing isPrimary");
+
+  /// The transform is deferred and has run [DeclaringTransformer.declareOutputs]
+  /// but hasn't yet been forced.
+  ///
+  /// This will transition to [APPLYING] when one of the outputs has been
+  /// forced.
+  static final DECLARED = const _State._("declared");
 
   /// The transform is running [Transformer.apply].
   ///
   /// If an input changes while in this state, it will transition to
   /// [NEEDS_APPLY]. If the [TransformNode] is still in this state when
   /// [Transformer.apply] finishes running, it will transition to [APPLIED].
   static final APPLYING = const _State._("applying");
 
   /// The transform is running [Transformer.apply], but an input changed after
   /// it started, so it will need to re-run [Transformer.apply].
   ///
   /// This will transition to [APPLYING] once [Transformer.apply] finishes
   /// running.
   static final NEEDS_APPLY = const _State._("needs apply");
 
   /// The transform has finished running [Transformer.apply], whether or not it
   /// emitted an error.
   ///
-  /// If the transformer is lazy, the [TransformNode] can also be in this state
-  /// when [Transformer.declareOutputs] has been run but [Transformer.apply] has
-  /// not.
+  /// If the transformer is deferred, the [TransformNode] can also be in this
+  /// state when [Transformer.declareOutputs] has been run but
+  /// [Transformer.apply] has not.
   ///
-  /// If an input changes, this will transition to [APPLYING].
+  /// If an input changes, this will transition to [DECLARED] if the transform
+  /// is deferred and [APPLYING] otherwise.
   static final APPLIED = const _State._("applied");
 
   /// The transform has finished running [Transformer.isPrimary], which returned
   /// `false`.
   ///
   /// This will never transition to another state.
   static final NOT_PRIMARY = const _State._("not primary");
 
   final String name;
 
   const _State._(this.name);
 
   String toString() => name;
 }