RFR: 8332249: Micro-optimize Method.hashCode

[Sean Gwizdak]

On Wed, 29 May 2024 15:20:56 GMT, Chen Liang <liach at openjdk.org> wrote:

> Also, have you considered micro-optimize Constructor.hashCode too? Given its similar status to Method.

Not at this time. This PR is motivated by observing Method.hashCode as a hotspot in a Spring heavy application when migrating to an ARM based platform. We did not observe Constructor.hashCode to be a hotspot in this application.

> src/java.base/share/classes/java/lang/reflect/Method.java line 99:
> 
>> 97:     private Method              root;
>> 98:     // Hash code of this object
>> 99:     private int                 hash;
> 
> We can declare this field `@Stable`, like the `methodAccessor`.

This was something that I tried and I observed a performance regression on the ARM platform that I was testing. From my understanding, `@Stable` tells the compiler to trust the contents of this field which is only given when the field holder is a known constant. This is generally true for cases like `Enums` but not usually for `Method`. 


Benchmark                         Mode  Cnt  Score   Error  Units
# Intel Skylake
MethodHashCode.benchmarkHashCode  avgt    5  1.113 ± 1.146  ns/op
# Arm Neoverse N1
MethodHashCode.benchmarkHashCode  avgt    5  3.204 ± 0.001  ns/op

> src/java.base/share/classes/java/lang/reflect/Method.java line 388:
> 
>> 386:         int hc = hash;
>> 387: 
>> 388:         if (hc == 0) {
> 
> Should we add an extra field `hashIsZero` like in `String` to avoid repeated computation if the hash is 0?

Unlike with `String`, I don't often see cases where `Method` results in a hashCode of zero, which should be weighed with the overhead of adding in an additional variable. I believe some of the reasoning for handling zero-hashed Strings was about how easy it is to construct hash collisions and feed the JVM with bad Strings from an external actor. I don't believe this is an issue for `Method`.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/19433#issuecomment-2137699178
PR Review Comment: https://git.openjdk.org/jdk/pull/19433#discussion_r1619053915
PR Review Comment: https://git.openjdk.org/jdk/pull/19433#discussion_r1619079001