Reapply "VM: Add result cid information for recognized methods."

Reapply "VM: Add result cid information for recognized methods."

 - _HashVMBase._index is nullable
 - _HashVMBase._data is a regular Array
 - DBC fixes from Zach.

R=fschneider@google.com

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

[rmacnak, 2016-06-02 00:28:20]

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

[Florian Schneider, 2016-06-02 12:39:39]

LGTM.

Thanks for fixing!

[Florian Schneider, 2016-06-02 12:42:12]

https://codereview.chromium.org/2033553002/diff/1/runtime/vm/method_recognizer.h
File runtime/vm/method_recognizer.h (right):

https://codereview.chromium.org/2033553002/diff/1/runtime/vm/method_recognize...
runtime/vm/method_recognizer.h:141: V(_HashVMBase, get:_index,
LinkedHashMap_getIndex, Dynamic,                  \
I wonder if we can change the implementation so that it is always non-nullable.

[rmacnak, 2016-06-02 16:58:24]

https://codereview.chromium.org/2033553002/diff/1/runtime/vm/method_recognizer.h
File runtime/vm/method_recognizer.h (right):

https://codereview.chromium.org/2033553002/diff/1/runtime/vm/method_recognize...
runtime/vm/method_recognizer.h:141: V(_HashVMBase, get:_index,
LinkedHashMap_getIndex, Dynamic,                  \
On 2016/06/02 12:42:12, Florian Schneider wrote:
> I wonder if we can change the implementation so that it is always
non-nullable.

Not sure. The array is set to null during snapshotting so the reader will
rehash.

[rmacnak, 2016-06-02 16:59:52]

Description was changed from

==========
Reapply "VM: Add result cid information for recognized methods."

 - _HashVMBase._index is nullable
 - _HashVMBase._data is a regular Array
 - DBC fixes from Zach.
==========

to

==========
Reapply "VM: Add result cid information for recognized methods."

 - _HashVMBase._index is nullable
 - _HashVMBase._data is a regular Array
 - DBC fixes from Zach.

R=fschneider@google.com

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

[rmacnak, 2016-06-02 16:59:53]

Committed patchset #1 (id:1) manually as
2e9c5f9a91eb25f09ee1e844c7a4f0273d138f79 (presubmit successful).

[regis, 2016-06-02 22:17:17]

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

[regis, 2016-06-02 22:17:18]

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

https://codereview.chromium.org/2033553002/diff/1/runtime/vm/object.cc#newcod...
runtime/vm/object.cc:7127: prefix, fp, prefix, SourceFingerprint());
The generated string does not match anymore, making it impossible to update
fingerprints.

The real question is whether these fingerprints are really helping finding bugs.
I have spent numerous hours updating them after a trivial change (type hash
change, new token, etc...).

I vote for removing them totally.

[regis, 2016-06-02 22:33:31]

On 2016/06/02 22:17:18, regis wrote:
> https://codereview.chromium.org/2033553002/diff/1/runtime/vm/object.cc
> File runtime/vm/object.cc (right):
> 
>
https://codereview.chromium.org/2033553002/diff/1/runtime/vm/object.cc#newcod...
> runtime/vm/object.cc:7127: prefix, fp, prefix, SourceFingerprint());
> The generated string does not match anymore, making it impossible to update
> fingerprints.
> 
> The real question is whether these fingerprints are really helping finding
bugs.
> I have spent numerous hours updating them after a trivial change (type hash
> change, new token, etc...).
> 
> I vote for removing them totally.

In the meantime, this quick fix is good enough and works better than the
previous version, which needed manual escaping of several characters (this one
may fail in case of a fp collision) 
      THR_Print("s/ %d)/ %d)/\n", fp, SourceFingerprint());

I'll send a cl.