RFR: 8044107 Add Diagnostic Command to list all ClassLoaders
Staffan Larsen
staffan.larsen at oracle.com
Tue Jun 3 13:11:03 UTC 2014
Stefan, thanks for the comments. See below.
On 3 jun 2014, at 11:22, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> Hi Staffan,
>
> Thanks for implementing this feature!
>
> On 2014-05-28 16:50, Staffan Larsen wrote:
>> This change adds a new Diagnostic Command to list all ClassLoaders and some statistics for each classloader. The command is called “GC.classloader_stats” and the information listed is:
>>
>> * An id for the ClassLoader. This is the pointer to the Klass of the ClassLoader. The reason for using the Klass* (instead of the oop) is that it is stable across invocations.
>
> I agree that it's good to get a stable value, but IMHO the code would be easier if you just stored the oops for _classloader and _parent instead of the Klass*.
>
> 71 ClassLoaderData* _cld;
> 72 Klass* _classloader;
> 73 Klass* _parent;
>
>
> This will work since all this is done during a safe point.
The reason I choose Klass* was that I wanted something that was stable /across/ invocations, not just during one invocation of the Diagnostic Command. That way you can compare the output over time.
> As the code is written right now, it's awkward that we put Klass*s in fields named _parent and _classloader. We usually put oops and CLD*s in fields that are named that way.
I can change the names of the fields. Suggestions?
>
> The following code:
>
> 144 // It does not exist in our table - add it
> 145 ClassLoaderStats* cls = new ClassLoaderStats();
> 146 cls->_classloader = cl->klass();
> 147 oop parent = java_lang_ClassLoader::parent(cl);
> 148 if (parent != NULL) {
> 149 cls->_parent = parent->klass();
> 150 }
>
> could be condensed down to:
>
> 144 // It does not exist in our table - add it
> 145 ClassLoaderStats* cls = new ClassLoaderStats();
> 146 cls->_classloader = cl
> 147 cls->_parent = java_lang_ClassLoader::parent(cl);
>
> or even:
>
> 144 // It does not exist in our table - add it
> 145 ClassLoaderStats* cls = new ClassLoaderStats(cl, java_lang_ClassLoader::parent(cl));
>
>
> and
>
> 63 cls->_classloader = (cl == NULL ? NULL : cl->klass());
> 64 if (cl != NULL) {
> 65 oop parent = java_lang_ClassLoader::parent(cl);
> 66 if (parent != NULL) {
> 67 cls->_parent = parent->klass();
> 68 addEmptyParents(parent);
> 69 }
> 70 }
>
> would be come:
>
> 63 cls->_classloader = cl;
> 64 if (cl != NULL) {
> 65 cls->_parent = java_lang_ClassLoader::parent(cl);
> 68 addEmptyParents(cls->_parent);
> 70 }
>
>
>> * The id of the ClassLoader’s parent ClassLoader.
>> * The pointer to the ClassLoaderData structure in the VM. This can be useful for debugging.
>> * The number of classes loaded by the ClassLoader.
>> * The total size of all allocated metaspace chunks for the ClassLoader.
>> * The total size of all allocated metaspace blocks for the ClassLoader.
>>
>> If there are anonymous classes (invokedynamic classes) attributed to the ClassLoader, the following additional information is listed:
>> * The number of anonymous classes loaded by the ClassLoader.
>> * The total size of all allocated metaspace chunks for anonymous classes in the ClassLoader.
>> * The total size of all allocated metaspace blocks for anonymous classes in the ClassLoader.
>>
>> The information is gathered during a safe point to guarantee that the data structures are consistent.
>>
>> I have added a small test and have run this through jprt. A CCC request has been filed.
>>
>> webrev: http://cr.openjdk.java.net/~sla/8044107/webrev.00/
>> bug: https://bugs.openjdk.java.net/browse/JDK-8044107
>>
>> Example output:
>>
>> ClassLoader Parent CLData* Classes ChunkSz BlockSz Type
>
> We usually say ClassLoaderData or CLD in the code, but never CLData. Could this be changed to CLD* or CLD? I guess ClassLoaderData would be too long. Erik also had a suggestion on a more user-friendly name, I'm fine with that as well.
Changed it to CLD*.
>
>> 0x00000007c002d908 0x0000000000000000 0x0000000000000000 0 0 0 sun.misc.Launcher$ExtClassLoader
>> 0x0000000000000000 0x0000000000000000 0x00007fb239c08de0 761 4694016 4241312 <boot classloader>
>> 37 75776 50928 + invokedynamic classes
>> 0x00000007c0061028 0x00000007c0036878 0x00007fb239c2de60 1 6144 1976 ClassLoaderStatsTest$DummyClassLoader
>> 1 2048 1288 + invokedynamic classes
>> 0x00000007c0036878 0x00000007c002d908 0x00007fb239e10fc0 8 88064 31544 sun.misc.Launcher$AppClassLoader
>> Total = 4 808 4866048 4327048
>> ChunkSz: Total size of all allocated metaspace chunks
>> BlockSz: Total size of all allocated metaspace blocks (each chunk has several blocks)
>
> http://cr.openjdk.java.net/~sla/8044107/webrev.02/src/share/vm/classfile/classLoaderStats.hpp.html
>
> Is there a reason why these are intx instead of uintx?
>
> 113 intx _total_loaders;
> 114 intx _total_classes;
Changed to uintx.
>
>
> Would you mind changing the name of _classloader to _class_loader?
>
> 69 class ClassLoaderStats : public ResourceObj {
> 70 public:
> 71 ClassLoaderData* _cld;
> 72 Klass* _classloader;
Fixed.
>
>
>
> http://cr.openjdk.java.net/~sla/8044107/webrev.02/src/share/vm/classfile/classLoaderStats.cpp.html
>
> There's a couple of usages of SSIZE_FORMAT_W where I think you should be using SIZE_FORMAT_W
Fixed.
>
>
> At some point it would be nice to have an option to be able to list the usages of all individual anonymous class loaders.
>
>
> http://cr.openjdk.java.net/~sla/8044107/webrev.02/test/serviceability/dcmd/ClassLoaderStatsTest.java.html
>
> I don't know if this is a common style in our testing, but I often read test code without the aid of an IDE so import statements with *s make it harder for me to understand where imported classes come from:
>
> 31 import java.nio.*;
> 32 import java.nio.channels.*;
> 33 import java.io.*;
> 34 import java.util.regex.Matcher;
> 35 import java.util.regex.Pattern;
I agree that this is sloppy. Fixed.
updated webrev is here: http://cr.openjdk.java.net/~sla/8044107/webrev.03/
Thanks,
/Staffan
>
>
> thanks,
> StefanK
>
>>
>>
>> Thanks,
>> /Staffan
More information about the hotspot-dev
mailing list