RFR(M) 8208999 Some use of Klass* should be replaced by InstanceKlass*
Ioi Lam
ioi.lam at oracle.com
Tue Aug 7 04:49:27 UTC 2018
On 8/6/18 3:40 PM, David Holmes wrote:
> Hi Ioi,
>
> I took a quick look at this but it is not a review. This seems to
> highlight further problems in the arrangement of types and methods to
> me. For example in klass.cpp we have:
>
> bool Klass::verify_itable_index(int i) {
> assert(is_instance_klass(), "");
> ! int method_count =
> klassItable::method_count_for_interface(InstanceKlass::cast(this));
> assert(i >= 0 && i < method_count, "index out of bounds");
> return true;
> }
>
> but if "this" must be an instanceKlass then verify_itable_index should
> not be a method in klass but a method in instanceKlass.
>
Hi David,
Thanks for spotting this. I've move this function to InstanceKlass.
Updated webrev:
http://cr.openjdk.java.net/~iklam/jdk12/8208999_klass_should_be_instanceklass.v02/
Thanks
- Ioi
> Cheers,
> David
>
> On 7/08/2018 7:26 AM, 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
More information about the hotspot-runtime-dev
mailing list