On Mon, 9 Dec 2024 19:26:53 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
> The Class.getModifiers() method is implemented as a native method in
> java.lang.Class to access a field that we've calculated when creating the
> mirror. The field is final after that point. The VM doesn't need it anymore,
> so there's no real need for the jdk code to call into the VM to get it. This
> moves the field to Java and removes the intrinsic code. I promoted the
> compute_modifiers() functions to return int since that's how java.lang.Class
> uses the value. It should really be an unsigned short though.
>
> There's a couple of JMH benchmarks added with this change. One does show
> that for array classes for non-bootstrap class loader, this results in one
> extra load which in a long loop of just that, is observable. I don't think
> this is real life code. The other benchmarks added show no regression.
>
> Tested with tier1-8.
src/java.base/share/classes/java/lang/Class.java line 1005:
> 1003: private transient Object[] signers; // Read by VM, mutable
> 1004:
> 1005: @Stable
The `modifiers` field doesn’t need to be `@Stable`:
Suggestion:
test/micro/org/openjdk/bench/java/lang/reflect/Clazz.java line 65:
> 63: */
> 64: @Benchmark
> 65: public int getModifiers() throws NoSuchMethodException {
The only `Throwable`s that can be thrown by calling `Class::getModifiers()` are
`Error`s (e.g.: `StackOverflowError`) and `RuntimeException`s (e.g.:
`NullPointerException`):
Suggestion:
public int getModifiers() {
test/micro/org/openjdk/bench/java/lang/reflect/Clazz.java line 71:
> 69: Clazz[] clazzArray = new Clazz[1];
> 70: @Benchmark
> 71: public int getAppArrayModifiers() throws NoSuchMethodException {
Suggestion:
public int getAppArrayModifiers() {
test/micro/org/openjdk/bench/java/lang/reflect/Clazz.java line 81:
> 79: */
> 80: @Benchmark
> 81: public int getArrayModifiers() throws NoSuchMethodException {
Suggestion:
public int getArrayModifiers() {
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1888757754
PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1888760732
PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1888760967
PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1888761412
