RFR: 8346567: Make Class.getModifiers() non-native [v2]

[Dean Long]

On Tue, 4 Feb 2025 14:43:51 GMT, Coleen Phillimore <coleenp at 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.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix copyright and param name

src/hotspot/share/compiler/compileLog.cpp line 116:

> 114:         print(" unloaded='1'");
> 115:       } else {
> 116:         print(" flags='%d'", klass->access_flags());

There may be tools that parse the log file and get confused by this change.  Maybe we should also change the label from "flags" to "access flags".

src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSet.cpp line 350:

> 348:   writer->write(mark_symbol(klass, leakp));
> 349:   writer->write(package_id(klass, leakp));
> 350:   writer->write(klass->compute_modifier_flags());

Isn't this much more expensive than grabbing the value from the mirror, especially if we have to iterate over inner classes?

src/hotspot/share/oops/instanceKlass.hpp line 1128:

> 1126: #endif
> 1127: 
> 1128:   int compute_modifier_flags() const;

I don't see why this can't stay u2.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1943680670
PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1943679056
PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1943682936