RFR 8209821: Make JVMTI GetClassLoaderClasses not walk CLDG

[coleen.phillimore at oracle.com]

Hi Serguei,  Thank you for looking at this change.

On 8/24/18 1:25 PM, serguei.spitsyn at oracle.com wrote:
> Hi Coleen,
>
> It looks like a nice simplification and cleanup.
> Thank you for taking care about it!
>
> Could you, please, explain a little bit the changes in the 
> getClassLoaderClasses?
>
> If I understand it correctly, the iteration
> ClassLoaderDataGraph::dictionary_all_entries_do(&JvmtiGetLoadedClassesClosure::increment_with_loader)
> is replaced with the data->dictionary()->all_entries_do(&closure), 
> orfor boot class loader, with
> ClassLoaderData::the_null_class_loader_data()->dictionary()->all_entries_do(&closure).

The version of ClassLoaderDataGraph::dictionary_all_entries do iterated 
through all the CLDG, and for each class found, would match whether the 
dictionary was the same as the initiatingLoader parameter.   Since the 
one dictionary per class loader change, all the classes in the 
initiatingLoader's ClassLoaderData's dictionary will match.  For the 
NULL class loader, we need to walk the_null_class_loader_datas's dictionary.
>
> It is not clear (I have just some guesses) if it would do the same and 
> why does it support concurrent class unloading for ZGC.
> Is it worth to add a minimal comment explaining this?

Yes, it does the same thing.  It helps with concurrent class unloading 
because we might be eventually removing a ClassLoaderData from the graph 
while this code was walking it (ie we'd have to add a lock).  But it was 
walking it unnecessarily.

How about a new comment like:

     // All classes loaded from this loader as initiating loader are
     // requested, so only need to walk this loader's ClassLoaderData
     // dictionary, or the NULL ClassLoaderData dictionary for bootstrap 
loader.
     if (loader != NULL) {
       ClassLoaderData* data = java_lang_ClassLoader::loader_data(loader);
...

Thanks,
Coleen

>
> Thanks,
> Serguei
>
>
> On 8/23/18 05:37, 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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180824/528e34bb/attachment.html>