Chris,

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.

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.

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.

e.g.
Class java.util.Arrays$ArrayList requires default method processing
java/util/Arrays$ArrayList
  java/util/AbstractList
    java/util/AbstractCollection
      java/lang/Object
      java/util/Collection
        java/lang/Object
        java/lang/Iterable
          java/lang/Object
    java/util/List
      java/lang/Object
      java/util/Collection
        java/lang/Object
        java/lang/Iterable
          java/lang/Object
  java/util/RandomAccess
    java/lang/Object
  java/io/Serializable
    java/lang/Object

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? 

Is there value in printing the defining class loader for each class - maybe 
optionally?
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?
Love opinions on that  - especially from the serviceability folks 

thanks,
Karen


On Jan 7, 2015, at 6:29 PM, Chris Plummer wrote:

> Hi,
> 
> 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.
> 
> thanks,
> 
> Chris

Reply via email to