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

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Wed Apr 29 23:40:08 UTC 2020



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.

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



More information about the hotspot-runtime-dev mailing list