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

[Thomas Stüfe]

Hi David,

since I have my two reviews, I would like to push this before the door
closes tomorrow afternoon. Do you have any serious objections left?

Thanks, Thomas

On Wed, Jun 27, 2018 at 2:21 PM, David Holmes <david.holmes at oracle.com> wrote:
> 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