RFR 8209821: Make JVMTI GetClassLoaderClasses not walk CLDG

[coleen.phillimore at oracle.com]

On 8/24/18 3:27 PM, serguei.spitsyn at oracle.com wrote:
> Hi Coleen,
>
> Thank you for the explanations!
> The fix (and update with the comment) looks good to me.

Thank you Serguei!
Coleen

>
> Thanks,
> Serguei
>
>
> On 8/24/18 12:11, coleen.phillimore at oracle.com wrote:
>>
>> 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/de1e1de2/attachment-0001.html>