RFR(M) 8208999 Some use of Klass* should be replaced by InstanceKlass*

[coleen.phillimore at oracle.com]

http://cr.openjdk.java.net/~iklam/jdk12/8208999_klass_should_be_instanceklass.v01/src/hotspot/share/classfile/systemDictionaryShared.cpp.udiff.html

1007 return entry != NULL ? entry->instance_klass() : (InstanceKlass*)NULL;

do you need this cast here?

http://cr.openjdk.java.net/~iklam/jdk12/8208999_klass_should_be_instanceklass.v01/src/hotspot/share/memory/metadataFactory.hpp.udiff.html

+ static void free_array(ClassLoaderData* loader_data, const Array<T>* 
data) {


But we are modifying the contents of the data (?).  This is for deleting 
secondary_supers?   I think maybe adding the const for them might be 
overkill.

http://cr.openjdk.java.net/~iklam/jdk12/8208999_klass_should_be_instanceklass.v01/src/hotspot/share/oops/arrayKlass.cpp.udiff.html

+ set_super(Universe::is_bootstrapping() ? (InstanceKlass*)NULL : 
SystemDictionary::Object_klass());


Do you need this cast?  Object_klass() returns InstanceKlass - all of 
these do.

http://cr.openjdk.java.net/~iklam/jdk12/8208999_klass_should_be_instanceklass.v01/src/hotspot/share/oops/klass.hpp.udiff.html

- Array<Klass*>* _secondary_supers;
+ const Array<Klass*>* _secondary_supers;

I think the improvement outweighs this messy part of this.  This looks 
like a difficult thing to change.

This looks really good. Thank you for doing this!

Coleen



On 8/6/18 5:26 PM, Ioi Lam wrote:
> Hi, please review this clean up
>
> https://bugs.openjdk.java.net/browse/JDK-8208999
> http://cr.openjdk.java.net/~iklam/jdk12/8208999_klass_should_be_instanceklass.v01/ 
>
>
> I changed the following APIs so that we can avoid many unnecessary 
> type casts
> from Klass* -> InstanceKlass*.
>
>     Klass::java_super()                    -> now returns InstanceKlass*
>     InstanceKlass::local_interfaces()      -> Array<InstanceKlass*>*
>     InstanceKlass::transitive_interfaces() -> Array<InstanceKlass*>*
>
> I also modified APIs in SystemSictionary and SystemSictionary to 
> return InstanceKlass*
> where possible.
>
> Most changes are mundane, but here are some notable changes:
>
> 1. To save space, Klass::_secondary_supers (an Array<Klass*>*) may be 
> pointing
>    to the same array as InstanceKlass::_transitive_interfaces (an
>    Array<InstanceKlass*>*).
>
>    To make this happen, I added Array::as_const_array_of().
>    See InstanceKlass::compute_secondary_supers() and array.h.
>
>    I've added enough 'const' and asserts to catch mis-uses of this type
>    of casting. If someone has a better suggestion, please let me know.
>
> 2. I changed Klass::_secondary_supers to 'const Array<Klass*>*' because
>    of #1.
>
>    There are many pointers returned by Klass/InstanceKlass that should be
>    'const', as most callers should read them only without modifying, 
> but that's
>    outside the scope of this RFE.
>
> 3. SystemDictionary::resolve_super_or_fail() currently accepts the format
>    of "Ljava/lang/Object;" for the "super_name" parameter. I am 
> keeping the
>    same behavior in my patch. However, I wonder if this actually correct
>    per JVMLS. I filed JDK-8209030.
>
>    See the call to resolve_instance_class_or_null_helper from 
> resolve_super_or_fail.
>
>
> Testing: hs tiers 1/2/3/4
>
> Thanks
> - Ioi