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

[Ioi Lam]

On 4/29/20 4:40 PM, coleen.phillimore at oracle.com wrote:
>
>
> On 4/29/20 6:43 PM, Ioi Lam wrote:
>>
>>
>> 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;
>>>
>
> I see your point but it seems odd to me to call it a single 
> array_klass.  Sure.  I'll change it and make sure it builds before I 
> push.
>
After discussing with Coleen off-line, I am not insistent on the 
renaming anymore. I am OK with the current patch as is.

Thanks
- Ioi

> Coleen
>
>>> 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
>>>>>>
>>>>>
>>>>
>>>
>>
>