--collect_dynamic_function_names (default false): find unique virtual function names and use them t…

--collect_dynamic_function_names (default false): find unique virtual function names and use them to populate ICData (precompilation only) Measured some performance improvement, some performance loss. Next: tune inlining of getters/setters, check heuristics.

BUG=
R=rmacnak@google.com

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

[srdjan, 2015-12-02 23:03:27]

srdjan@google.com changed reviewers:
+ fschneider@google.com, rmacnak@google.com

[rmacnak, 2015-12-03 00:34:35]

https://codereview.chromium.org/1496713002/diff/20001/runtime/vm/precompiler.cc
File runtime/vm/precompiler.cc (right):

https://codereview.chromium.org/1496713002/diff/20001/runtime/vm/precompiler....
runtime/vm/precompiler.cc:785: if (function.IsDynamicFunction()) {
Regular methods need to be added under their getter name and vice versa.

Consider 

class A {
  foo() => 'A';
}
class B {
  get foo => () => 'B';
}

confuse(x) {
  try {
    throw x;
  } catch (e) {
    return e;
  }
}

main() {
  print(confuse(new A()).foo());
  print(confuse(new B()).foo());
}

[srdjan, 2015-12-04 17:14:25]

PTAL

https://codereview.chromium.org/1496713002/diff/20001/runtime/vm/precompiler.cc
File runtime/vm/precompiler.cc (right):

https://codereview.chromium.org/1496713002/diff/20001/runtime/vm/precompiler....
runtime/vm/precompiler.cc:785: if (function.IsDynamicFunction()) {
On 2015/12/03 00:34:34, rmacnak wrote:
> Regular methods need to be added under their getter name and vice versa.
> 
> Consider 
> 
> class A {
>   foo() => 'A';
> }
> class B {
>   get foo => () => 'B';
> }
> 
> confuse(x) {
>   try {
>     throw x;
>   } catch (e) {
>     return e;
>   }
> }
> 
> main() {
>   print(confuse(new A()).foo());
>   print(confuse(new B()).foo());
> }

Thanks, good catch. Fixed and also added checks (here and in optimizer) to skip
functions with named arguments (at compile time we do not know if the target is
compatible with the call instruction) and do not use the target if the argument
count of a call instruction is incompatible with the target.

[rmacnak, 2015-12-04 19:37:23]

lgtm

https://codereview.chromium.org/1496713002/diff/60001/runtime/vm/precompiler.cc
File runtime/vm/precompiler.cc (right):

https://codereview.chromium.org/1496713002/diff/60001/runtime/vm/precompiler....
runtime/vm/precompiler.cc:106: CollectDynamicFunctionNames();
Nit: Move up to keep AddRoots and Iterate together.

https://codereview.chromium.org/1496713002/diff/60001/runtime/vm/precompiler....
runtime/vm/precompiler.cc:820: // Locate all entries with one function only, and
which owner is neither
whose owner

https://codereview.chromium.org/1496713002/diff/60001/runtime/vm/precompiler....
runtime/vm/precompiler.cc:845: }
Consider something like

THR_PRINT("%d of %d dynamic selectors are unique\n", functions_set.Size(),
table.Size());

[srdjan, 2015-12-04 20:40:19]

https://codereview.chromium.org/1496713002/diff/60001/runtime/vm/precompiler.cc
File runtime/vm/precompiler.cc (right):

https://codereview.chromium.org/1496713002/diff/60001/runtime/vm/precompiler....
runtime/vm/precompiler.cc:106: CollectDynamicFunctionNames();
On 2015/12/04 19:37:23, rmacnak wrote:
> Nit: Move up to keep AddRoots and Iterate together.

Done.

https://codereview.chromium.org/1496713002/diff/60001/runtime/vm/precompiler....
runtime/vm/precompiler.cc:820: // Locate all entries with one function only, and
which owner is neither
On 2015/12/04 19:37:23, rmacnak wrote:
> whose owner

Done.

https://codereview.chromium.org/1496713002/diff/60001/runtime/vm/precompiler....
runtime/vm/precompiler.cc:845: }
On 2015/12/04 19:37:23, rmacnak wrote:
> Consider something like
> 
> THR_PRINT("%d of %d dynamic selectors are unique\n", functions_set.Size(),
> table.Size());

Done. s/Size/NumOccupied/

[srdjan, 2015-12-04 21:19:04]

Description was changed from

==========
--collect_dynamic_function_names (default false): find unique virtual function
names and use them to populate ICData (precompilation only) Measured some
performance improvement, some performance loss. Next: tune inlining of
getters/setters, check heuristics.

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

to

==========
--collect_dynamic_function_names (default false): find unique virtual function
names and use them to populate ICData (precompilation only) Measured some
performance improvement, some performance loss. Next: tune inlining of
getters/setters, check heuristics.

BUG=
R=rmacnak@google.com

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

[srdjan, 2015-12-04 21:19:05]

Committed patchset #5 (id:80001) manually as
a9ea73b60b1866a3939fe4b12ce76bc94c3834d3 (presubmit successful).