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

Coleen Phillimore coleenp at openjdk.org
Thu Feb 6 13:13:29 UTC 2025


On Wed, 5 Feb 2025 21:24:25 GMT, Dean Long <dlong at openjdk.org> wrote:

>> 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".

Okay, I wanted to remove the one use of ciKlass::modifier_flags() and the field with this change, but I'll add it back since I added a Klass::modifier_flags() function.

> 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?

I was trying not to add a Klass::modifier_flags function, but now I have.

> src/hotspot/share/opto/memnode.cpp line 2458:
> 
>> 2456:           return TypePtr::NULL_PTR;
>> 2457:         }
>> 2458:         // ???
> 
> I suspect that we still need this code to support intrinsics like LibraryCallKit::inline_native_classID() and maybe other users of this field, but the comment below no longer makes sense.

Thank you for noticing the ??? that I left in and the comment.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1944651499
PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1944640356
PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1944697467


More information about the serviceability-dev mailing list