RFR(s): 8203682: Add jcmd "VM.classloaders" command to print out class loader hierarchy, details

[David Holmes]

Hi Thomas,

On 28/05/2018 3:59 PM, Thomas Stüfe wrote:
> Hi David,
> 
> On Mon, May 28, 2018 at 7:23 AM, David Holmes <david.holmes at oracle.com> wrote:
>> Hi Thomas,
>>
>> I had a look at this and overall seems okay - the output looks good
> 
> thanks!
> 
>> (though I'm not sure how useful the hex values are?).
> 
> That is mainly a developer option for us; CLD* and Klass* are useful
> if one wishes to dig into core files; Loader oop too (I know that one
> is volatile, which is were the jcmd-bundle-commands-at-safepoint idea
> comes from.
> 
> But yes, this is normally too much noise, therefore disabled by
> default. You need to set "verbose" explicitly to see this.

Ok.

>>
>> Can't comment too much on the pretty-printing details - the proof is in the
>> output there. (Though have to wonder whether there is any existing
>> tree/graph printing logic somewhere in the OpenJDK code?)
> 
> None as good as mine :) Seriously, there is similar but not as evolved
> printing for class hierarchy. But it does not really print a tree,
> just a bunch of '|' dividers.
> 
> If this patch gets in, I would in a follow up patch unify tree
> printing for these two commands and any other tree-ish structures I
> find.

Sounds good.

>>
>> Two queries:
>>
>> 1. Have we previously established whether a CSR request is needed for a new
>> Dcmd? (My initial feeling is that it is.)
> 
> My feeling is no, since this adds a new command, so there can be no
> backward compat issues. What is the general policy for new jcmd
> commands, or for that matter anything new added to the outside facing
> interface (new options, new Xlog tracing flags, changed output for
> existing options)? Do these things require CSR?

Generally yes.

https://wiki.openjdk.java.net/display/csr/CSR+FAQs

Q: What sort of changes require CSR review?
A: Any change to a JDK interface meant to be used outside of the JDK 
itself requires CSR review. In this context "interface" isn't limited to 
the Java programing language definition of an interface, but encompasses 
the broader concept of a protocol between the JDK and users of the JDK. 
Examples of interfaces by this definition include:

     Changes to public exported APIs in java.* and javax.* packages.
     Changes to public and exported APIs in jdk.*packages.
     New language updates to the Java Programming Language
     New structures in the Java Virtual Machine Specification
     Adding or removing a command in $JDK/bin
     Adding, removing, or changing a command line option
     Using or defining an environment variable
     Using or defining a new file format or wire format
     Changing or defining a new system or security property

Interfaces that are experimental or for diagnostic purposes do not need 
to go through CSR process, but the CSR process may be employed if 
feedback from the CSR reviewers is desired.
---

IIRC (and I was hoping you may have recalled this :) ) last time this 
was raised it was stated that as jcmd provides diagnostic commands that 
adding or changing them doesn't need to go through the CSR process.

Unfortunately I also found that such commands are documented:

https://docs.oracle.com/javase/10/tools/jcmd.htm#GUID-59153599-875E-447D-8D98-0078A5778F05__DIAGNOSTICCOMMANDSFORJCMD-043BDB32

which may have an impact as it may mean we have to do something special 
to get the documentation updated. :( Not sure.

Anyway I think it is okay to proceed without a CSR request at this time, 
and I/we can check on the documentation issue.

> My problem with CSR is that it introduces a bottleneck, since it can
> only be approved by three very busy people at Oracle - if I understand
> the process right. Yes, we need a process to agree e.g. on syntax -
> desperately so, since e.g. sub option syntax in jcmd is a mess - but
> we seem to be strapped for reviewers even for normal code reviews, so
> the effect of creating a CSR in my experience is just a stop-of-work.

First note that a CSR request reviewer does not need to an OpenJDK 
Reviewer - they simply need to be a competent engineer with an OpenJDK 
username who basically sanity checks the CSR request to make sure it 
meets the expected requirements as per the CSR documentation.

Second, yes Joe tends to be the final approver, unless he delegates to 
someone else when he is away.

The expectation is that the need for a CSR request is established as one 
of the first tasks when working on something, so that the request can be 
put in and processed well ahead of the time you're ready to push. Again 
as per CSR docs.

But please raise any issues you have with the process with the CSR group 
- as that is why it was created. mailto:csr-discuss at openjdk.java.net

>>
>> 2. Is ClassLoaderHierarchyVMOperation a safepoint VM-op? I would expect it
>> needs to be to be able to walk the CLD hierarchy, unless that is already
>> guaranteed to be safely walkable. Either way a comment clearly stating that
>> would be useful I think.
> 
> According to Coleen, CLDG can be walked outside a safepoint, but I did
> not want to risk it so I made it a safepoint operation (like other
> commands walking the CLDG, e.g. VM.metaspace).

Okay. Can you document that please.
>>
>> Related to #2, is it really possible to encounter a CLD in the process of
>> being unloaded? Wouldn't that happen at a safepoint?
> 
> Not sure, I am not a GC expert. I see places where this may be called
> concurrently, e.g. in the process of
> -XX:+ClassUnloadingWithConcurrentMark?
> 
> Since a diagnostic command should never endanger the VM it monitors, I
> coded defensively.

Okay. Someone else may be able to clarify whether it is indeed possible 
or not, but defensive is fine by me.

Thanks,
David

>>
>> Thanks,
>> David
>>
> 
> Thank you,
> 
> Thomas
> 
>>
>> On 28/05/2018 2:50 PM, Thomas Stüfe wrote:
>>>
>>> All tests passed on jdk-submit.
>>>
>>> Anyone interested in a review?
>>>
>>> More output examples for jcmd VM.classloaders :
>>>
>>> Spring framework, basic tree:
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/example_spring_short.txt
>>>
>>> Spring framework, including all classes:
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/example_spring_long.txt
>>>
>>> ... Thomas
>>>
>>> On Wed, May 23, 2018 at 2:46 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
>>> wrote:
>>>>
>>>> Dear all,
>>>>
>>>> (not sure if this would be a serviceability or runtime rfe, so sorry
>>>> for crossposting)
>>>>
>>>> may I please have feedback/reviews for this small enhancement.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8203682
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/webrev.00/webrev/
>>>>
>>>> This adds a new command to jcmd, "VM.classloaders". It complements the
>>>> existing command "VM.classloader_stats".
>>>>
>>>> This command, in its simplest form, prints the class loader tree. In
>>>> addition to that, it optionally prints out loaded classes (both
>>>> non-anonymous and anonymous) and various classloader specific
>>>> information.
>>>>
>>>> Examples:
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/example.txt
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/example-with-classes.txt
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/example-with-reflection-and-noinflation.txt
>>>>
>>>>
>>>> Thanks and Best Regards,
>>>>
>>>> Thomas