On Wed, 5 Feb 2025 21:24:25 GMT, Dean Long <dl...@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
