On Tue, 4 Feb 2025 14:43:51 GMT, Coleen Phillimore <cole...@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/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.

src/hotspot/share/opto/memnode.cpp line 2459:

> 2457:         }
> 2458:         // ???
> 2459:         // (Folds up the 1st indirection in 
> aClassConstant.getModifiers().)

Suggestion:

        // Fold up the load of the hidden field

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1943695585
PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1943696867

Reply via email to