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 lazy.
   ///
   /// 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.

[Bob Nystrom, 2014/04/10 22:58:56]

I think we should disambiguate these terms. How about "lazy" means "a
transformer that doesn't run until needed" and "deferred" is a transformation
that is either lazy or declared and downstream from something lazy?

[nweiz, 2014/04/11 00:47:59]

Done.

-  bool _isLazy;
+  final bool _isLazy;
+
+  /// Whether this is a lazy transform waiting for [force] to be called to
+  /// generate inputs.
+  ///
+  /// This defaults to `true` for lazy 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;

[Bob Nystrom, 2014/04/10 22:58:56]

This name is a bit confusing. How about "_deferred"?

[nweiz, 2014/04/11 00:47:59]

That doesn't match your previous suggestion. You suggested that "deferred" means
"lazy or downstream from something lazy", but this variable isn't that; it
indicates whether the transformer has emitted lazy outputs that have yet to be
materialized. 

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

Ah, good point.

 
   /// 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) {
+    _awaitingForce = _isLazy;
+
     _onLogPool.add(_onLogController.stream);
 
     if (!_isLazy) primary.force();
 
     _primarySubscription = primary.onStateChange.listen((state) {
       if (state.isRemoved) {
         remove();
       } else {
+        if (state.isDirty && !_isLazy) primary.force();
         _dirty();
       }
     });
 
     _phaseSubscription = phase.previous.onAsset.listen((node) {
-      if (_missingInputs.contains(node.id)) _dirty();
+      if (!_missingInputs.contains(node.id)) return;
+      if (!_isLazy) 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 14 matching lines @@
     _clearOutputs();
     if (_passThroughController != null) {
       _passThroughController.setRemoved();
       _passThroughController = null;
     }
   }
 
   /// If [transformer] is lazy, 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 (_state == _State.DECLARING || _awaitingForce) return;

[Bob Nystrom, 2014/04/10 22:58:56]

Add some documentation here about what the DECLARING check is for.

[nweiz, 2014/04/11 00:47:59]

Done.

+
+    if (_state == _State.APPLIED && _isLazy) {
+      for (var controller in _outputControllers.values) {
+        controller.setLazy(force);
+      }
+      _state = _State.DECLARED;

[Bob Nystrom, 2014/04/10 22:58:56]

This could use some docs here. It isn't obvious why it transitions from APPLIED
to DECLARED. (I assume the answer is because the declaration is only based on
the primary ID now, so won't change and doesn't need to be re-run.)

[nweiz, 2014/04/11 00:47:59]

Done.

+      _awaitingForce = true;
+      return;
+    }
 
     if (_passThroughController != null) _passThroughController.setDirty();
     for (var controller in _outputControllers.values) {
       controller.setDirty();
     }
 
-    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) {
+          if (state.isDirty) node.force();

[Bob Nystrom, 2014/04/10 22:58:56]

So if an input to this transform changes to a dirty state, this transform forces
it to apply?

Document what's going on here.

[nweiz, 2014/04/11 00:47:59]

Removed, as per discussion.

+          _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 lazy, and [APPLYING] otherwise.
+  static final DECLARING = const _State._("computing isPrimary");
+
+  /// The transform is lazy 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");

[Bob Nystrom, 2014/04/10 22:58:56]

"DEFERRED"?

[nweiz, 2014/04/11 00:47:59]

That would be confusing in light of your terminology suggestion above, since the
state of being lazy or transitively beneath a lazy transformer is static whereas
a transform can move into and out of this state.

 
   /// 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 an input changes, this will transition to [APPLYING].
+  /// If an input changes, this will transition to [DECLARED] if the transform
+  /// is lazy 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;
 }