On 8/24/18 3:27 PM, serguei.spit...@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.phillim...@oracle.com wrote:
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
