RFR(s): 8203682: Add jcmd "VM.classloaders" command to print out class loader hierarchy, details

[Thomas Stüfe]

Hi Coleen,

On Thu, May 31, 2018 at 11:02 PM,  <coleen.phillimore at oracle.com> wrote:
>
> It seems that similar information can be found in the other patch that you
> sent:
>
> RFR(s): 8203219: VM.metaspace jcmd should optionally show loaded classes for
> loaders
>
> So you can do jcmd VM.metaspace with options to show all classes,
> VM.systemdictionary and show all classes (including initiated classes) and
> now VM.classloaders command.  This one seems to have nice output though.
> Maybe it's worth having another.
>

I think so too. But of course I would say this since I proposed the
patch :) Seriously, we have had a similar command for ages in our
support tools and our support people use it a lot.

My hope is that common parts - at least in the long run - should use
common coding. So, having "show-classes" in two or more commands
should generate identical output in contexts where this makes sense,
and as much as possible use shared coding. When I add a new feature, I
usually earmark it for future unification if things change. I do not
always get to it, but I try.

<snip>

>> My feeling is no, since this adds a new command, so there can be no
>> backward compat issues. What is the general policy for new jcmd
>> commands, or for that matter anything new added to the outside facing
>> interface (new options, new Xlog tracing flags, changed output for
>> existing options)? Do these things require CSR?
>>
>> My problem with CSR is that it introduces a bottleneck, since it can
>> only be approved by three very busy people at Oracle - if I understand
>> the process right. Yes, we need a process to agree e.g. on syntax -
>> desperately so, since e.g. sub option syntax in jcmd is a mess - but
>> we seem to be strapped for reviewers even for normal code reviews, so
>> the effect of creating a CSR in my experience is just a stop-of-work.
>
>
> We haven't normally filed CSRs to add jcmd options.  These things seem to be
> hidden in the implementation and that seems appropriate.

Thanks for confirming.

>>>
>>> 2. Is ClassLoaderHierarchyVMOperation a safepoint VM-op? I would expect
>>> it
>>> needs to be to be able to walk the CLD hierarchy, unless that is already
>>> guaranteed to be safely walkable. Either way a comment clearly stating
>>> that
>>> would be useful I think.
>>
>> According to Coleen, CLDG can be walked outside a safepoint, but I did
>> not want to risk it so I made it a safepoint operation (like other
>> commands walking the CLDG, e.g. VM.metaspace).
>
>
> Calling ClassLoaderDataGraph::cld_do() doesn't keep the ClassLoaderData
> alive while iterating since it's the GC interface to walking the CLDG, so
> this should be done in a safepoint.  For printing, the aliveness doesn't
> really matter so it could be done concurrently but I think it's better in a
> safepoint.

Good point, thanks.

>
>>
>>> Related to #2, is it really possible to encounter a CLD in the process of
>>> being unloaded? Wouldn't that happen at a safepoint?
>>
>> Not sure, I am not a GC expert. I see places where this may be called
>> concurrently, e.g. in the process of
>> -XX:+ClassUnloadingWithConcurrentMark?
>>
>> Since a diagnostic command should never endanger the VM it monitors, I
>> coded defensively.
>
>
> Thank you for checking for is_unloading.  I've recently changed the
> ClassLoaderData to not allow you to access the class_loader oop if the class
> is unloading.   For the existing GCs, the CLDG will not have classes that
> are unloading in the list, but this may not always be the case.
>

I briefly though about allowing unloaded CLDs and use Lois' (?) CLD
functions which use cached values for name and class name. But decided
against it since I saw that the CLD for unloaded classes gets moved to
an own list, so I should not see them. I would also have to show them
apart from the tree since I do not know the loader oop for this CLD
and hence do not know the former parent loader.

> I'll comment on the code in a separate reply.

Thanks! I got your webrev and will prepare an updated patch.

..Thomas