RFR: JDK-8293156: Dcmd VM.classloaders fails to print the full hierarchy [v2]

[Thomas Stuefe]

On Mon, 19 Sep 2022 18:45:35 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   cjplummer feedback
>
> 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.

Fixed

> 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.

Fixed

> 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.

I'd prefer to leave it this way. I tried what you suggested before, but the output can contain trailing spaces in the branchwork of the tree - that is a byproduct of the indentation writer (the class writing the branches) and not easy to fix. I wanted the test to be somewhat tolerant against minor future changes in how the tree is printed.

-------------

PR: https://git.openjdk.org/jdk/pull/10312