RFR(s): 8203682: Add jcmd "VM.classloaders" command to print out class loader hierarchy, details
Thomas Stüfe
thomas.stuefe at gmail.com
Tue Jun 5 19:02:53 UTC 2018
On Tue, Jun 5, 2018 at 8:26 PM, <coleen.phillimore at oracle.com> wrote:
>
> 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?
>
Yes, I think both cases should be impossible. The latter case should
never happen since I already handle the bootstrap loader.
But I did not dare to add asserts, since with diagnostic code I am
very careful not to kill the VM. Its embarrassing when you tell a
customer to run a jcmd on his server node and that command crashes the
node :/
> 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.
Yes. It looks quite beautiful on my 2K monitor :)
Btw, I opened an cleanup issue to unify the tree printing so that I
would not loose track of it:
https://bugs.openjdk.java.net/browse/JDK-8204349 .
Thanks a lot, Coleen!
..Thomas
> 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