Hi Coleen,
Thank you for the explanations!
The fix (and update with the comment) looks good to me.
Thanks,
Serguei
On 8/24/18 12:11, coleen.phillim...@oracle.com wrote:
Hi Serguei, Thank you for looking at this change.
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),
or for 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 =
"">
...
Thanks,
Coleen
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
|
