Make Phase.getInput in barback play nicely with the push model.

Make Phase.getInput in barback play nicely with the push model.

The previous implementation made assumptions about each phase being
finished before the next one started. These assumptions were
invalidated by r33391.

R=rnystrom@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=33408

[Bob Nystrom, 2014-03-06 23:59:22]

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/asset_ca...
File pkg/barback/lib/src/asset_cascade.dart (right):

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/asset_ca...
pkg/barback/lib/src/asset_cascade.dart:107: if (_phases.last == oldLastPhase)
return node;
Document why this may change.

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/asset_ca...
pkg/barback/lib/src/asset_cascade.dart:109: }).then((result) => result);
Delete the .then()?

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/phase.dart
File pkg/barback/lib/src/phase.dart (right):

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/phase.da...
pkg/barback/lib/src/phase.dart:142: if (_isTransitivelyDirty) return;
If we're listening on every phase, why do we need to check for transitive
dirtiness?

This is, what, n^2? Can you do this a more efficient way?

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/phase.da...
pkg/barback/lib/src/phase.dart:238: // If the requested output exists but isn't
yet available, wait to see if
Long line.

[nweiz, 2014-03-07 00:20:18]

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/asset_ca...
File pkg/barback/lib/src/asset_cascade.dart (right):

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/asset_ca...
pkg/barback/lib/src/asset_cascade.dart:107: if (_phases.last == oldLastPhase)
return node;
On 2014/03/06 23:59:22, Bob Nystrom wrote:
> Document why this may change.

Done.

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/asset_ca...
pkg/barback/lib/src/asset_cascade.dart:109: }).then((result) => result);
On 2014/03/06 23:59:22, Bob Nystrom wrote:
> Delete the .then()?

Done.

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/phase.dart
File pkg/barback/lib/src/phase.dart (right):

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/phase.da...
pkg/barback/lib/src/phase.dart:142: if (_isTransitivelyDirty) return;
On 2014/03/06 23:59:22, Bob Nystrom wrote:
> If we're listening on every phase, why do we need to check for transitive
> dirtiness?

This listener only fires when one phase finishes its work, but each phase's
dirty state is independent of the others'. We can't guarantee that an output
won't be generated until *no* parent phases are dirty, which is what this
checks.

> This is, what, n^2? Can you do this a more efficient way?

Which part? [_isTransitivelyDirty] is O(n) in the number of phases, since it
just walks upward through the phase tree. Creating a bunch of phases is O(n^2),
since each one has to create a listener for all the phases above, but that only
happens once when the cascade is created. We could theoretically make that O(n)
as well by adding an [onTransitivelyDone] stream, but that feels like too much
extra API surface for too little gain.

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/phase.da...
pkg/barback/lib/src/phase.dart:238: // If the requested output exists but isn't
yet available, wait to see if
On 2014/03/06 23:59:22, Bob Nystrom wrote:
> Long line.

Done.

[Bob Nystrom, 2014-03-07 00:54:15]

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/phase.dart
File pkg/barback/lib/src/phase.dart (right):

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/phase.da...
pkg/barback/lib/src/phase.dart:142: if (_isTransitivelyDirty) return;
On 2014/03/07 00:20:19, nweiz wrote:
> On 2014/03/06 23:59:22, Bob Nystrom wrote:
> > If we're listening on every phase, why do we need to check for transitive
> > dirtiness?
> 
> This listener only fires when one phase finishes its work, but each phase's
> dirty state is independent of the others'. We can't guarantee that an output
> won't be generated until *no* parent phases are dirty, which is what this
> checks.
> 
> > This is, what, n^2? Can you do this a more efficient way?
> 
> Which part? [_isTransitivelyDirty] is O(n) in the number of phases, since it
> just walks upward through the phase tree. Creating a bunch of phases is
O(n^2),
> since each one has to create a listener for all the phases above, but that
only
> happens once when the cascade is created.

Say there are phases A, B, C, and D. A's onDone fires:

B receives it and checks to see if A is done.
C receives it and checks to see if A and B are done.
D receives it and checks to see if A, B, and C are done.

> We could theoretically make that O(n)
> as well by adding an [onTransitivelyDone] stream, but that feels like too much
> extra API surface for too little gain.

That feels a bit gross too, but something that only requires looking at the
previous phase seems like a better solution here. Given how hard you try to keep
things encapsulated, it seems weird that here you're looping over all of the
previous phases.

[nweiz, 2014-03-07 01:08:27]

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/phase.dart
File pkg/barback/lib/src/phase.dart (right):

https://codereview.chromium.org/189263002/diff/1/pkg/barback/lib/src/phase.da...
pkg/barback/lib/src/phase.dart:142: if (_isTransitivelyDirty) return;
On 2014/03/07 00:54:15, Bob Nystrom wrote:
> On 2014/03/07 00:20:19, nweiz wrote:
> > On 2014/03/06 23:59:22, Bob Nystrom wrote:
> > > If we're listening on every phase, why do we need to check for transitive
> > > dirtiness?
> > 
> > This listener only fires when one phase finishes its work, but each phase's
> > dirty state is independent of the others'. We can't guarantee that an output
> > won't be generated until *no* parent phases are dirty, which is what this
> > checks.
> > 
> > > This is, what, n^2? Can you do this a more efficient way?
> > 
> > Which part? [_isTransitivelyDirty] is O(n) in the number of phases, since it
> > just walks upward through the phase tree. Creating a bunch of phases is
> O(n^2),
> > since each one has to create a listener for all the phases above, but that
> only
> > happens once when the cascade is created.
> 
> Say there are phases A, B, C, and D. A's onDone fires:
> 
> B receives it and checks to see if A is done.
> C receives it and checks to see if A and B are done.
> D receives it and checks to see if A, B, and C are done.

Ah, I see. I'll add a TODO to find a better way to do this.

[Bob Nystrom, 2014-03-07 01:36:52]

LGTM.

[nweiz, 2014-03-07 01:41:44]

Committed patchset #3 manually as r33408 (presubmit successful).