On Fri, 16 Sep 2022 14:55:42 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
> Fixes a bug in the `VM.classloaders` jcmd that causes class loaders to be
> omitted from the output if a parent class loader never loaded any class and
> therefore had no associated DCmd.
>
> The fix changes the command to not rely on the existence of a CLD structure
> for the loader; instead, all information (loader class name, loader name,
> etc) is pulled via the loader oop, which has to be always there unless its
> the bootstrap loader.
>
> Also, the tests were expanded to test the display of empty loaders and empty
> parent loaders.
>
> Thanks to @dholmes-ora for finding this bug.
src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp line 187:
> 185: }
> 186:
> 187: void print_with_childs(outputStream* st, BranchTracker& branchtracker,
I know this isn't part of your changes, but the plural of child is children,
not childs. However, I think child_nodes would probably be best here.
src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp line 320:
> 318: return (loader_klass() != NULL) && (loader_klass() ==
> target_node->loader_klass()) &&
> 319: ((loader_name_oop() == NULL && target_node->loader_name_oop()
> == NULL) ||
> 320: (::strcmp(loader_name(), target_node->loader_name()) == 0));
This is hard to parse. I think using if/else statements would be cleaner.
test/hotspot/jtreg/serviceability/dcmd/vm/ClassLoaderHierarchyTest.java line
124:
> 122: output.shouldContain(" +-- \"Kevin\",
> ClassLoaderHierarchyTest$TestClassLoader");
> 123: output.shouldContain(" +--
> ClassLoaderHierarchyTest$TestClassLoader");
> 124: output.shouldContain(" | +-- \"Bill\",
> ClassLoaderHierarchyTest$TestClassLoader");
Can you test using one long string, and do the same for each section below? I
think embedding \\R as the newline character should work on all platforms. If
you run into unforeseen issues, then don't worry about.
-------------
PR: https://git.openjdk.org/jdk/pull/10312
