RFR(S) 8189688: NMT: Report per-class load metadata information

[Thomas Stüfe]

Hi Zhengyu,

this is a nice addition!

------

General remarks:

We already have a multitude of printing functions in metaspace.cpp, in
MetaspaceAux I find:

void MetaspaceAux::print_on(outputStream* out)

and

void MetaspaceAux::print_on(outputStream* out, Metaspace::MetadataType
mdtype)
void MetaspaceAux::dump(outputStream* out)

There even exists a cleanup issue:
https://bugs.openjdk.java.net/browse/JDK-8185034.

I am not happy that we add yet another printing function to this bunch,
which basically does the same. Is there any way to reuse the existing
functions?

If not, could we at least for now rename print_metadata() to something like
print_metadata_for_nmt(), to tell it apart from print_on etc? And mark them
for later cleanup and consolidation?

----------

About the numbers:

I am not sure who this output is for. Customers analyzing metaspace
shortage or hotspot developers debugging the metaspace allocation?
Depending on the answer I would change the output somewhat.

If this it is for customers, the numbers are too much and the names are
misleading. Especially the per-classloader numbers:

"Metadata   capacity=%10.2f%s used=%10.2f%s free=%10.2f%s waste=%10.2f%s"

If we only want to answer the question "how much metaspace does a
classloader use up?", either "capacity" or "used" should be sufficient. It
does not really matter much which, really, and for most cases the numbers
should be very similar.

In this case I would omit "free" and "waste", because those are
implementation details of the metaspace chunk allocation and - provided the
implementation is correct - provide no useful information. "free" just
means "space left in current chunk" which is a implementation detail. Once
the chunk is used up, a new one is aquired. "waste" is only one tiny part
of how memory can be wasted in the current metaspace implementation. It
does not include waste in retired VirtualspaceNodes or in free chunks
idling in free lists.

The latter would be an important addition, regardless if this is for
customers or for us. Chunks idling in freelists can make a huge impact, and
unfortunately this may happen in perfectly valid cases. See e.g. our JEP "
https://bugs.openjdk.java.net/browse/JDK-8166690". We have already a
printing routine for free chunks, in
ChunkManager::print_all_chunkmanagers(). Could you add this to your output?

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

Other than above notes, the changes seem straighforward, I did not see
anything wrong. Minor nits:

- PrintCLDMetaspaceInfoClosure::_out, _scale and _unit could all be
constant (with _out being a outputStream* const).
- We also do not need to provide scale *and* unit as argument, one can be
deduced from the other. This would also prevent mismatches.

I did not look closely at locking issues, I assume
MetaspaceAux::print_metadata() is supposed to run only in Safepoints?


Thanks & Kind Regards, Thomas






On Fri, Oct 20, 2017 at 4:00 PM, Zhengyu Gu <zgu at redhat.com> wrote:

> Up to now, there is little visibility into how metaspaces are used, and by
> whom.
>
> This proposed enhancement gives NMT the ability to report metaspace usages
> on per-classloader level, via a new NMT command "metadata"
>
>
> For example:
>
> 2496:
> Metaspaces:
>   Metadata space: reserved=      8192KB committed=      5888KB
>   Class    space: reserved=   1048576KB committed=       640KB
>
> Per-classloader metadata:
>
> ClassLoader: for anonymous class
>   Metadata   capacity=      5.00KB used=      1.16KB free=      3.84KB
> waste=      0.05KB
>   Class data capacity=      1.00KB used=      0.59KB free=      0.41KB
> waste=      0.00KB
>
> ....
>
> ClassLoader: <bootloader>
>   Metadata   capacity=   5640.00KB used=   5607.42KB free=     32.58KB
> waste=      0.46KB
>   Class data capacity=    520.00KB used=    500.94KB free=     19.06KB
> waste=      0.02KB
>
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8189688
> Webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>
> Test:
>
>   hotspot_tier1_runtime, fastdebug and release on x64 Linux.
>
> Thanks,
>
> -Zhengyu
>
>
>