Canonicalize function types

Canonicalize function types.  Brings sample app load time
down to the 6-7s range.


BUG=
R=sra@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/21252fe99b55f1bbd041009ccdddb4e444485b6a

[Leaf, 2016-05-10 23:18:16]

Description was changed from

==========
Canonicalize function types

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

to

==========
Canonicalize function types.  Brings sample app load time
down to the 6-7s range.


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

[Leaf, 2016-05-10 23:18:16]

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

[sra1, 2016-05-10 23:48:59]

LGTM but I'm concerned about some reflective path providing extras = [].

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
File tool/input_sdk/private/ddc_runtime/types.dart (right):

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:127: final _fnTypeNamedArgMap =
JS('', 'new Map()');
maybe a comment or two about what the structure of the Maps?

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:146: static _shaveParameter(a) {
Maybe there is a better name.

[a, b, c]  -->  [a, [b, c]]

What are these two forms called?

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:159: return arr;
can you just return the above rather than overwriting the closed variable 'arr'?

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:177: return named;
ditto about not assigning the closed variable.

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:182: result = map.get(key);
merge this and previous line?

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:221: return
FunctionType._createSmall(args.length, definite, returnType, args);
line lengths

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:229: } else if (extra instanceof
Array) {
can extra be an empty array, making this case return a different object to the
case below or the fast path above, when it is semantically the same type?

[Leaf, 2016-05-11 01:42:49]

PTAL.

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
File tool/input_sdk/private/ddc_runtime/types.dart (right):

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:127: final _fnTypeNamedArgMap =
JS('', 'new Map()');
On 2016/05/10 23:48:58, sra1 wrote:
> maybe a comment or two about what the structure of the Maps?

Done.

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:146: static _shaveParameter(a) {
On 2016/05/10 23:48:59, sra1 wrote:
> Maybe there is a better name.
> 
> [a, b, c]  -->  [a, [b, c]]
> 
> What are these two forms called?

Changed the name, added comment.

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:159: return arr;
On 2016/05/10 23:48:59, sra1 wrote:
> can you just return the above rather than overwriting the closed variable
'arr'?

Done.

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:177: return named;
On 2016/05/10 23:48:58, sra1 wrote:
> ditto about not assigning the closed variable.

Done.

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:182: result = map.get(key);
On 2016/05/10 23:48:58, sra1 wrote:
> merge this and previous line?

Done.

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:221: return
FunctionType._createSmall(args.length, definite, returnType, args);
On 2016/05/10 23:48:58, sra1 wrote:
> line lengths

Done.

https://codereview.chromium.org/1969523002/diff/80001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:229: } else if (extra instanceof
Array) {
On 2016/05/10 23:48:59, sra1 wrote:
> can extra be an empty array, making this case return a different object to the
> case below or the fast path above, when it is semantically the same type?

We don't rely on having a canonical instance, so it doesn't matter if we miss
some corner cases.  As discussed, added a comment pointing out the possible
canonicalization miss.

[sra1, 2016-05-11 04:48:41]

lgtm

[Leaf, 2016-05-11 17:08:33]

Description was changed from

==========
Canonicalize function types.  Brings sample app load time
down to the 6-7s range.


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

to

==========
Canonicalize function types.  Brings sample app load time
down to the 6-7s range.


BUG=
R=sra@google.com

Committed:
https://github.com/dart-lang/dev_compiler/commit/21252fe99b55f1bbd041009ccddd...
==========

[Leaf, 2016-05-11 17:08:35]

Committed patchset #6 (id:100001) manually as
21252fe99b55f1bbd041009ccdddb4e444485b6a (presubmit successful).