Sped up hashCode by removing megamorphic call to _identityHashCode.

Sped up hashCode by removing megamorphic call to _identityHashCode.
Microbenchmark measuring megamorhic hashCode access shows 25% speedup.

BUG=
R=asiva@google.com

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

[bakster, 2016-07-08 20:58:26]

bak@google.com changed reviewers:
+ asiva@google.com

[bakster, 2016-07-08 20:58:26]

Please take a look at these hashCode changes.

[sra1, 2016-07-08 21:35:58]

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

[sra1, 2016-07-08 21:35:59]

I'd like to see a test that doubles 'x' having integral values satisfy

    x.toInt().hashCode == x.hashCode

covering at least several values / signs from each representation of int.

---

I look forward to the immediate relief this CL provides.

I would like to see is this automated.  The VM could determine that the trivial
code like

    int get hashCode => _identityHashCode;

should always be cloned into a subclass that defines _identityHashCode without
shadowing the above definition, and then inline the single-target call. That
would pay dividends in many other situations (e.g. the public read-only accessor
for a private mutable field).

https://codereview.chromium.org/2137673002/diff/1/runtime/lib/double.dart
File runtime/lib/double.dart (right):

https://codereview.chromium.org/2137673002/diff/1/runtime/lib/double.dart#new...
runtime/lib/double.dart:11: // Code is replicated to avoid unnecessary dispath
overhead.
They could both call a common static method.
Since there is an open bug on the simplistic implementation
(https://github.com/dart-lang/sdk/issues/2884) the implementation is likely to
become more complicated for non-integral values.

[bakster, 2016-07-08 22:04:23]

The only solution that solves the general problem is selective method
customization/specialization.
This should eliminate the chained megamorphic calls.
//Lars

https://codereview.chromium.org/2137673002/diff/1/runtime/lib/double.dart
File runtime/lib/double.dart (right):

https://codereview.chromium.org/2137673002/diff/1/runtime/lib/double.dart#new...
runtime/lib/double.dart:11: // Code is replicated to avoid unnecessary dispath
overhead.
On 2016/07/08 21:35:59, sra1 wrote:
> They could both call a common static method.
> Since there is an open bug on the simplistic implementation
> (https://github.com/dart-lang/sdk/issues/2884) the implementation is likely to
> become more complicated for non-integral values.

I'll add a todo in the code for now. A static only makes sense when the code
gets more complicated.

[siva, 2016-07-08 22:12:10]

lgtm

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

https://codereview.chromium.org/2137673002/diff/1/runtime/lib/object.cc#newco...
runtime/lib/object.cc:44: // The performance benefit is more than 5% when using
hashCode.
Maybe add:

ASSERT(arguments->NativeArgAt(0)->IsDartInstance());

[bakster, 2016-07-08 22:15:11]

Thanks

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

https://codereview.chromium.org/2137673002/diff/1/runtime/lib/object.cc#newco...
runtime/lib/object.cc:44: // The performance benefit is more than 5% when using
hashCode.
On 2016/07/08 22:12:10, siva wrote:
> Maybe add:
> 
> ASSERT(arguments->NativeArgAt(0)->IsDartInstance());

Done.

[bakster, 2016-07-08 22:19:59]

Description was changed from

==========
Sped up hashCode by removing megamorphic call to _identityHashCode.
Microbenchmark measuring megamorhic hashCode access shows 25% speedup.

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

to

==========
Sped up hashCode by removing megamorphic call to _identityHashCode.
Microbenchmark measuring megamorhic hashCode access shows 25% speedup.

BUG=
R=asiva@google.com

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

[bakster, 2016-07-08 22:20:00]

Committed patchset #2 (id:20001) manually as
bc6358e5ef01b009f43aa6a88089ef72603992bc (presubmit successful).