locateSingleElement improvement for mixins.

locateSingleElement improvement for mixins.

I see several code improvements:

1. More inlining of small methods in mixin classes like ListMixin.isEmpty
2. Fewer interceptor calls where the target is now known but not inlined.
3. Implied getters and setters on mixin classes are inlined to direct field references.

Addresses https://github.com/dart-lang/sdk/issues/27160

R=johnniwinther@google.com

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

[sra1, 2016-08-26 19:45:55]

Description was changed from

==========
Try to find more inlining candidates.
==========

to

==========
locateSingleElement improvement for mixins.

I see several code improvements:

1. More inlining of small methods in mixin classes like ListMixin.isEmpty

2. Fewer interceptor calls where the target is now known but not inlined.

3. Implied getters and setters on mixin classes are inlined to direct field
references.

Addresses https://github.com/dart-lang/sdk/issues/27160
==========

[sra1, 2016-08-26 19:45:56]

sra@google.com changed reviewers:
+ johnniwinther@google.com, sigmund@google.com

[sra1, 2016-08-26 19:46:53]

Description was changed from

==========
locateSingleElement improvement for mixins.

I see several code improvements:

1. More inlining of small methods in mixin classes like ListMixin.isEmpty

2. Fewer interceptor calls where the target is now known but not inlined.

3. Implied getters and setters on mixin classes are inlined to direct field
references.

Addresses https://github.com/dart-lang/sdk/issues/27160
==========

to

==========
locateSingleElement improvement for mixins.

I see several code improvements:

1. More inlining of small methods in mixin classes like ListMixin.isEmpty
2. Fewer interceptor calls where the target is now known but not inlined.
3. Implied getters and setters on mixin classes are inlined to direct field
references.

Addresses https://github.com/dart-lang/sdk/issues/27160
==========

[sra1, 2016-08-27 02:59:31]

Patchset #4 (id:60001) has been deleted

[sra1, 2016-08-27 02:59:39]

Patchset #1 (id:1) has been deleted

[sra1, 2016-08-27 02:59:46]

Patchset #1 (id:20001) has been deleted

[sra1, 2016-08-27 02:59:53]

Patchset #1 (id:40001) has been deleted

[sra1, 2016-08-27 03:00:09]

Patchset #1 (id:80001) has been deleted

[sra1, 2016-08-27 03:02:45]

sra@google.com changed reviewers:
- sigmund@google.com

[sra1, 2016-08-27 03:02:46]

This addresses the problem that we were not inlining trivial methods from mixin
classes.

[Siggi Cherem (dart-lang), 2016-08-29 19:26:00]

sigmund@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2016-08-29 19:26:01]

https://codereview.chromium.org/2278143003/diff/100001/pkg/compiler/lib/src/t...
File pkg/compiler/lib/src/types/flat_type_mask.dart (right):

https://codereview.chromium.org/2278143003/diff/100001/pkg/compiler/lib/src/t...
pkg/compiler/lib/src/types/flat_type_mask.dart:697: // if
(classWorld.isUsedAsMixin(enclosing)) {
delete?

https://codereview.chromium.org/2278143003/diff/100001/pkg/compiler/lib/src/w...
File pkg/compiler/lib/src/world.dart (right):

https://codereview.chromium.org/2278143003/diff/100001/pkg/compiler/lib/src/w...
pkg/compiler/lib/src/world.dart:515: bool isSubclassOfMixinOf(ClassElement cls,
ClassElement mixin) {
nit: _Of_Of confuses me a bit, consider renaming?

An idea: isSubclassOfMixinApplication(cls, mixin)?

https://codereview.chromium.org/2278143003/diff/100001/pkg/compiler/lib/src/w...
pkg/compiler/lib/src/world.dart:539: subtypesOf(x).every(
I wonder how much this affects compile-time.

If it is expensive, could switching the order here help? I mean, instead of
first searching for subtypes and then checking for the subclass/mixin check, I
wonder if there is a way to use `mixinUsesOf` as a way to limit the search space
first.

[sra1, 2016-08-29 20:37:18]

https://codereview.chromium.org/2278143003/diff/100001/pkg/compiler/lib/src/w...
File pkg/compiler/lib/src/world.dart (right):

https://codereview.chromium.org/2278143003/diff/100001/pkg/compiler/lib/src/w...
pkg/compiler/lib/src/world.dart:515: bool isSubclassOfMixinOf(ClassElement cls,
ClassElement mixin) {
On 2016/08/29 19:26:00, Siggi Cherem (dart-lang) wrote:
> nit: _Of_Of confuses me a bit, consider renaming?
> 
> An idea: isSubclassOfMixinApplication(cls, mixin)?

Johnni - what do you think?

For consistency with the rest of the file I think maybe change it to
isSubclassOfMixinUseOf

https://codereview.chromium.org/2278143003/diff/100001/pkg/compiler/lib/src/w...
pkg/compiler/lib/src/world.dart:539: subtypesOf(x).every(
On 2016/08/29 19:26:01, Siggi Cherem (dart-lang) wrote:
> I wonder how much this affects compile-time.
> 
> If it is expensive, could switching the order here help? I mean, instead of
> first searching for subtypes and then checking for the subclass/mixin check, I
> wonder if there is a way to use `mixinUsesOf` as a way to limit the search
space
> first.

I didn't see any perf issues on Golem.
Two things tend to weed out massive queries:

1. The query is only called when there is a single possible target method.
2. The query is only called when the mask is a subtype mask. Many subtype masks
are strengthened to subclass masks.

[Johnni Winther, 2016-08-30 11:36:08]

lgtm

https://codereview.chromium.org/2278143003/diff/100001/pkg/compiler/lib/src/w...
File pkg/compiler/lib/src/world.dart (right):

https://codereview.chromium.org/2278143003/diff/100001/pkg/compiler/lib/src/w...
pkg/compiler/lib/src/world.dart:515: bool isSubclassOfMixinOf(ClassElement cls,
ClassElement mixin) {
On 2016/08/29 20:37:18, sra1 wrote:
> On 2016/08/29 19:26:00, Siggi Cherem (dart-lang) wrote:
> > nit: _Of_Of confuses me a bit, consider renaming?
> > 
> > An idea: isSubclassOfMixinApplication(cls, mixin)?
> 
> Johnni - what do you think?
> 
> For consistency with the rest of the file I think maybe change it to
> isSubclassOfMixinUseOf

I'd go for 'isSubclassOfMixinUseOf' or maybe `inheritsMixin(cls, mixin)`.

[sra1, 2016-08-30 21:09:08]

Description was changed from

==========
locateSingleElement improvement for mixins.

I see several code improvements:

1. More inlining of small methods in mixin classes like ListMixin.isEmpty
2. Fewer interceptor calls where the target is now known but not inlined.
3. Implied getters and setters on mixin classes are inlined to direct field
references.

Addresses https://github.com/dart-lang/sdk/issues/27160
==========

to

==========
locateSingleElement improvement for mixins.

I see several code improvements:

1. More inlining of small methods in mixin classes like ListMixin.isEmpty
2. Fewer interceptor calls where the target is now known but not inlined.
3. Implied getters and setters on mixin classes are inlined to direct field
references.

Addresses https://github.com/dart-lang/sdk/issues/27160

R=johnniwinther@google.com

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

[sra1, 2016-08-30 21:09:09]

Committed patchset #3 (id:140001) manually as
7b3bc6933a7dc26e36ac56eef472bde1eedaad1e (presubmit successful).