Hi Thomas,
I forgot to tell that I did not want another webrev as it is a minor update.
Please, ship it.
Thanks,
Serguei
On 6/23/18 00:13, Thomas Stüfe wrote:
Hi Serguei,
On Fri, Jun 22, 2018 at 11:55 PM, serguei.spit...@oracle.com
<serguei.spit...@oracle.com> wrote:
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".
Makes all sense. Update:
Delta:
http://cr.openjdk.java.net/~stuefe/webrevs/8205531-vm.classloader-tree-folding/webrev.00-to-01/webrev/
Full:
http://cr.openjdk.java.net/~stuefe/webrevs/8205531-vm.classloader-tree-folding/webrev.01/webrev/
I only changed the comments as suggested.
Tests (jdk-submit) came back fine.
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. :)
:) I really must have been tired yesterday. But I will not change the
subject again, to not tear up the thread.
Thanks, Thomas
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
