RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy
chris.plummer at oracle.com
Thu Jan 8 21:29:26 UTC 2015
On 1/8/15 8:07 AM, Karen Kinnear wrote:
> Thank you for doing this. I had a couple of questions/comments.
> I like your idea of being able to start with a specific class to show all subclasses of.
Ok. I'll add that.
> I think the way I read the code this shows the superclass hierarchy, not the superinterfaces. With the addition
> of default methods in interfaces, I think we have increased the value in seeing superinterfaces.
It does include interfaces in the output, but not as part of any class
hierarchy. Interfaces are just shown as regular classes that inherit
from Object. This is the case if one interface extends another, I
suppose because "extends" is just interpreted as "implements" in this case.
> So what I personally would find useful would be to be able to start with a specific class and
> find the superclasses and superinterfaces of that class - for the debugging I do, I usually am
> trying to look up and need both sets of information. Today if you run -XX:+TraceDefaultMethods
> there is one sample way to display all the supertypes of a single type, all the way up. I don't know the
> best way to make that consistent with the current output approach, e.g. using the |- syntax.
> Class java.util.Arrays$ArrayList requires default method processing
> Do you think there could be value to others in the ability to walk up the hierarchy as well as to
> see superclasses and superinterfaces at least from that perspective?
This is a inverted from how my dcmd prints the hierarchy, plus also
includes interfaces. Inverting the hierarchy means a class is listed
with every subclass of the class, which I don't think is desirable.
Including interfaces has the same issue, but introduces a new issue even
when not inverting the hierarchy. The same interface can be in more than
one location in the hierarchy, so there is no avoiding printing it more
than once, so we need to decide how to best include interfaces in the
output. The could just be a simple list right after the class that
| | implements -> MyInterface1
| | implements -> MyInterface2
| implements -> MyInterface1
| implements -> MyInterface2
The "implements" lines could be optional.
I know cvm would distinguish between interfaces the Class declared it
implemented, and those it inherited from the interfaces it declared it
implemented. This was necessary for reflection, and I think also to
properly build up interfaces tables. I assume hotspot does something
similar. Is there any need for this information to be conveyed in the
above output, or just list out every interface implemented, and not
worry about how the class acquired it.
> Is there value in printing the defining class loader for each class - maybe optionally?
This is already available with GC.class_stats, although not in the
default output. I suppose the reality is that it might be better handled
by this DCMD. The main puprose of GC.class_stats is to print statistics
(counts and sizes), so printing the ClassLoader name there is probably
not appropriate, but then it's not really appropriate for
VM.class_hierarchy either. I'm not sure how best to handle this. One or
both DCMDs possibly should be re-purposed and more clearly define what
type of data it displays.
> If so, I'm wondering if there might be value in future for the jigsaw project adding the module for each class - maybe optionally as well?
This relates to my above statement. We need to figure out what type of
data is useful, and which dcmds should handle them.
> Love opinions on that - especially from the serviceability folks
Thanks for the input.
> On Jan 7, 2015, at 6:29 PM, Chris Plummer wrote:
>> Please review the following changes for the addition of the VM.class_hierarchy DCMD. Please read the bug first for some background information.
>> Webrev: http://cr.openjdk.java.net/~cjplummer/8054888/webrev.00/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8054888
>> I expect there will be further restructuring or additional feature work. More discussion on that below. I'm not sure if that additional work will be done later with a separately bug filed or with this initial commit. That's one thing I want to work out with this review.
>> Currently the bulk of the DCMD is implemented in heapInspection.cpp. The main purpose of this file is to implement the GC.class_stats and GC.class_histogram DCMDs. Both of them require walking the java heap to count live objects of each type, thus the name "heapInspection.cpp". This new VM.class_hierarchy DCMD does not require walking the heap, but is implemented in this file because it leverages the existing KlassInfoTable and related classes (KlassInfoEntry, KlassInfoBucket, and KlassClosure).
>> KlassInfoTable makes it easy to build a database of all loaded classes, save additional info gathered for each class, iterate over them quickly, and also do quick lookups. This exactly what I needed for this DCMD, thus the reuse. There is some downside to this. For starters, heapInspection.cpp is not the proper place for a DCMD that has nothing to do with heap inspection. Also, KlassInfoEntry is being overloaded now to support 3 different DCMDs, as is KlassInfoTable. As a result each has a few fields and methods that are not used for all 3 DCMDs. Some subclassing might be in order here, but I'm not sure if it's worth it. Opinions welcomed. If I am going to refactor, I would prefer that be done as a next step so I'm not disturbing the existing DCMDs with this first implementation.
>> I added some comments to code only used for GC.class_stats and GC.class_histogram. I did this when trying to figure them out so I could better understand how to implement VM.class_hierarchy. I can take them out if you think they are not appropriate for this commit.
>> One other item I like to discuss is whether it is worth adding a class name argument to this DCMD. That would cause just the superclasses and subclasses of the named class to be printed. If you think that is useful, I think it can be added without too much trouble.
>> At the moment not much testing has been done other than running the DCMD and looking at the output. I'll do more once it's clear the code has "settled". I would like to know if there are any existing tests for GC.class_stats and GC.class_histogram (there are none in the "test" directory). If so, possibly one could serve as the basis for a new test for VM.class_hierarchy.
More information about the hotspot-runtime-dev