[GR-37191] Record equals/hashCode use reflection

[TheMode]

Describe the issue
Record equals (and I assume hashCode, even if untested) currently use reflection, and effectively run much slower (about x10-20 in my case compared to my specialized method comparing the fields directly)

Describe GraalVM and your environment:

• GraalVM 22.0.0.2
• JDK major version: 17
• OS: Linux/PopOS
• Architecture:AMD64

More details
I have noticed the issue due to a hot path in my code being weirdly expensive:
https://github.com/Minestom/Minestom/blob/2c0b026e46f271a88cd9824e00f211c25980a7f1/src/main/java/net/minestom/server/listener/PlayerPositionListener.java#L36
https://github.com/Minestom/Minestom/blob/2c0b026e46f271a88cd9824e00f211c25980a7f1/src/main/java/net/minestom/server/entity/Entity.java#L1239
(notice the Pos#equals calls)

Flamegraph obtained with perf:

[TheMode]

Is there any plan to "fix" it? It does make native-image unsuitable for any jdk16+ project, and seem to be, I assume, easily fixable.

[oubidar-Abderrahim]

Hi, Thank you for reporting this, We will take a look into it and get back to you.

[oubidar-Abderrahim]

Tracked internally on GR--37191

[loicottet]

This is due to our current method handle implementation, which uses reflection internally and has quite poor performance as a result. We are planning on changing this implementation in the near future, which should make this code run faster.

[TheMode]

What about specializing those record methods to do not use method handles? I assume that it would give you more implementation freedom.

[loicottet]

The problem is that these record methods do not simply use method handles, they are method handles. This makes it more difficult to substitute them out, since we would have to generate alternative methods for every record class in the image. We also want to introduce substitutions only when strictly necessary, and will therefore primarily focus on improving the performance of our method handle implementation.

[TheMode]

My issue in this case is that improving the performance of method handles may not be enough, as equals/hashCode are likely to be present in a lot of hot path. I would consider it a bug for my hand-coded methods to perform better.

I can however understand that exceptions to the rule are not always desirable.

[loicottet]

Our goal for this reimplementation would be to use JDK-generated, optimized bytecode method handle invokers when the handle is known at build time, which is the case for record methods. We expect to see a performance similar to a hand-written method in that case.

[kkriske]

What is the current state of this? Did the mentioned rewrite happen or is there possibly an indication of a timeline when it will happen?

[loicottet]

This is still on the to-do list with a fairly high priority, but there is no fixed timeline for this change yet.

[kkriske]

@loicottet This pr #7196 and more specifically the change-log entry below, added in this pr #7262 vaguely mention method handle intrinsification of records:

graal/substratevm/CHANGELOG.md

Line 23 in 35634fa

* (GR-29688) More sophisticated intrinsification and inlining of method handle usages, both explicit and implicit (lambdas, string concatenation and record classes), and various fixes for non-intrinsified usages.

Does that address this issue as well?

[loicottet]

@peter-hofer can you confirm this?

[christianwimmer]

The performance problem of equals, hashCode, and toString methods for records will be fixed by #8109.

The plan is to integrate the fix into the upcoming GraalVM for JDK 22, and also backport to GraalVM for JDK 21 in the April CPU release.

[luneo7]

Can this be backported to release/graal-vm/23.0 (JDK 17) as well?

[christianwimmer]

Can this be backported to release/graal-vm/23.0 (JDK 17) as well?

This is not planned, since it would require backporting a lot of feature code into JDK 17.

[kristofdho]

and also backport to GraalVM for JDK 21 in the April CPU

@christianwimmer I'm thorought confused, there still is no 21.0.3 available, and it's not on the release calendar either. Is the release still in the works but delayed? I can't find any information on this.
I know this is not really the place to ask but I've tried asking this on slack as well but I've not been able to reach anyone.

[luneo7]

There are LibericaNIK builds with the fix https://github.com/bell-sw/LibericaNIK/releases/tag/23.1.3%2B1-21.0.3%2B10

[christianwimmer]

All the current GraalVM for JDK 21 downloads on graalvm.org are for the latest CPU release, so downloading GraalVM for JDK 21 gives you the CPU release 21.0.3.

[kkriske]

Those are Oracle GraalVM downloads which we cannot use due to the license.
The latest CE release is 21.0.2 which does not contain this fix: https://github.com/graalvm/graalvm-ce-builds/releases/
So is the community edition for GraalVM for JDK 21 already unmaintained?

[christianwimmer]

Those are Oracle GraalVM downloads which we cannot use due to the license.

Oracle GraalVM 21.0.3 is free to use, for all commercial purposes.

[kkriske]

That's what Oracle keeps repeating, but unless the license changed, it's not allowed to distibute binaries to paying customers, ergo not usable for us.

Oracle grants to You, as a recipient of this Program, subject to the conditions stated herein, a nonexclusive, nontransferable, limited license to:
[...]
(b) redistribute the unmodified Program and Program Documentation, under the terms of this License, provided that You do not charge Your licensees any fees associated with such distribution or use of the Program, including, without limitation, fees for products that include or are bundled with a copy of the Program or for services that involve the use of the distributed Program.

https://www.oracle.com/downloads/licenses/graal-free-license.html

[luneo7]

Yup, that's what we've read from the Graal free license as well... if we use it... as we do charge a "service fee" for our stuff in production, if we use Oracle GraalVM we would be breaking license...
GraalVM CE should be publishing versions though... as there is no new LTS version out there...

[luneo7]

LibericaNIK in the other hand right now is free to use.... it is mainly GraalVM CE + ParallelGC (which is seems to be not production ready), and sometimes some backport... it is the default native image for Spring now... BellSW charges for support (seems like RedHat), not for usage of their native image built version...