RFR(s): 8203682: Add jcmd "VM.classloaders" command to print out class loader hierarchy, details
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Tue Jun 5 18:26:18 UTC 2018
http://cr.openjdk.java.net/~stuefe/webrevs/8203682-jcmd-print-classloader-hierarchy/webrev.00-to-01/webrev/src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp.udiff.html
+ if (_cld == NULL) {
+ // Not sure how this could happen: we added a preliminary node for a
parent but then never encountered
+ // its CLD?
+ return;
}
I hope this is impossible.
+ st->print(" %s", loader_klass != NULL ? loader_klass->external_name()
: "??");
If the loader_klass is null, this should be <bootstrap> right?
Otherwise, this change looks good. It wraps a bit on my screen, but I
guess you should have a wide screen to look at this output.
thanks,
Coleen
On 6/5/18 1:43 PM, Thomas Stüfe wrote:
> I did like Kirk's proposal, which mirrors Coleen's proposal about
> printing the address and also is consistent with how
> VM.system_dictionary prints the loader instance.
>
> I changed my change accordingly (in-place, since I did not want to
> confuse reviewers).
>
> The output looks like this:
>
> +--
> org.springframework.boot.devtools.restart.classloader.RestartClassLoader
> {0x00000007065adba8}
> | |
> | | Classes: hello.Application
> | | hello.GreetingController
> | |
> hello.Application$$EnhancerBySpringCGLIB$$6738b31f
> | | com.sun.proxy.$Proxy49
> | |
> org.springframework.boot.autoconfigure.http.HttpMessageConverters$$EnhancerBySpringCGLIB$$1d90bff9
> | |
> org.springframework.boot.autoconfigure.http.HttpMessageConverters$$FastClassBySpringCGLIB$$d5af8918
> | |
> org.springframework.boot.autoconfigure.http.HttpMessageConverters$$EnhancerBySpringCGLIB$$1d90bff9$$FastClassBySpringCGLIB$$8fc601a2
> | | (7 classes)
> | |
> | +--
> org.springframework.boot.web.embedded.tomcat.TomcatEmbeddedWebappClassLoader
> {0x000000070f900e40}
> | |
> | +-- jdk.internal.reflect.DelegatingClassLoader
> {0x000000070f9004c0}
> |
> | Classes:
> jdk.internal.reflect.GeneratedSerializationConstructorAccessor4
> | (1 class)
>
>
> so, class loader instance address now is printed unconditionally and
> trails the name/class of the loader.
>
> For reviewing, the current webrevs remain:
>
> 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/
>
> Thanks a lot,
>
> Thomas
>
>
> On Mon, Jun 4, 2018 at 8:56 PM, Kirk Pepperdine
> <kirk.pepperdine at gmail.com> wrote:
>>>> 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
>> I can bike shed here…
>>
>> “platform”,jdk.internal.loader.ClassLoaders$PlatformClassLoader{0x000000060d077b90}
>> |
>> | Loader Data: 0x00007fc4583f7350
>> | Loader Klass: 0x00000008000107b0
>> |
>> +-- "app",jdk.internal.loader.ClassLoaders$AppClassLoader{0x000000060d079b30}
>> |
>> | Loader Data: 0x00007fc4583eb740
>> | Loader Klass: 0x0000000800010348
>>
>> Kind regards,
>> Kirk
>>
More information about the hotspot-runtime-dev
mailing list