On 1/9/15 4:38 AM, Staffan Larsen
wrote:
It�s getting difficult to get all the information into the same
output: hierarchy, interfaces, class loaders and modules. I took a
stab at it and it could look like this:
java.lang.Object
|--java.io.Serializable
(java.base, 0x00000007c00375f8, iface)
|--java.util.RandomAccess
(java.base, 0x00000007c00375f8, iface)
|--java.lang.Iterable
(java.base, 0x00000007c00375f8, iface)
|
|--java.util.Collection (java.base, 0x00000007c00375f8,
iface)
| |
|--java.util.List (java.base, 0x00000007c00375f8, iface)
|--java.util.AbstractCollection
(java.base, 0x00000007c00375f8)
| | implements
java.util.Collection
| | implements java.lang.Iterable
|
|--java.util.AbstractList (java.base, 0x00000007c00375f8)
| | implements java.util.List
| |
|--java.util.Arrays$ArrayList (java.base,
0x00000007c00375f8)
| | | implements java.io.Serializable
| | | implements java.util.RandomAccess
For the most part I like this. The one part I'm
not too fond of is hex long which I assume represents the
ClassLoader. Maybe we could just use a simple index, and at the
end of the dump include a table of the referenced ClassLoaders.
Possibly just hijack what VM.classloader_stats outputs, but
include the simple index in the first column.
But this is pushing what can be visualized in one
place.
We will need to add module and class loader
information somehow to all the Diagnostic Commands that list
classes. In addition we will need a way to see how modules
relate to one another. Perhaps it isn�t possible to have all
the information in one place, but instead make it possible to
cross reference between different diagnostic commands. For
example, GC.class_stats could have the module and class loader
information, and GC.class_hierarchy would not have to include
it. What is missing from making that possible is a unique way
of identifying a class (since the name is not unique). All
output would need to include that unique identifier and it
would be possible to cross reference. The identifier has to be
stable during a JVM run, but not between runs.
I don't know if there is some sort of simple stable ID for a class
other than the address of the Klass instance. And of course this
assumes there isn't any class unloading going on that could result
in reuse of the address. Is that acceptable?
The above would then become:
java.lang.Object/0x12345600
|--java.io.Serializable/0x12345601
|--java.util.RandomAccess/0x12345602
|--java.lang.Iterable/0x12345603
|
|--java.util.Collection/0x12345604
| |
|--java.util.List/0x12345605
|--java.util.AbstractCollection/0x12345606
| | implements
java.util.Collection/0x12345604
| | implements java.lang.Iterable/0x12345603
|
|--java.util.AbstractList/0x12345607
| | implements java.util.List/0x12345605
| |
|--java.util.Arrays$ArrayList/0x12345608
| | | implements java.io.Serializable/0x12345601
| | | implements java.util.RandomAccess/0x12345602
With additions to GC.class_stats:
Index Super
ClassLoader ClassName
1 -1
0x00000007c0034c48 java.lang.Object/0x12345600
2 1
0x00000007c0034c48 java.util.List/0x12345605
3
31 0x00000007c0034c48 java.util.AbstractList/0x12345607
num
#instances #bytes class name
----------------------------------------------
1:
945 117736 java.lang.Object/0x12345600
2:
442 50352 java.util.List/0x12345605
3:
499 25288 java.util.AbstractList/0x12345607
Would it be better for the class identifier to be in a separate
column from the class name, or is combining them intentional? In any
case, it's easy enough to add to all of the above assuming we are
talking about an existing value such as the address of the Klass
instance.
Chris
/Staffan
On 01/08/2015 10:29 PM, Chris Plummer wrote:
Hi Karen,
Comments inline.
On 1/8/15 8:07 AM, Karen Kinnear wrote:
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.
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.
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?
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.
It seems to me that we have two very
different use cases here, each one
best served with a different output format:
1 - Listing of all classes/interfaces
hierarchy when the dcmd is
invoked without arguments:
-> Chris' output format as
described below (with interfaces)
2 - Investigation on a particular class or
interface when a class
or interface is passed in argument to the
dcmd
-> Karen's output format, much
easier to work with to
track default methods. Because the
output is limited to the
hierarchy from a single class,
there's no class duplication
in output (single parent class
inheritance) and limited
interfaces duplication.
If the implementations of the two features
are too different, we could
consider having two different dcmds.
My 2 cents,
Fred
The could
just be a simple list right after the class that
implements them:
java.lang.Object
| ...
|--MyBaseClass
| | implements -> MyInterface1
| | implements -> MyInterface2
| |--MySubClass
| implements -> MyInterface1
| implements -> MyInterface2
| ...
|--MyInterface1
|--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,
Karen
Thanks for the input.
Chris
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
--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: frederic.par...@oracle.com
|