RFR(s): jcmd VM.classloaders should fold similar loaders

[David Holmes]

Hi Thomas,

On 27/06/2018 10:13 PM, Thomas Stüfe wrote:
> Hi David,
> 
> On Mon, Jun 25, 2018 at 7:48 AM, David Holmes <david.holmes at oracle.com> wrote:
>> Hi Thomas,
>>
>>
>> On 23/06/2018 5:10 AM, Thomas Stüfe wrote:
>>>
>>> Resent with the correct subject, sorry.
>>>
>>> ..Thomas
>>>
>>> On Fri, Jun 22, 2018 at 9:08 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
>>> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> may I have reviews for this small enhancement to the jcmd
>>>> VM.classloader diagnostic command:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8205531
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8205531-vm.classloader-tree-folding/webrev.00/webrev/
>>>>
>>>>
>>>> VM.classloaders prints a tree of class loaders. This tree can grow a
>>>> lot and become unwieldy, especially with a lot of similar loaders. One
>>>> prime example is the DelegatingClassLoader. It would be helpful were
>>>> all these loaders:
>>>>
>>>> 13114:
>>>> +-- <bootstrap>
>>>>         |
>>>>         +-- "platform",
>>>> jdk.internal.loader.ClassLoaders$PlatformClassLoader
>>>>               |
>>>>               +-- "app", jdk.internal.loader.ClassLoaders$AppClassLoader
>>>>                     |
>>>>                     +-- test3.internals.InMemoryClassLoader
>>>>                           |
>>>>                           +-- jdk.internal.reflect.DelegatingClassLoader
>>>>                           |
>>>>                           +-- jdk.internal.reflect.DelegatingClassLoader
>>>>                           |
>>>>                           +-- jdk.internal.reflect.DelegatingClassLoader
>>>>                           |
>>>>                           +-- jdk.internal.reflect.DelegatingClassLoader
>>>>                           |
>>>>                           +-- jdk.internal.reflect.DelegatingClassLoader
>>>>                           |
>>>>                           ...... repeat 1495 times
>>>>
>>>>    folded into one:
>>>>
>>>> 13114:
>>>> +-- <bootstrap>
>>>>         |
>>>>         +-- "platform",
>>>> jdk.internal.loader.ClassLoaders$PlatformClassLoader
>>>>               |
>>>>               +-- "app", jdk.internal.loader.ClassLoaders$AppClassLoader
>>>>                     |
>>>>                     +-- test3.internals.InMemoryClassLoader
>>>>                           |
>>>>                           +-- jdk.internal.reflect.DelegatingClassLoader
>>>> (+ 1499 more)
>>
>>
>> I don't quite understand that. These are different instances aren't they -
>> potentially uniquely named? So if we gave all loaders a default name (like
>> we do threads) they would all show up expanded again - right?
>>
> 
> This patch will fold loaders loaded by the same parent, having the
> same class and the same or no name.
> 
> Example:
> 
> 25401:
> +-- <bootstrap>
>        |
>        +-- "platform", jdk.internal.loader.ClassLoaders$PlatformClassLoader
>        |     |
>        |     +-- "app", jdk.internal.loader.ClassLoaders$AppClassLoader
>        |
>        +-- "small-loader", test3.internals.InMemoryClassLoader (+ 1101 more)
> 
> Note that I compare addresses of _Symbol_. So, loaders which have the
> _same_ default name still fold, see example above.

Right. So in the case of DelegatingClassloader, today this change will 
fold them all into one because they are unnamed. If tomorrow we give 
them a unique name then they will no longer fold. There's no substantive 
difference between the two cases that I can see so why should they 
present differently? I guess I'm not sure the name is actually 
significant here.

What happens when some of these similar loaders have distinct child 
loaders? How will things be grouped and displayed then?

Thanks,
David

> Thanks, Thomas
> 
> 
>> typo:
>>
>> !   // we we add a
>>
>> Not a review as I don't know any of the logic being modified.
>>
>> David
>>
>>
>>>>
>>>>
>>>> Original idea by Bernd Eckenfels, see
>>>>
>>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-May/023824.html
>>>>
>>>> Thank you, Thomas