Hi Thomas,
It looks good to me. A couple of minor comments below. http://cr.openjdk.java.net/~stuefe/webrevs/8205531-vm.classloader-tree-folding/webrev.00/webrev/src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp.frames.html 135 // we we add a preliminary empty LoaderTreeNode for it. This preliminary node
A typo: "we we" 151 // In default view, similar tree nodes (same loader class, same or no name)
I'd suggest a minor tweak: "same or no name"
=> "same name or no name" as at the
line: 291 // and they have the same name or no name (note: leaf check is done by caller).
Otherwise, it is a little bit confusing
because of the preceding "same loader
class".On 6/22/18 12:10, Thomas Stüfe wrote: Resent with the correct subject, sorry. Also, it is better to list a bug number in the email subject. :) Thanks, Serguei ..Thomas On Fri, Jun 22, 2018 at 9:08 PM, Thomas Stüfe <thomas.stu...@gmail.com> wrote:Hi all, may I have reviews for this small enhancement to the jcmd VM.classloader diagnostic command: https://bugs.openjdk.java.net/browse/JDK-8205531 http://cr.openjdk.java.net/~stuefe/webrevs/8205531-vm.classloader-tree-folding/webrev.00/webrev/ VM.classloaders prints a tree of class loaders. This tree can grow a lot and become unwieldy, especially with a lot of similar loaders. One prime example is the DelegatingClassLoader. It would be helpful were all these loaders: 13114: +-- <bootstrap> | +-- "platform", jdk.internal.loader.ClassLoaders$PlatformClassLoader | +-- "app", jdk.internal.loader.ClassLoaders$AppClassLoader | +-- test3.internals.InMemoryClassLoader | +-- jdk.internal.reflect.DelegatingClassLoader | +-- jdk.internal.reflect.DelegatingClassLoader | +-- jdk.internal.reflect.DelegatingClassLoader | +-- jdk.internal.reflect.DelegatingClassLoader | +-- jdk.internal.reflect.DelegatingClassLoader | ...... repeat 1495 times folded into one: 13114: +-- <bootstrap> | +-- "platform", jdk.internal.loader.ClassLoaders$PlatformClassLoader | +-- "app", jdk.internal.loader.ClassLoaders$AppClassLoader | +-- test3.internals.InMemoryClassLoader | +-- jdk.internal.reflect.DelegatingClassLoader (+ 1499 more) Original idea by Bernd Eckenfels, see http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-May/023824.html Thank you, Thomas |
- Re: RFR(s): jcmd VM.classloaders should fold si... Thomas Stüfe
- Re: RFR(s): jcmd VM.classloaders should fo... serguei.spit...@oracle.com
- Re: RFR(s): jcmd VM.classloaders shoul... Thomas Stüfe
- Re: RFR(s): jcmd VM.classloaders s... serguei.spit...@oracle.com
- Re: RFR(s): jcmd VM.classloaders should fo... David Holmes
- Re: RFR(s): jcmd VM.classloaders shoul... Thomas Stüfe
- Re: RFR(s): jcmd VM.classloaders s... David Holmes
- Re: RFR(s): jcmd VM.classloade... Thomas Stüfe