RFR 8209821: Make JVMTI GetClassLoaderClasses not walk CLDG

[coleen.phillimore at oracle.com]

On 8/24/18 3:31 PM, Lois Foltan wrote:
> On 8/24/2018 2:59 PM, coleen.phillimore at oracle.com wrote:
>
>>
>>
>> On 8/24/18 11:44 AM, Lois Foltan wrote:
>>> On 8/23/2018 8:37 AM, coleen.phillimore at oracle.com wrote:
>>>
>>>> Summary: And also added function with KlassClosure to remove the 
>>>> hacks.
>>>>
>>>> There are about 10 vmTestbase/nsk/jvmti tests that test various 
>>>> parts of this change.  Also ran mach5 tier1-7.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8209821.01/webrev
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8209821
>>>>
>>>> Thanks,
>>>> Coleen
>>>
>>> Hi Coleen,
>>>
>>> I think this is a good clean up.  Couple of comments.
>>>
>>> - memory/universe.cpp
>>> You could make basic_type_classes_do() be a for loop
>>>     for (int i = 0; i < T_VOID+1; i++) {
>>>       closure->do_klass(typeArrayKlassObjs[i]());
>>>   }
>>
>> Interesting observation.  This is equivalent except T_OBJECT and 
>> T_ARRAY elements aren't initiatialized.  I believe that the do_klass 
>> in the closure I am passing will choke on NULL.  I've never 
>> understood why these statics needed to be duplicated like this, and 
>> have tried to clean this up before.  Maybe an RFE to do so would be 
>> better.
> Hi Coleen,
>
> Then just have the for loop be instead  for (int i = T_BOOLEAN; i < 
> T_LONG+1; i++).  I'm okay with what you decide.  However, to me the 
> code in Universe::metaspace_pointers_do() at line 230-232 looks wrong:
>
> for (int i = 0; i < T_VOID+1; i++) {
>     it->push(&_typeArrayKlassObjs[i]);
>   }
>
> The VM does not appear to store anything in _typeArrayKlassObjs for 
> elements 0-3, so hopefully those elements are NULL.  It should at 
> least start that for loop off at T_BOOLEAN.

Hm, you're right T_BOOLEAN starts at 4.  The closure will still crash on 
zero for _typeArrayKlassObj[T_ARRAY] and [T_OBJECT]. Actually 
_typeArrayKlassObj only goes to T_LONG, so I could change the loop to be 
from T_BOOLEAN to T_LONG inclusively (and rerunning tests).

http://cr.openjdk.java.net/~coleenp/8209821.02/webrev/src/hotspot/share/memory/universe.cpp.udiff.html

I opened this RFE to clean these up some more.

https://bugs.openjdk.java.net/browse/JDK-8209958

Thanks,
Coleen
>
> Thanks,
> Lois
>
>
>>>
>>> - prims/jvmtiGetLoadedClasses.cpp
>>> In JvmtiGetLoadedClasses::getClassLoaderClasses() you could pull the 
>>> call to basic_type_classes_do() from both sections of the if 
>>> statement to line #139
>>
>> Thanks, I'll fix it.
>>
>> Thanks for the code review.
>> Coleen
>>>
>>> Thanks,
>>> Lois
>>
>