Hi Serguei, Thank you for looking at this change.
On 8/24/18 1:25 PM, serguei.spit...@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.phillim...@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
