RFR: 8044107 Add Diagnostic Command to list all ClassLoaders

[Stefan Karlsson]

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 safepoint.

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.

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.

> 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;


Would you mind changing the name of _classloader to _class_loader?

   69 class ClassLoaderStats : public ResourceObj {
   70 public:
   71   ClassLoaderData*  _cld;
   72   Klass*            _classloader;



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


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;


thanks,
StefanK

>
>
> Thanks,
> /Staffan