Add a whitelist for functions that we always want to inline.

Add a whitelist for functions that we always want to inline.

R=johnniwinther@google.com

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

[karlklose, 2014-09-04 07:19:56]

karlklose@google.com changed reviewers:
+ floitsch@google.com

[Johnni Winther, 2014-09-04 07:33:39]

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

[Johnni Winther, 2014-09-04 07:33:40]

lgtm

https://codereview.chromium.org/543583002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/js_backend/backend.dart (right):

https://codereview.chromium.org/543583002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:86: /// List
if [FunctionElement]s that we want to inline always.  This list is
'List if' -> 'List of'.

https://codereview.chromium.org/543583002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:1701:
internalLibrary = library;
Put this in [onLibraryCreated] instead.

[karlklose, 2014-09-04 07:54:05]

Thanks, Johnni.

https://codereview.chromium.org/543583002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/js_backend/backend.dart (right):

https://codereview.chromium.org/543583002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:86: /// List
if [FunctionElement]s that we want to inline always.  This list is
On 2014/09/04 07:33:40, Johnni Winther wrote:
> 'List if' -> 'List of'.

Done.

https://codereview.chromium.org/543583002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:1701:
internalLibrary = library;
On 2014/09/04 07:33:40, Johnni Winther wrote:
> Put this in [onLibraryCreated] instead.

Done.

[karlklose, 2014-09-04 12:55:24]

On 2014/09/04 07:54:05, karlklose wrote:
> Thanks, Johnni.
> 
>
https://codereview.chromium.org/543583002/diff/1/sdk/lib/_internal/compiler/i...
> File sdk/lib/_internal/compiler/implementation/js_backend/backend.dart
(right):
> 
>
https://codereview.chromium.org/543583002/diff/1/sdk/lib/_internal/compiler/i...
> sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:86: /// List
> if [FunctionElement]s that we want to inline always.  This list is
> On 2014/09/04 07:33:40, Johnni Winther wrote:
> > 'List if' -> 'List of'.
> 
> Done.
> 
>
https://codereview.chromium.org/543583002/diff/1/sdk/lib/_internal/compiler/i...
> sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:1701:
> internalLibrary = library;
> On 2014/09/04 07:33:40, Johnni Winther wrote:
> > Put this in [onLibraryCreated] instead.
> 
> Done.

PTAL, I removed the inlining of ListIterator.moveNext because it has negative
performance implications that I need to take a closer look at.

[Johnni Winther, 2014-09-04 12:57:15]

lgtm

[karlklose, 2014-09-04 13:35:25]

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

[sra1, 2014-12-16 02:05:41]

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

[sra1, 2014-12-16 02:05:41]

We already have a NoInline annotation available to the internal libraries.

Could this CL be replaced by a ForceInline annotation?

[sra1, 2014-12-16 02:05:50]

We already have a NoInline annotation available to the internal libraries.

Could this CL be replaced by a ForceInline annotation?

[karlklose, 2014-12-16 07:43:03]

We could use an annotation, but I did not want to introduce a new annotation to
non-dart2js libraries.

[floitsch, 2014-12-16 11:49:46]

On 2014/12/16 07:43:03, karlklose wrote:
> We could use an annotation, but I did not want to introduce a new annotation
to
> non-dart2js libraries.

I'm torn.
I agree with Karl that adding dart2js specific annotations to non-dart2js
libraries feels wrong.
However, the current approach is brittle: changes to the core-libraries might
make the list go stale without us really noticing. For example, the
IterableMixinWorkaround class is phased out, and there are fewer and fewer
users.

[karlklose, 2014-12-16 12:09:21]

On 2014/12/16 11:49:46, floitsch wrote:
> On 2014/12/16 07:43:03, karlklose wrote:
> > We could use an annotation, but I did not want to introduce a new annotation
> to
> > non-dart2js libraries.
> 
> I'm torn.
> I agree with Karl that adding dart2js specific annotations to non-dart2js
> libraries feels wrong.
> However, the current approach is brittle: changes to the core-libraries might
> make the list go stale without us really noticing. For example, the
> IterableMixinWorkaround class is phased out, and there are fewer and fewer
> users.

When the target of this list cannot be found, there will be an exception. That
means that we will not silently ignore these changes. However, if the
implementation changes in a way that makes inlining not desirable, we will not
notice.