Implement eager instantiation and canonicalization of type arguments at run

Implement eager instantiation and canonicalization of type arguments at run
time using a cache in uninstantiated type arguments.
Remove InstantiatedTypeArguments class.
Fix a bug in optimized code: do not update unused type test cache for Smi.
Fix a bug in optimized code on ARM and MIPS in type test cache lookup.

R=iposva@google.com, srdjan@google.com

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

[regis, 2014-02-04 23:55:25]

This is too early to review, since I want to remove InstantiatedTypeArguments
completely, as well as the temporary flag --canonicalize_type_arguments_eagerly.

This first patch can be found on golem4 under r32302 as regis-e5z7.

Early comments are welcome.

Thanks,
Regis

[regis, 2014-02-06 22:33:12]

On 2014/02/04 23:55:25, regis wrote:
> This is too early to review, since I want to remove InstantiatedTypeArguments
> completely, as well as the temporary flag
--canonicalize_type_arguments_eagerly.
> 
> This first patch can be found on golem4 under r32302 as regis-e5z7.
> 
> Early comments are welcome.
> 
> Thanks,
> Regis

This is now ready for submission. Please, have another look.

Thanks,
Regis

[regis, 2014-02-06 23:17:01]

On 2014/02/06 22:33:12, regis wrote:
> On 2014/02/04 23:55:25, regis wrote:
> > This is too early to review, since I want to remove
InstantiatedTypeArguments
> > completely, as well as the temporary flag
> --canonicalize_type_arguments_eagerly.
> > 
> > This first patch can be found on golem4 under r32302 as regis-e5z7.
> > 
> > Early comments are welcome.
> > 
> > Thanks,
> > Regis
> 
> This is now ready for submission. Please, have another look.
> 
> Thanks,
> Regis


BTW, this change speeds up dart2js compiling itself in checked mode from 40.3
sec down to 36.0 sec on my workstation. It makes no difference in production
mode (17.7 sec).

It also fixes ARM and MIPS in checked mode. For example, DeltaBlueIterators
(ARMv7HF) is 4352.552% faster in checked mode :-)

For other benchmarks results, see Golem: submitted as 'regis-e5z7' against
revision 32387.

[srdjan, 2014-02-07 17:32:16]

LGTM nice cleanup, nice speedup.

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:4229: length = (length == 0) ? 2 : length * 2;
The growth seems to aggressive. I would never grow than maybe 4 entries?

[Ivan Posva, 2014-02-07 18:48:43]

LGTMwC

-Ivan

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:4224: }
Please add a comment explaining that everything below here is only with a
successful instantiation.

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/stub_code_ia...
File runtime/vm/stub_code_ia32.cc (right):

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/stub_code_ia...
runtime/vm/stub_code_ia32.cc:1172: __ jmp(&slow_case);
Slow paths should mirror the conditional branches.

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/stub_code_ia...
runtime/vm/stub_code_ia32.cc:1235: __ movl(EDX, Address(ESP,
kObjectTypeArgumentsOffset));
This part is only needed in the else block of the above if-statement. Which will
save the use of the slow_case_with_type_arguments label.

[regis, 2014-02-07 19:41:58]

Thanks!

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:4224: }
On 2014/02/07 18:48:43, Ivan Posva wrote:
> Please add a comment explaining that everything below here is only with a
> successful instantiation.

Done.

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:4229: length = (length == 0) ? 2 : length * 2;
On 2014/02/07 17:32:17, srdjan wrote:
> The growth seems to aggressive. I would never grow than maybe 4 entries?

Done:
    length = (length == 0) ? 2 : length + 4;

But I will revisit in a follow-up change after more measurements.

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/stub_code_ia...
File runtime/vm/stub_code_ia32.cc (right):

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/stub_code_ia...
runtime/vm/stub_code_ia32.cc:1172: __ jmp(&slow_case);
On 2014/02/07 18:48:43, Ivan Posva wrote:
> Slow paths should mirror the conditional branches.

Used slow_case_with_type_arguments label.

https://codereview.chromium.org/154393003/diff/130001/runtime/vm/stub_code_ia...
runtime/vm/stub_code_ia32.cc:1235: __ movl(EDX, Address(ESP,
kObjectTypeArgumentsOffset));
On 2014/02/07 18:48:43, Ivan Posva wrote:
> This part is only needed in the else block of the above if-statement. Which
will
> save the use of the slow_case_with_type_arguments label.

This part is needed as the target of slow_case_reload_type_arguments and if
--inline_alloc is false or if IsAllocatableInNewSpace is false.

So I am not quite following. I can revisit when I introduce the sentinel_array.

[regis, 2014-02-07 19:54:49]

Committed patchset #3 manually as r32447 (presubmit successful).

[srdjan, 2014-02-07 21:59:46]

https://codereview.chromium.org/154393003/diff/320001/runtime/vm/code_generat...
File runtime/vm/code_generator.cc (right):

https://codereview.chromium.org/154393003/diff/320001/runtime/vm/code_generat...
runtime/vm/code_generator.cc:372: TypeArguments::Handle();
This should fit on one line.

https://codereview.chromium.org/154393003/diff/320001/runtime/vm/dart_api_imp...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/154393003/diff/320001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:3165: TypeArguments::Handle(isolate,
type_obj.arguments());
Indent 4 spaces