RFR: 8265129: Add intrinsic support for JVM.getClassId [v6]
Denghui Dong
ddong at openjdk.java.net
Tue May 18 15:21:45 UTC 2021
On Tue, 18 May 2021 14:39:42 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:
> Hi Denghui,
>
> My testing did not find any more issues but this should be reviewed by someone who knows the JFR internals better than me. Just some quick comments:
>
Thank you.
Erik suggested that this patch should be reviewed by the compiler team, so I'm not sure who should review the patch...
> * Your bug/PR title is misleading. You are not adding an intrinsic for JVM.getClassId but you are updating the existing intrinsic to handle more cases
> * You are completely removing the C1 version of the intrinsic, I'm wondering if that is okay, assuming there was a good reason for adding it in the first place.
In fact, the existing intrinsic implementation for JVM.getClassId is not used, I think the reason is that the underlying implementation of getClassId changed, but the corresponding intrinsic implementation has not been maintained. In other words, the intrinsic implementation for this method in C1 and C2 is wrong.
At the same time, we can find a method named getClassIdNonIntrinsic in JVM. Java
This title may not be appropriate. How about "Reimplementing the intrinsics of JVM.getClassId"
I have already implemented the C1 part, but it contains some cross platform code(only support x86 and aarch64), I think it's better to implement it in C2 first.
> * You wrote "Therefore, intensifying this method will decrease the overhead for this usage." Is that really measurable?
>
I only wrote a microbenchmark, and I can see the obvious performance improvement from it. (see my previous message).
-------------
PR: https://git.openjdk.java.net/jdk/pull/3470
More information about the hotspot-compiler-dev
mailing list