Extract ClassHierarchyImpl implementation from ClassHierarchy.

Extract ClassHierarchyImpl implementation from ClassHierarchy.

As a preparation step for using different implementation during
incremental kernel generation.

R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=

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

[Siggi Cherem (dart-lang), 2017-05-26 15:56:55]

Changes look good to me. I'll let Kevin give the final green light.

Kevin, some additional context: we noticed that during incremental kernel
generation a big chunk of the time is spent in the class-hierarchy analysis. I
can't recall if it was half of the time or more, but Konstantin should be able
to give you more details. The reason it is so high is because we are running it
many times, once on each intermediate build step (once per SCC of libraries in
the program graph).

Konstantin wants to experiment with a lazy implementation, so we don't spend
time on classes from transitive dependencies that are not used by a given
library.

[scheglov, 2017-05-26 16:08:02]

On 2017/05/26 15:56:55, Siggi Cherem (dart-lang) wrote:
> Changes look good to me. I'll let Kevin give the final green light.
> 
> Kevin, some additional context: we noticed that during incremental kernel
> generation a big chunk of the time is spent in the class-hierarchy analysis. I
> can't recall if it was half of the time or more, but Konstantin should be able
> to give you more details. The reason it is so high is because we are running
it
> many times, once on each intermediate build step (once per SCC of libraries in
> the program graph).
> 
> Konstantin wants to experiment with a lazy implementation, so we don't spend
> time on classes from transitive dependencies that are not used by a given
> library.

Thank you for adding context, Siggi.

Currently ClassHierarchy is 53% of total compilation time.

I hope to make it less by (1) making it lazy and compute only required
superclass and dispatch target information; (2) make it incremental and keep the
same instance as we go from one SCC of libraries to another.

One obvious design difference would be that we cannot think that we are in the
closed world - there are always following libraries which may extend existing
classes. So, any requests that return subclasses or all dispatch targets are
meaningless. Fortunately, Fasta does not need this, only treeshaker does.

[ahe, 2017-05-29 13:56:18]

L*G*T*M, but wait for Kevin :-)

https://codereview.chromium.org/2904203002/diff/1/pkg/kernel/lib/class_hierar...
File pkg/kernel/lib/class_hierarchy.dart (right):

https://codereview.chromium.org/2904203002/diff/1/pkg/kernel/lib/class_hierar...
pkg/kernel/lib/class_hierarchy.dart:127: class ClassHierarchyImpl implements
ClassHierarchy {
I suggest that this class is renamed to, for example, ClosedWorldClassHierarchy.

The reason for that is that this class ends up be used in several places, and
all these places it seems like the important reason for using this class is that
it is based on a closed-world assumption.

[scheglov, 2017-05-30 01:18:18]

https://codereview.chromium.org/2904203002/diff/1/pkg/kernel/lib/class_hierar...
File pkg/kernel/lib/class_hierarchy.dart (right):

https://codereview.chromium.org/2904203002/diff/1/pkg/kernel/lib/class_hierar...
pkg/kernel/lib/class_hierarchy.dart:127: class ClassHierarchyImpl implements
ClassHierarchy {
On 2017/05/29 13:56:18, ahe wrote:
> I suggest that this class is renamed to, for example,
ClosedWorldClassHierarchy.
> 
> The reason for that is that this class ends up be used in several places, and
> all these places it seems like the important reason for using this class is
that
> it is based on a closed-world assumption.

Good idea.
Done.

[ahe, 2017-05-30 09:46:27]

Kevin didn't get around to take a look yesterday, and is taking a vacation day
today.

So I'll say LGTM, as I know you'll address any issues he may discover.

[ahe, 2017-05-30 09:46:30]

Kevin didn't get around to take a look yesterday, and is taking a vacation day
today.

So I'll say LGTM, as I know you'll address any issues he may discover.

[Kevin Millikin (Google), 2017-05-30 15:19:57]

Yes, go ahead and commit this when you are ready.  I'll take a look later (I'm
on my phone right now) and let you know if I have any major concerns.

[scheglov, 2017-05-30 16:45:14]

Description was changed from

==========
Extract ClassHierarchyImpl implementation from ClassHierarchy.

As a preparation step for using different implementation during
incremental kernel generation.

R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=
==========

to

==========
Extract ClassHierarchyImpl implementation from ClassHierarchy.

As a preparation step for using different implementation during
incremental kernel generation.

R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=

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

[scheglov, 2017-05-30 16:45:15]

Committed patchset #2 (id:20001) manually as
f9af7bf5bc545fa4d7dcd750c68a178310e0d566 (presubmit successful).