RFR(s): 8203682: Add jcmd "VM.classloaders" command to print out class loader hierarchy, details
Thomas Stüfe
thomas.stuefe at gmail.com
Mon Jun 4 18:47:18 UTC 2018
Hi Coleen,
thank you for your review!
Here the new webrevs:
Full: http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/webrev.01/webrev/
Delta: http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/webrev.00-to-01/webrev/
Please find comments inline.
On Fri, Jun 1, 2018 at 12:07 AM, <coleen.phillimore at oracle.com> wrote:
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/webrev.00/webrev/src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp.html
>
> Can you make BranchTracker : public StackObj {} ?
Done.
> Are you going to move
> BranchTracker to utilities if you find other uses for it?
>
That was my plan. One other use I see is when printing the class
hierarchy. I did not want to mix it in into this change, though, so I
earmarked it for a future cleanup.
> I don't like the use of mtInternal, but there doesn't seem to be a better
> category (or one to add).
The point is moot now since I followed your suggestion below, using
resource area instead.
>
> 152 if (_loader_oop != NULL) {
> 153 loader_klass = _loader_oop->klass();
> 154 if (loader_klass != NULL) {
> 155 loader_class_name = loader_klass->external_name();
> 156 }
> 157 oop nameOop = java_lang_ClassLoader::name(_loader_oop);
> 158 if (nameOop != NULL) {
> 159 const char* s = java_lang_String::as_utf8_string(nameOop);
> 160 if (s != NULL) {
> 161 loader_name = s;
> 162 }
> 163 }
>
> These are available to the ClassLoaderData (_cld) as class_loader_name() and
> class_loader_klass(). I would rather see ClassLoaderData::loader_name()
> used - once fixed. Also, I don't like <unnamed> in the output. It seems
> that most class loaders will be unnamed, so the output will be too noisy
> with that. Also, there's an unnamed module so that makes it very confusing.
Okay to all of that. I changed the coding to use
class_loader_{klass|name} consistently and only print out loader names
when set:
24942:
+-- <bootstrap>
|
+-- "platform", instance of
jdk.internal.loader.ClassLoaders$PlatformClassLoader
|
+-- "app", instance of
jdk.internal.loader.ClassLoaders$AppClassLoader
|
+--
test3.internals.InMemoryJavaFileManager$InMemoryClassLoader
See more examples:
http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/out2.txt
http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/out3.txt
>
> Since this is in a safepoint, maybe you can add a ResourceMark at 472,
> remove it from 362 and make all classes ResourceObj allocated. Then you can
> remove the code to delete the types.
>
I originally did not do it since I did not know whether
ClassLoaderDataGraph::cld_do() would use resource marks. But it does
not, and you are right, this makes the coding simpler.
> Can you add assert that you are at a safepoint at the beginning of this, so
> you don't have to worry about LoaderTreeNode::_loader_oop getting collected.
Done. (only in full webrev, sorry, I messed up the delta)
> It seems that by saving _cld in LoaderTreeNode, that you don't need
> _loader_oop.
Unfortunately not. To add a node to the tree, I need its parent node.
At that time I may not yet have encountered the parent loader CLD. So
I add the parent node, and mark it with the parent loader oop. Later,
when I encounter the CLD for the parent, I complete the node.
I added a comment to make that clearer.
>
> This code looks like it's walking the CLDG in order to make a different
> Class loader graph for printing. Is it necessary to show the hierarchy
> information?
>
It proved useful in the classic delegation scenarios.
> It seems like printing the address is better than <unnamed>, which is what
> I've looked at with the system dictionary logging.
>
Oh, I do that, when option "verbose" is given:
+-- <bootstrap>
|
| Loader Data: 0x00007fc458239040
| Loader Klass: 0x0000000000000000
| Loader Oop: 0x0000000000000000
|
+-- "platform", instance of
jdk.internal.loader.ClassLoaders$PlatformClassLoader
|
| Loader Data: 0x00007fc4583f7350
| Loader Klass: 0x00000008000107b0
| Loader Oop: 0x000000060d077b90
|
+-- "app", instance of
jdk.internal.loader.ClassLoaders$AppClassLoader
|
| Loader Data: 0x00007fc4583eb740
| Loader Klass: 0x0000000800010348
| Loader Oop: 0x000000060d079b30
|
I wondered whether I should print the loader oop more prominently, and
always. Should I? For example:
+-- {0x000000060d077b90} "platform", instance of
jdk.internal.loader.ClassLoaders$PlatformClassLoader
|
| Loader Data: 0x00007fc4583f7350
| Loader Klass: 0x00000008000107b0
|
+-- {0x000000060d079b30} "app", instance of
jdk.internal.loader.ClassLoaders$AppClassLoader
|
| Loader Data: 0x00007fc4583eb740
| Loader Klass: 0x0000000800010348
> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/webrev.00/webrev/test/hotspot/jtreg/serviceability/dcmd/vm/ClassLoaderHierarchyTest.java.html
>
> What is JMXExecutor? Is this something to get in the way of debugging this
> test if it fails?
I followed the trail left by the other diagnostic command tests in
test/hotspot/jtreg/serviceability/dcmd/vm. They all used the same
technique, so I did not want to reinvent the wheel. To me, it looks
harmless enough. The test itself also prints diagnostic output and
should be easy to reproduce on the command line if it fails.
>
> Sorry for the delay reviewing.
No problem, thank you for the review!
Best Regards, Thomas
> thanks,
> Coleen
>
>
>
> On 5/28/18 5:33 AM, Thomas Stüfe wrote:
>>
>> Hi Bernd,
>>
>> definitely. I have two local patches here and corresponding RFEs open:
>> https://bugs.openjdk.java.net/browse/JDK-8203343
>> "VM.{metaspace|classloaders} jcmd should show invocation targets for
>> Generated{Method|Constructor}AccessorImpl classes"
>> https://bugs.openjdk.java.net/browse/JDK-8203849
>> "DelegatingClassLoader name could be used to indicate reflected class"
>>
>> Both complement each other and should make it possible to understand
>> core reflection class loader issues better.
>>
>> Nice idea about that folding, I can add this.
>>
>> I plan to get them upstream for 11, but that may be illusory seeing
>> how slow reviews are currently going. But certainly 12.
>>
>> Gruss, Thomas
>>
>>
>> On Mon, May 28, 2018 at 9:21 AM, Bernd Eckenfels <ecki at zusammenkunft.net>
>> wrote:
>>>
>>> Anything which can be done with the DelegatingClassLoaders (either add
>>> Info on their target or collabs them to a „30 x unnamed
>>> DelegatingClassloader“. Even in Verbose mode they only reveal their type
>>> (constructor, method) but not much more.
>>>
>>> Gruss
>>> Bernd
>>> --
>>> http://bernd.eckenfels.net
>>> ________________________________
>>> From: serviceability-dev <serviceability-dev-bounces at openjdk.java.net> on
>>> behalf of Thomas Stüfe <thomas.stuefe at gmail.com>
>>> Sent: Monday, May 28, 2018 6:50:34 AM
>>> To: serviceability-dev at openjdk.java.net
>>> serviceability-dev at openjdk.java.net; Hotspot dev runtime
>>> Subject: Re: RFR(s): 8203682: Add jcmd "VM.classloaders" command to print
>>> out class loader hierarchy, details
>>>
>>> All tests passed on jdk-submit.
>>>
>>> Anyone interested in a review?
>>>
>>> More output examples for jcmd VM.classloaders :
>>>
>>> Spring framework, basic tree:
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/example_spring_short.txt
>>>
>>> Spring framework, including all classes:
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/example_spring_long.txt
>>>
>>> ... Thomas
>>>
>>> On Wed, May 23, 2018 at 2:46 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
>>> wrote:
>>>>
>>>> Dear all,
>>>>
>>>> (not sure if this would be a serviceability or runtime rfe, so sorry
>>>> for crossposting)
>>>>
>>>> may I please have feedback/reviews for this small enhancement.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8203682
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/webrev.00/webrev/
>>>>
>>>> This adds a new command to jcmd, "VM.classloaders". It complements the
>>>> existing command "VM.classloader_stats".
>>>>
>>>> This command, in its simplest form, prints the class loader tree. In
>>>> addition to that, it optionally prints out loaded classes (both
>>>> non-anonymous and anonymous) and various classloader specific
>>>> information.
>>>>
>>>> Examples:
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/example.txt
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/example-with-classes.txt
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/example-with-reflection-and-noinflation.txt
>>>>
>>>>
>>>> Thanks and Best Regards,
>>>>
>>>> Thomas
>
>
More information about the hotspot-runtime-dev
mailing list