Use READY_RESOLVED_UNIT flag for ordering tasks.

Use READY_RESOLVED_UNIT flag for ordering tasks.

Fixes #25271.

BUG=
R=brianwilkerson@google.com

Committed: https://github.com/dart-lang/sdk/commit/a34bab7fde56c015c57ee2fd2152daef4b5ca962

[Leaf, 2015-12-18 18:18:14]

leafp@google.com changed reviewers:
+ brianwilkerson@google.com

[Brian Wilkerson, 2015-12-18 18:52:19]

https://codereview.chromium.org/1534033004/diff/1/pkg/analyzer/lib/src/task/d...
File pkg/analyzer/lib/src/task/dart.dart (right):

https://codereview.chromium.org/1534033004/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:2950:
READY_RESOLVED_UNIT.of(unit.librarySource))
I'm a little concerned by the specifics of this change. I like the general idea
a lot, but I'm concerned because READY_RESOLVED_UNIT implies a lot more analysis
(such as constant evaluation) than RESOLVED_UNIT9 did. Even assuming that we can
safely perform that extra analysis at this point without creating a cycle in the
dependency graph (which I haven't convinced myself of), I'd like to (a) do less
work rather than more and (b) remember the real dependencies so that it's clear
in the future what can and cannot change in terms of the task structure.

Perhaps we could define a result (named something like CREATED_RESOLVED_UNIT9)
that gets created by the same task that creates RESOLVED_UNIT9, and then we
could depend on that flag here. The flag would have the semantics that we know
that the work of creating a RESOLVED_UNIT9 had been done, whether the result was
still in the cache or not.

[Leaf, 2015-12-18 20:03:52]

I had a bit of the same debate in my head.  I think it's fine from a dependency
standpoint - these are all things that strictly precede the given library cycle
in the import/export graph, so unless I'm missing something even the constant
evaluation code couldn't induce a cycle here.  But  it does make the strong mode
code more eager.  I'm not 100% sure this is a bad thing from a throughput
standpoint, but might have latency effects.  I'll put together a version that
keeps the original dependency structure.

On 2015/12/18 18:52:19, Brian Wilkerson wrote:
>
https://codereview.chromium.org/1534033004/diff/1/pkg/analyzer/lib/src/task/d...
> File pkg/analyzer/lib/src/task/dart.dart (right):
> 
>
https://codereview.chromium.org/1534033004/diff/1/pkg/analyzer/lib/src/task/d...
> pkg/analyzer/lib/src/task/dart.dart:2950:
> READY_RESOLVED_UNIT.of(unit.librarySource))
> I'm a little concerned by the specifics of this change. I like the general
idea
> a lot, but I'm concerned because READY_RESOLVED_UNIT implies a lot more
analysis
> (such as constant evaluation) than RESOLVED_UNIT9 did. Even assuming that we
can
> safely perform that extra analysis at this point without creating a cycle in
the
> dependency graph (which I haven't convinced myself of), I'd like to (a) do
less
> work rather than more and (b) remember the real dependencies so that it's
clear
> in the future what can and cannot change in terms of the task structure.
> 
> Perhaps we could define a result (named something like CREATED_RESOLVED_UNIT9)
> that gets created by the same task that creates RESOLVED_UNIT9, and then we
> could depend on that flag here. The flag would have the semantics that we know
> that the work of creating a RESOLVED_UNIT9 had been done, whether the result
was
> still in the cache or not.

[Leaf, 2015-12-18 21:48:31]

PTAL.  I added CREATED_RESOLVED_UNITX flag targets for all of the RESOLVED_UNIT
targets, and replaced uses of RESOLVED_UNITX with CREATED_RESOLVED_UNITX where
appropriate.

[Brian Wilkerson, 2015-12-18 21:57:20]

LGTM! Do we know what the performance impact is?

[Leaf, 2015-12-18 22:06:02]

On 2015/12/18 21:57:20, Brian Wilkerson wrote:
> LGTM! Do we know what the performance impact is?

On my laptop, with the large app that was driving this:

Prior to Konstantin's CL for the non-strong mode tasks: 

Regular analyzer: 56 seconds
Strong mode:  indefinite

Post Konstantin's CL:

Regular analyzer: 26 seconds
Strong mode: indefinite

Post this CL:

Regular analyzer: 24 seconds
Strong mode: 43 seconds

[Leaf, 2015-12-18 22:17:57]

Description was changed from

==========
Use READY_RESOLVED_UNIT flag for ordering tasks.

Fixes #25271.

BUG=
==========

to

==========
Use READY_RESOLVED_UNIT flag for ordering tasks.

Fixes #25271.

BUG=
R=brianwilkerson@google.com

Committed:
https://github.com/dart-lang/sdk/commit/a34bab7fde56c015c57ee2fd2152daef4b5ca962
==========

[Leaf, 2015-12-18 22:17:58]

Committed patchset #3 (id:40001) manually as
a34bab7fde56c015c57ee2fd2152daef4b5ca962 (presubmit successful).