Incremental compilation: set up inheritance.

Incremental compilation: set up inheritance.

R=johnniwinther@google.com

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

[ahe, 2014-11-11 15:40:50]

ahe@google.com changed reviewers:
+ herhut@google.com, johnniwinther@google.com

[ahe, 2014-11-11 15:40:51]

https://codereview.chromium.org/719443002/diff/1/dart/pkg/compiler/lib/src/ss...
File dart/pkg/compiler/lib/src/ssa/codegen.dart (right):

https://codereview.chromium.org/719443002/diff/1/dart/pkg/compiler/lib/src/ss...
dart/pkg/compiler/lib/src/ssa/codegen.dart:1572: if
(compiler.hasIncrementalSupport) {
Stephan: please take a look at this.

This is to remove customizations based on receiver, for example:

class A {
  get name => 'A.m';

  m() {
    print('Called $name');
  }
}

class B extends A {
  get name => 'B.m';
}

var instance;
main() {
  if (instance == null) {
    instance = new A();
  // Comment in this code in second version:
  // } else {
  // instance = new B();
  }
  instance.m();
}

Here, instance was assumed to be an instance of A, so B.name wasn't enqueued.

[ahe, 2014-11-12 10:49:51]

Uploaded new patch set to completely disable creation of TypedSelector.

[herhut, 2014-11-12 14:37:21]

On 2014/11/12 10:49:51, ahe wrote:
> Uploaded new patch set to completely disable creation of TypedSelector.

How about adding a flag [isIncremental] (in the lack of better name) to
[ClassWorld] and then use that to steer what the TypedSelector factory does?
That would be much more local than this approach.

Also, in the longer term, it might be a good approach to use that flag in all
operations on [TypeMask] to take the incremental nature of the world into
account (e.g. only report a single target if the class in question is final).

[ahe, 2014-11-12 15:46:52]

Patchset #3 (id:40001) has been deleted

[ahe, 2014-11-12 15:50:24]

On 2014/11/12 14:37:21, herhut wrote:
> On 2014/11/12 10:49:51, ahe wrote:
> > Uploaded new patch set to completely disable creation of TypedSelector.
> 
> How about adding a flag [isIncremental] (in the lack of better name) to
> [ClassWorld] and then use that to steer what the TypedSelector factory does?
> That would be much more local than this approach.
> 
> Also, in the longer term, it might be a good approach to use that flag in all
> operations on [TypeMask] to take the incremental nature of the world into
> account (e.g. only report a single target if the class in question is final).

Works really well.

This is the example from the test:

main() {
  if (instance == null) {
    instance = new A();
  } else {
    instance = new B();
  }
  instance.m();
}

Before following your suggestion, this would be generated:

(function(f) {
  init.globalFunctions.main = B.main = f;
})(function() {
  var t1 = $.instance;
  if (J.getInterceptor(t1).$eq(t1, null) === true) {
    t1 = B.A$();
    $.instance = t1;
  } else {
    t1 = B.B$();
    $.instance = t1;
  }
  t1.m$0();
});

Now it looks like:

(function(f) {
  init.globalFunctions.main = B.main = f;
})(function() {
  if ($.instance == null) {
    var t1 = B.A$();
    $.instance = t1;
  } else {
    t1 = B.B$();
    $.instance = t1;
  }
  t1.m$0();
});

Clear improvement!

[Johnni Winther, 2014-11-14 10:20:53]

lgtm

https://codereview.chromium.org/719443002/diff/60001/dart/pkg/compiler/lib/sr...
File dart/pkg/compiler/lib/src/world.dart (right):

https://codereview.chromium.org/719443002/diff/60001/dart/pkg/compiler/lib/sr...
dart/pkg/compiler/lib/src/world.dart:100: bool get hasClosedWorldAssumption;
Document what this implies.

As I read the use it seem to be more like 'canUseClosedWorldAssumption' in which
case the value is `!compiler.hasIncrementalSupport`.

[ahe, 2014-11-14 14:40:05]

Thank you, Johnni!

I uploaded a new patch set where I negated hasClosedWorldAssumption.

https://codereview.chromium.org/719443002/diff/60001/dart/pkg/compiler/lib/sr...
File dart/pkg/compiler/lib/src/world.dart (right):

https://codereview.chromium.org/719443002/diff/60001/dart/pkg/compiler/lib/sr...
dart/pkg/compiler/lib/src/world.dart:100: bool get hasClosedWorldAssumption;
On 2014/11/14 10:20:53, Johnni Winther wrote:
> Document what this implies.
> 
> As I read the use it seem to be more like 'canUseClosedWorldAssumption' in
which
> case the value is `!compiler.hasIncrementalSupport`.

I got this wrong. The return value should have been negated, and the use should
be negated.

[ahe, 2014-11-17 09:56:24]

Committed patchset #4 (id:80001) manually as 41769 (presubmit successful).

[Johnni Winther, 2014-11-17 09:56:32]

LGTM

https://codereview.chromium.org/719443002/diff/80001/dart/pkg/compiler/lib/sr...
File dart/pkg/compiler/lib/src/universe/universe.dart (right):

https://codereview.chromium.org/719443002/diff/80001/dart/pkg/compiler/lib/sr...
dart/pkg/compiler/lib/src/universe/universe.dart:750: if
(!world.hasClosedWorldAssumption) {
Add a TODO (for me) to improve the use of TypedSelector in an open world.