RFR: 8349860: Make Class.isArray(), Class.isInterface() and Class.isPrimitive() non-native

[Coleen Phillimore]

On Wed, 19 Feb 2025 02:54:05 GMT, Dean Long <dlong at openjdk.org> wrote:

>> Class.isInterface() can check modifier flags, Class.isArray() can check whether component mirror is non-null and Class.isPrimitive() needs a new final transient boolean in java.lang.Class that the JVM code initializes.
>> Tested with tier1-4 and performance tests.
>
> src/hotspot/share/classfile/javaClasses.inline.hpp line 301:
> 
>> 299: #ifdef ASSERT
>> 300:   // The heapwalker walks through Classes that have had their Klass pointers removed, so can't assert this.
>> 301:   // assert(is_primitive == java_class->bool_field(_is_primitive_offset), "must match what we told Java");
> 
> I don't understand this comment about the heapwalker.  It sounds like we could have `is_primitive` set to true incorrectly.  If so, what prevents the asserts below from failing?  And why not use the value from _is_primitive_offset instead?

This is a good question.  The heapwalker walks through dead mirrors so I can't assert that a null klass field matches our boolean setting but I don't know why this never asserts (can't find any instances in the bug database) but it seems like it could.  I'll use the bool field in the mirror in the assert though but not in the return since the caller likely will fetch the klass pointer next.

> src/hotspot/share/prims/jvm.cpp line 2283:
> 
>> 2281: // Otherwise it returns its argument value which is the _the_class Klass*.
>> 2282: // Please, refer to the description in the jvmtiThreadState.hpp.
>> 2283: 
> 
> Does this "RedefineClasses support" comment still belong here?

I think so. The comment in jvmtiThreadState.hpp has details why this is.  We do a mirror switch before verification apparently because of bug 6214132 it says.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23572#discussion_r1961770573
PR Review Comment: https://git.openjdk.org/jdk/pull/23572#discussion_r1962059680