RFR (T) 8242898: Clean up InstanceKlass::_array_klasses

[Ioi Lam]

On 4/29/20 3:38 PM, Ioi Lam wrote:
>
>
> On 4/29/20 2:41 PM, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 4/29/20 12:15 PM, Ioi Lam wrote:
>>> Hi Coleen,
>>>
>>> This looks good, but should we also rename array_klasses to 
>>> array_klass, since in most cases we are talking about one klass 
>>> instead of many klasses?
>>
>> It is many classes because they're all linked through _array_klasses 
>> through the higher_dimension(), so it's essentially more than one.
>>
>
> But this plural usage is inconsistent with other places we use link 
> lists. E.g., klass.hpp
>
>   // First subclass (NULL if none); _subklass->next_sibling() is next one
>   Klass* volatile _subklass;
>   // Sibling link (or NULL); links all subklasses of a klass
>   Klass* volatile _next_sibling;
>   // All klasses loaded by a class loader are chained through these links
>   Klass*      _next_link;
>
> and we don't use _supers even if you can use _super to walk all super 
> classes.
>
>   Klass*      _super;
>
>
Also, when you walk "up" the dimensions, it's

ik->array_klasses()->array_klasses()->array_klasses()->....

but to walk "down", it's

nth_dim_obj_array_klass->element_klass()->element_klass()->element_klass()->...

so it's inconsistent even when you are walking the same linked list.
> Thanks
> - Ioi
>
>
>> So I'd rather leave the name as is.
>> Thanks,
>> Coleen
>>>
>>> The plural makes sense only in places like this:
>>>
>>>
>>> 1625 void InstanceKlass::array_klasses_do(void f(Klass* k, TRAPS), 
>>> TRAPS) {
>>> 1626   if (array_klass() != NULL)
>>> 1627     array_klass()->array_klasses_do(f, THREAD);
>>> 1628 }
>>>
>>> Thanks
>>> - Ioi
>>>
>>>
>>> On 4/29/20 6:06 AM, coleen.phillimore at oracle.com wrote:
>>>>
>>>>
>>>> On 4/29/20 8:44 AM, David Holmes wrote:
>>>>> Hi Coleen,
>>>>>
>>>>> On 29/04/2020 10:11 pm, coleen.phillimore at oracle.com wrote:
>>>>>> Summary: Use more specific ObjArrayKlass type.
>>>>>>
>>>>>> This avoids some casts.  Tested with tier1-3.
>>>>>>
>>>>>> open webrev at 
>>>>>> http://cr.openjdk.java.net/~coleenp/2020/8242898.01/webrev
>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8242898
>>>>>
>>>>> This looks fine to me.
>>>>>
>>>>> I have to wonder why this was not defined this way all along?
>>>>
>>>> It was likely klassOop and stayed that way until Ioi noticed.
>>>> Thanks,
>>>> Coleen
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> thanks,
>>>>>> Coleen
>>>>
>>>
>>
>