Save and restore feedback from JIT.

Save and restore feedback from JIT.

 - Adjust fingerprints to be independent of the library's private key, which varies with load order.
 - Use usage_counter in AOT inlining decisions if JIT feedback is available.
 - Reduce inlining in cold functions.

dart2js product aot snapshot 15841351 -> 14436273 (-8.86%)

R=fschneider@google.com

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

[rmacnak, 2016-12-13 21:24:16]

Description was changed from

==========
Save and restore feedback from JIT.
==========

to

==========
Save and restore feedback from JIT.

 - Adjust fingerprints to be independent of the library's private key, which
varies with load order.
 - Use usage_counter in AOT inlining decisions if JIT feedback is available.
 - Reduce inlining in cold functions.

dart2js product aot snapshot 15841351 -> 14436273 (-8.86%)
==========

[rmacnak, 2016-12-13 21:24:34]

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

[Florian Schneider, 2016-12-15 19:09:36]

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/dart_api_im...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:6550: intptr_t kNumArgsChecked =
ic_data_.NumArgsTested();
s/kNumArgsChecked/num_args_checked/

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser.cc
File runtime/vm/json_parser.cc (right):

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser...
runtime/vm/json_parser.cc:9: ParsedJSONArray* ParsedJSONObject::ArrayAt(const
char* key) {
Can this be moved into the .h file by using a forward declaration?

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser.h
File runtime/vm/json_parser.h (right):

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser...
runtime/vm/json_parser.h:18: ParsedJSONValue() {}
Empty constructor not needed.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser...
runtime/vm/json_parser.h:64: ParsedJSONNull() {}
Empty constructor not needed.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser...
runtime/vm/json_parser.h:71: explicit ParsedJSONObject(intptr_t length,
ParsedJSONValue** keys_and_values)
explicit not needed.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser...
runtime/vm/json_parser.h:121: explicit ParsedJSONArray(intptr_t length,
ParsedJSONValue** elements)
explicit not needed.

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

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/precompiler...
runtime/vm/precompiler.cc:2825: zone, ICData::New(function,
String::Handle(target.name()),
Pass zone into String::Handle as well.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/precompiler...
runtime/vm/precompiler.cc:2835: if (this == NULL) {
Check this at the call-site to avoid C++ undefined behavior.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/precompiler...
runtime/vm/precompiler.cc:2907: intptr_t kNumArgsChecked = ic.NumArgsTested();
Not really a compile-time constant:

s/kNumArgsChecked/num_args_checked/

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/precompiler.h
File runtime/vm/precompiler.h (right):

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/precompiler...
runtime/vm/precompiler.h:366: return key.token_ ^ key.owner_cid_;
Add kind_ to the computation?

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/program_vis...
File runtime/vm/program_visitor.h (right):

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/program_vis...
runtime/vm/program_visitor.h:27: static void VisitFunctions(FunctionVisitor*
visitor);
Can we  make these two top-level functions?

[rmacnak, 2016-12-15 21:27:19]

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/dart_api_im...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:6550: intptr_t kNumArgsChecked =
ic_data_.NumArgsTested();
On 2016/12/15 19:09:35, Florian Schneider wrote:
> s/kNumArgsChecked/num_args_checked/

Done.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser.cc
File runtime/vm/json_parser.cc (right):

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser...
runtime/vm/json_parser.cc:9: ParsedJSONArray* ParsedJSONObject::ArrayAt(const
char* key) {
On 2016/12/15 19:09:35, Florian Schneider wrote:
> Can this be moved into the .h file by using a forward declaration?

Yes, I can mark in inline.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser.h
File runtime/vm/json_parser.h (right):

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser...
runtime/vm/json_parser.h:18: ParsedJSONValue() {}
On 2016/12/15 19:09:35, Florian Schneider wrote:
> Empty constructor not needed.

Done.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser...
runtime/vm/json_parser.h:64: ParsedJSONNull() {}
On 2016/12/15 19:09:35, Florian Schneider wrote:
> Empty constructor not needed.

Done.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser...
runtime/vm/json_parser.h:71: explicit ParsedJSONObject(intptr_t length,
ParsedJSONValue** keys_and_values)
On 2016/12/15 19:09:35, Florian Schneider wrote:
> explicit not needed.

Done.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/json_parser...
runtime/vm/json_parser.h:121: explicit ParsedJSONArray(intptr_t length,
ParsedJSONValue** elements)
On 2016/12/15 19:09:35, Florian Schneider wrote:
> explicit not needed.

Done.

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

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/precompiler...
runtime/vm/precompiler.cc:2825: zone, ICData::New(function,
String::Handle(target.name()),
On 2016/12/15 19:09:35, Florian Schneider wrote:
> Pass zone into String::Handle as well.

Done.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/precompiler...
runtime/vm/precompiler.cc:2835: if (this == NULL) {
On 2016/12/15 19:09:35, Florian Schneider wrote:
> Check this at the call-site to avoid C++ undefined behavior.

Done.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/precompiler...
runtime/vm/precompiler.cc:2907: intptr_t kNumArgsChecked = ic.NumArgsTested();
On 2016/12/15 19:09:35, Florian Schneider wrote:
> Not really a compile-time constant:
> 
> s/kNumArgsChecked/num_args_checked/

Done.

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/program_vis...
File runtime/vm/program_visitor.h (right):

https://codereview.chromium.org/2562693003/diff/100001/runtime/vm/program_vis...
runtime/vm/program_visitor.h:27: static void VisitFunctions(FunctionVisitor*
visitor);
On 2016/12/15 19:09:35, Florian Schneider wrote:
> Can we  make these two top-level functions?

I think with a class the friend declarations are easier to understand.

[Florian Schneider, 2016-12-16 18:00:36]

Lgtm

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

https://codereview.chromium.org/2562693003/diff/120001/runtime/vm/precompiler...
runtime/vm/precompiler.cc:2899: ASSERT(entry != NULL);
How are calls where num_args_checked == 2 handled? (MathMin, MathMax, etc)?

[rmacnak, 2016-12-16 18:09:34]

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

https://codereview.chromium.org/2562693003/diff/120001/runtime/vm/precompiler...
runtime/vm/precompiler.cc:2899: ASSERT(entry != NULL);
On 2016/12/16 18:00:36, Florian Schneider wrote:
> How are calls where num_args_checked == 2 handled? (MathMin, MathMax, etc)?

The count is at the end for these too.

[rmacnak, 2016-12-16 18:21:44]

Description was changed from

==========
Save and restore feedback from JIT.

 - Adjust fingerprints to be independent of the library's private key, which
varies with load order.
 - Use usage_counter in AOT inlining decisions if JIT feedback is available.
 - Reduce inlining in cold functions.

dart2js product aot snapshot 15841351 -> 14436273 (-8.86%)
==========

to

==========
Save and restore feedback from JIT.

 - Adjust fingerprints to be independent of the library's private key, which
varies with load order.
 - Use usage_counter in AOT inlining decisions if JIT feedback is available.
 - Reduce inlining in cold functions.

dart2js product aot snapshot 15841351 -> 14436273 (-8.86%)

R=fschneider@google.com

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

[rmacnak, 2016-12-16 18:21:45]

Committed patchset #7 (id:120001) manually as
31ee20297e83e88a9a1b6002be906fea9e43eca0 (presubmit successful).