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