RFR 8209821: Make JVMTI GetClassLoaderClasses not walk CLDG

[Lois Foltan]

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.

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
>