On Tue, 4 Feb 2025 14:43:51 GMT, Coleen Phillimore <[email protected]> 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
