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

classic Classic list List threaded Threaded
33 messages Options
12
Reply | Threaded
Open this post in threaded view
|

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

Zhengyu Gu-2
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


Reply | Threaded
Open this post in threaded view
|

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

Thomas Stüfe-2
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 <[hidden email]> 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
>
>
>
Reply | Threaded
Open this post in threaded view
|

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

Zhengyu Gu-2
Hi Thomas,

Thanks for the review. Please see comments inline.

On 10/23/2017 09:01 AM, Thomas Stüfe wrote:

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

Yes, I noticed the existing print_on functions and thought about using
them. However, none of them matches NMT convention, e.g. NMT uses "="
between label and value.

I will rename print_metadata to print_metadata_for_nmt.


>
> ----------
>
> 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.
>
As far as I know, NMT is largely used by JVM developers.

"free" and "waste" information, actually, is very important for us to
understand some of issues with current implementation.
(e.g. https://bugs.openjdk.java.net/browse/JDK-8187338)

This patch is derived from our investigation of memory cost for classes,
these numbers play important roles.

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

Make sense! Could you recommend what data and format you would like to see?


>
> -------------
>
> 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.We do need scale here, since nmt command line can set scale and we do
have to honor that.

However, passing unit is ugly! I don't want to introduce inverse
dependence (metaspace -> nmt), which is also ugly.

Probably should move scale mapping code from NMTUtil to a common util?


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

No. sum_xxx_xxx_in_use queries are guarded by locks.

Thanks,

-Zhengyu

>
>
> Thanks & Kind Regards, Thomas
>
>
>
>
>
>
> On Fri, Oct 20, 2017 at 4:00 PM, Zhengyu Gu <[hidden email]
> <mailto:[hidden email]>> 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
>     <https://bugs.openjdk.java.net/browse/JDK-8189688>
>     Webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>     <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>
>     Test:
>
>        hotspot_tier1_runtime, fastdebug and release on x64 Linux.
>
>     Thanks,
>
>     -Zhengyu
>
>
>
Reply | Threaded
Open this post in threaded view
|

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

Thomas Stüfe-2
Hi Zhengyu,

On Mon, Oct 23, 2017 at 4:03 PM, Zhengyu Gu <[hidden email]> wrote:

> Hi Thomas,
>
> Thanks for the review. Please see comments inline.
>
> On 10/23/2017 09:01 AM, Thomas Stüfe wrote:
>
>> 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?
>>
>
> Yes, I noticed the existing print_on functions and thought about using
> them. However, none of them matches NMT convention, e.g. NMT uses "="
> between label and value.
>
> I will rename print_metadata to print_metadata_for_nmt.
>
>
Thank you.

>
>
>> ----------
>>
>> 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.
>>
>> As far as I know, NMT is largely used by JVM developers.
>
> "free" and "waste" information, actually, is very important for us to
> understand some of issues with current implementation.
> (e.g. https://bugs.openjdk.java.net/browse/JDK-8187338)
>
> This patch is derived from our investigation of memory cost for classes,
> these numbers play important roles.
>
>
Okay. So this is important for understanding cases where we have a large
number of miniscule class loaders, each one only having a few numbers of
chunks. In this case, would it be useful to have a running total of "free"
and "waste", too? Or even better, also average and peak values of "free"
and "waste" (to tell apart cases where you have one outlier vs cases where
just all metaspaces waste a lot of memory).

Just out of curiosity, I looked at
http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt  and it seems
that "free" is the problem, not "waste"? So I guess what happens is that
all the little classloaders allocate just enough memory to push them over
"_small_chunk_limit=4", so they allocate the first medium chunk, from which
then they take a small bite and leave the rest laying around?



> 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?
>>
>
> Make sense! Could you recommend what data and format you would like to see?
>
>
>
Would not ChunkManager::print_all_chunkmanagers() be sufficient?


>
>> -------------
>>
>> 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.We do need scale
>> here, since nmt command line can set scale and we do
>>
> have to honor that.
>
> However, passing unit is ugly! I don't want to introduce inverse
> dependence (metaspace -> nmt), which is also ugly.
>
>
Yes, that would be regrettable.


> Probably should move scale mapping code from NMTUtil to a common util?
>
>
>
That would be possible, these functions (scale_from_name etc) are simple
enough to be moved to a generic file. However, if you pass scale, deducing
the string that goes with it is trivial and I would have no qualms doing
this by hand in metaspace.cpp. But it is a matter of taste.


>
>> I did not look closely at locking issues, I assume
>> MetaspaceAux::print_metadata() is supposed to run only in Safepoints?
>>
>
> No. sum_xxx_xxx_in_use queries are guarded by locks.
>
> Thanks,
>
> -Zhengyu
>
>
Thanks, Thomas


>
>>
>> Thanks & Kind Regards, Thomas
>>
>>
>>
>>
>>
>>
>> On Fri, Oct 20, 2017 at 4:00 PM, Zhengyu Gu <[hidden email] <mailto:
>> [hidden email]>> 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
>>     <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>     Webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>     <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>
>>     Test:
>>
>>        hotspot_tier1_runtime, fastdebug and release on x64 Linux.
>>
>>     Thanks,
>>
>>     -Zhengyu
>>
>>
>>
>>
Reply | Threaded
Open this post in threaded view
|

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

Andrew Dinn
On 23/10/17 15:40, Thomas Stüfe wrote:

> Okay. So this is important for understanding cases where we have a large
> number of miniscule class loaders, each one only having a few numbers of
> chunks. In this case, would it be useful to have a running total of "free"
> and "waste", too? Or even better, also average and peak values of "free"
> and "waste" (to tell apart cases where you have one outlier vs cases where
> just all metaspaces waste a lot of memory).
>
> Just out of curiosity, I looked at
> http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt  and it seems
> that "free" is the problem, not "waste"? So I guess what happens is that
> all the little classloaders allocate just enough memory to push them over
> "_small_chunk_limit=4", so they allocate the first medium chunk, from which
> then they take a small bite and leave the rest laying around?

Yes, that is pretty how it works. Only the problem is twice as bad when
you have separate class metadata and non-class metadata (i.e. when using
compressed class oops which is by far the most common case) since this
invariably wastes more space.

Similar behaviour happens when an anonymous class is created because of
a lambda, except that you only get one small allocation before a medium
chunk is created. So, for small lambdas, you almost always end up using
around 50% of 2 1MB chunks and, for more complex lambdas, around 50% of
a 1MB chunk plus less than 50% of a 1MB + 4MB chunk. So, the space waste
for lambdas is really quite spectacular.

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
Reply | Threaded
Open this post in threaded view
|

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

Zhengyu Gu-2
In reply to this post by Thomas Stüfe-2
>
>
> Okay. So this is important for understanding cases where we have a large
> number of miniscule class loaders, each one only having a few numbers of
> chunks. In this case, would it be useful to have a running total of
> "free" and "waste", too? Or even better, also average and peak values of
> "free" and "waste" (to tell apart cases where you have one outlier vs
> cases where just all metaspaces waste a lot of memory).
> Just out of curiosity, I looked at
> http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt  and it seems
> that "free" is the problem, not "waste"? So I guess what happens is that
> all the little classloaders allocate just enough memory to push them
> over "_small_chunk_limit=4", so they allocate the first medium chunk,
> from which then they take a small bite and leave the rest laying around?
>
Yes. The free space of anonymous classes should be counted as waste, in
my opinion. I am not sure if everyone agrees, so I took the summary line
out of this patch.

I would be more than happy to restore the summary line, if you find it
is useful :-)


>         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
>         <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?
>
>
>     Make sense! Could you recommend what data and format you would like
>     to see?
>
>
>
> Would not ChunkManager::print_all_chunkmanagers() be sufficient?
Okay, I might need to tweak output format.

Thanks,

-Zhengyu

>
>
>         -------------
>
>         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.We do need scale here, since nmt command line can set
>         scale and we do
>
>     have to honor that.
>
>     However, passing unit is ugly! I don't want to introduce inverse
>     dependence (metaspace -> nmt), which is also ugly.
>
>
> Yes, that would be regrettable.
>
>     Probably should move scale mapping code from NMTUtil to a common util?
>
>
>
> That would be possible, these functions (scale_from_name etc) are simple
> enough to be moved to a generic file. However, if you pass scale,
> deducing the string that goes with it is trivial and I would have no
> qualms doing this by hand in metaspace.cpp. But it is a matter of taste.
>
>
>         I did not look closely at locking issues, I assume
>         MetaspaceAux::print_metadata() is supposed to run only in
>         Safepoints?
>
>
>     No. sum_xxx_xxx_in_use queries are guarded by locks.
>
>     Thanks,
>
>     -Zhengyu
>
>
> Thanks, Thomas
>
>
>
>         Thanks & Kind Regards, Thomas
>
>
>
>
>
>
>         On Fri, Oct 20, 2017 at 4:00 PM, Zhengyu Gu <[hidden email]
>         <mailto:[hidden email]> <mailto:[hidden email]
>         <mailto:[hidden email]>>> 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
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>              <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>              Webrev:
>         http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>            
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>
>              Test:
>
>                 hotspot_tier1_runtime, fastdebug and release on x64 Linux.
>
>              Thanks,
>
>              -Zhengyu
>
>
>
>
Reply | Threaded
Open this post in threaded view
|

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

Thomas Stüfe-2
In reply to this post by Andrew Dinn
On Mon, Oct 23, 2017 at 4:52 PM, Andrew Dinn <[hidden email]> wrote:

> On 23/10/17 15:40, Thomas Stüfe wrote:
> > Okay. So this is important for understanding cases where we have a large
> > number of miniscule class loaders, each one only having a few numbers of
> > chunks. In this case, would it be useful to have a running total of
> "free"
> > and "waste", too? Or even better, also average and peak values of "free"
> > and "waste" (to tell apart cases where you have one outlier vs cases
> where
> > just all metaspaces waste a lot of memory).
> >
> > Just out of curiosity, I looked at
> > http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt  and it seems
> > that "free" is the problem, not "waste"? So I guess what happens is that
> > all the little classloaders allocate just enough memory to push them over
> > "_small_chunk_limit=4", so they allocate the first medium chunk, from
> which
> > then they take a small bite and leave the rest laying around?
>
> Yes, that is pretty how it works. Only the problem is twice as bad when
> you have separate class metadata and non-class metadata (i.e. when using
> compressed class oops which is by far the most common case) since this
> invariably wastes more space.
>
>
:( In this case, is the performance gain of CompressedClassPointers even
worth it?


> Similar behaviour happens when an anonymous class is created because of
> a lambda, except that you only get one small allocation before a medium
> chunk is created. So, for small lambdas, you almost always end up using
> around 50% of 2 1MB chunks and, for more complex lambdas, around 50% of
> a 1MB chunk plus less than 50% of a 1MB + 4MB chunk. So, the space waste
> for lambdas is really quite spectacular.
>
>
Ouch.

I am confused about the sizes though . Are we talking about the chunk sizes
in metaspace.cpp? Because they are way smaller, with medium chunks being
32/64K? Or are these large chunks a RedHat addition?

Best Regards, Thomas


> regards,
>
>
> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>
Reply | Threaded
Open this post in threaded view
|

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

Thomas Stüfe-2
In reply to this post by Zhengyu Gu-2
On Mon, Oct 23, 2017 at 5:03 PM, Zhengyu Gu <[hidden email]> wrote:

>
>>
>> Okay. So this is important for understanding cases where we have a large
>> number of miniscule class loaders, each one only having a few numbers of
>> chunks. In this case, would it be useful to have a running total of "free"
>> and "waste", too? Or even better, also average and peak values of "free"
>> and "waste" (to tell apart cases where you have one outlier vs cases where
>> just all metaspaces waste a lot of memory).
>> Just out of curiosity, I looked at http://cr.openjdk.java.net/~zg
>> u/cld_metaspace/wildfly.txt  and it seems that "free" is the problem,
>> not "waste"? So I guess what happens is that all the little classloaders
>> allocate just enough memory to push them over "_small_chunk_limit=4", so
>> they allocate the first medium chunk, from which then they take a small
>> bite and leave the rest laying around?
>>
>> Yes. The free space of anonymous classes should be counted as waste, in
> my opinion. I am not sure if everyone agrees, so I took the summary line
> out of this patch.
>
> I would be more than happy to restore the summary line, if you find it is
> useful :-)
>
>
I agree with you. Typically class loaders stop loading at some point,
especially these tine ones, and often will not resume loading. So,
effectivly, the space is wasted. It still helps to distinguish "free" in
the current chunks from "free" in the other chunks to tell this situation
apart from a situation where we have just a bug in metaspace chunk handling
preventing us to use up our chunks.


>
>         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
>>         <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?
>>
>>
>>     Make sense! Could you recommend what data and format you would like
>>     to see?
>>
>>
>>
>> Would not ChunkManager::print_all_chunkmanagers() be sufficient?
>>
> Okay, I might need to tweak output format.
>
>
Fine by me. Nothing depends on it yet.

Thanks, Thomas


> Thanks,
>
> -Zhengyu
>
>
>>
>>         -------------
>>
>>         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.We do need scale here, since nmt command line can set
>>         scale and we do
>>
>>     have to honor that.
>>
>>     However, passing unit is ugly! I don't want to introduce inverse
>>     dependence (metaspace -> nmt), which is also ugly.
>>
>>
>> Yes, that would be regrettable.
>>
>>     Probably should move scale mapping code from NMTUtil to a common util?
>>
>>
>>
>> That would be possible, these functions (scale_from_name etc) are simple
>> enough to be moved to a generic file. However, if you pass scale, deducing
>> the string that goes with it is trivial and I would have no qualms doing
>> this by hand in metaspace.cpp. But it is a matter of taste.
>>
>>
>>         I did not look closely at locking issues, I assume
>>         MetaspaceAux::print_metadata() is supposed to run only in
>>         Safepoints?
>>
>>
>>     No. sum_xxx_xxx_in_use queries are guarded by locks.
>>
>>     Thanks,
>>
>>     -Zhengyu
>>
>>
>> Thanks, Thomas
>>
>>
>>
>>         Thanks & Kind Regards, Thomas
>>
>>
>>
>>
>>
>>
>>         On Fri, Oct 20, 2017 at 4:00 PM, Zhengyu Gu <[hidden email]
>>         <mailto:[hidden email]> <mailto:[hidden email]
>>
>>         <mailto:[hidden email]>>> 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
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>              <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>              Webrev:
>>         http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                     <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>
>>              Test:
>>
>>                 hotspot_tier1_runtime, fastdebug and release on x64 Linux.
>>
>>              Thanks,
>>
>>              -Zhengyu
>>
>>
>>
>>
>>
Reply | Threaded
Open this post in threaded view
|

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

Andrew Dinn
In reply to this post by Thomas Stüfe-2
On 23/10/17 16:03, Thomas Stüfe wrote:
> I am confused about the sizes though . Are we talking about the chunk
> sizes in metaspace.cpp? Because they are way smaller, with medium chunks
> being 32/64K? Or are these large chunks a RedHat addition?
Oops, apologies -- wrong units!

Yes, this is indeed chunk sizes but I meant to say 1KB and 4KB chunks
(MBs would be truly dreadful :-). The relevant definitions in
metaspace.cpp are:

enum ChunkSizes {    // in words.
  ClassSpecializedChunk = 128,
  SpecializedChunk = 128,
  ClassSmallChunk = 256,
  SmallChunk = 512,
  ClassMediumChunk = 4 * K,
  MediumChunk = 8 * K
};

Anonymous classes (used for the anon classes used to back lambdas)
initially allocate a ClassSpecializedChunk and a SpecializedChunk i.e. 2
x 128 * 8 bytes = 2 x 1KB. If you have something that goes beyond the
most basic lambda then you may well also allocate an extra SmallChunk =
512 * 8 bytes = 4KB. We experimented (with both real and synthetic
examples) and found a high proportion of cases where 6KB chunks was
being allocated but around 50% was left unused as free tail ends of
these 2/3 chunks.

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
Reply | Threaded
Open this post in threaded view
|

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

Zhengyu Gu-2
In reply to this post by Thomas Stüfe-2
Hi Thomas,

I added chunkmanager statistics to the output.

However, I did not revive per-classload summary line. Had Second
thought, that NMT probably should just report facts and leave
interpretation to users.

Updated webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html

Thanks,

-Zhengyu


On 10/23/2017 11:08 AM, Thomas Stüfe wrote:

>
>
> On Mon, Oct 23, 2017 at 5:03 PM, Zhengyu Gu <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>
>
>         Okay. So this is important for understanding cases where we have
>         a large number of miniscule class loaders, each one only having
>         a few numbers of chunks. In this case, would it be useful to
>         have a running total of "free" and "waste", too? Or even better,
>         also average and peak values of "free" and "waste" (to tell
>         apart cases where you have one outlier vs cases where just all
>         metaspaces waste a lot of memory).
>         Just out of curiosity, I looked at
>         http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>  and
>         it seems that "free" is the problem, not "waste"? So I guess
>         what happens is that all the little classloaders allocate just
>         enough memory to push them over "_small_chunk_limit=4", so they
>         allocate the first medium chunk, from which then they take a
>         small bite and leave the rest laying around?
>
>     Yes. The free space of anonymous classes should be counted as waste,
>     in my opinion. I am not sure if everyone agrees, so I took the
>     summary line out of this patch.
>
>     I would be more than happy to restore the summary line, if you find
>     it is useful :-)
>
>
> I agree with you. Typically class loaders stop loading at some point,
> especially these tine ones, and often will not resume loading. So,
> effectivly, the space is wasted. It still helps to distinguish "free" in
> the current chunks from "free" in the other chunks to tell this
> situation apart from a situation where we have just a bug in metaspace
> chunk handling preventing us to use up our chunks.
>
>
>                  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
>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>         <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?
>
>
>              Make sense! Could you recommend what data and format you
>         would like
>              to see?
>
>
>
>         Would not ChunkManager::print_all_chunkmanagers() be sufficient?
>
>     Okay, I might need to tweak output format.
>
>
> Fine by me. Nothing depends on it yet.
>
> Thanks, Thomas
>
>     Thanks,
>
>     -Zhengyu
>
>
>
>                  -------------
>
>                  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.We do need scale here, since nmt command
>         line can set
>                  scale and we do
>
>              have to honor that.
>
>              However, passing unit is ugly! I don't want to introduce
>         inverse
>              dependence (metaspace -> nmt), which is also ugly.
>
>
>         Yes, that would be regrettable.
>
>              Probably should move scale mapping code from NMTUtil to a
>         common util?
>
>
>
>         That would be possible, these functions (scale_from_name etc)
>         are simple enough to be moved to a generic file. However, if you
>         pass scale, deducing the string that goes with it is trivial and
>         I would have no qualms doing this by hand in metaspace.cpp. But
>         it is a matter of taste.
>
>
>                  I did not look closely at locking issues, I assume
>                  MetaspaceAux::print_metadata() is supposed to run only in
>                  Safepoints?
>
>
>              No. sum_xxx_xxx_in_use queries are guarded by locks.
>
>              Thanks,
>
>              -Zhengyu
>
>
>         Thanks, Thomas
>
>
>
>                  Thanks & Kind Regards, Thomas
>
>
>
>
>
>
>                  On Fri, Oct 20, 2017 at 4:00 PM, Zhengyu Gu
>         <[hidden email] <mailto:[hidden email]>
>                  <mailto:[hidden email] <mailto:[hidden email]>>
>         <mailto:[hidden email] <mailto:[hidden email]>
>
>                  <mailto:[hidden email] <mailto:[hidden email]>>>> 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
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                       <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>                       Webrev:
>         http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                            
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>
>                       Test:
>
>                          hotspot_tier1_runtime, fastdebug and release on
>         x64 Linux.
>
>                       Thanks,
>
>                       -Zhengyu
>
>
>
>
>
Reply | Threaded
Open this post in threaded view
|

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

Thomas Stüfe-2
Hi Zhengyu,

On Mon, Oct 23, 2017 at 8:19 PM, Zhengyu Gu <[hidden email]> wrote:

> Hi Thomas,
>
> I added chunkmanager statistics to the output.
>
> However, I did not revive per-classload summary line. Had Second thought,
> that NMT probably should just report facts and leave interpretation to
> users.
>
>
Not sure whether we misunderstood each other. I was thinking of something
in the line of:

<<<<
....
ClassLoader: jdk/internal/reflect/DelegatingClassLoader
  Metadata:
     capacity:     5.00KB used:     3.32KB free:     1.68KB waste: 0.00KB
  Class data:
     capacity:     1.00KB used:     0.54KB free:     0.46KB waste: 0.00KB

ClassLoader: for anonymous class
  Metadata:
     capacity:     1.00KB used:     1.00KB free:     0.00KB waste: 0.00KB
  Class data:
     capacity:     1.00KB used:     0.64KB free:     0.36KB waste: 0.00KB
....

Summary:
XX class loaders encountered, total capacity: xx, total waste: xx.

Peak usage by class loader: xxxx
>>>>

These numbers would not be interpretation, just a convenience to the reader
to get a clear picture.

It even may be useful to separate the output between basic and detailed
mode, and in basic mode omit all the single class loaders and just print
the summary line.



> Updated webrev: http://cr.openjdk.java.net/~zg
> u/8189688/webrev.01/index.html
>
>

metaspace.hpp:

I would have preferred to keep scale_unit file local static instead of
exposing it via MetaspaceAux, because it does not really have anything to
do with metaspace.

Otherwise ok.

---

metaspace.cpp

- ChunkManager::print_statistics(): thanks for reusing this function!
    -> Scale can only be ever 1, K, M, G, yes? So, could you add an assert
at the start of the function, and a comment at the prototype or function
head?
    -> Also, now that ChunkManager::print_statistics() takes a scale, could
you please change the printout to use floats like you did in
PrintCLDMetaspaceInfoClosure::print_metaspace()?

- I am concerned about races in
PrintCLDMetaspaceInfoClosure::do_cld(ClassLoaderData* cld). Maybe my
understanding is limited. We bail if cld->is_unloading. But nothing
prevents a ClassLoaderDataGraph from starting to unload a ClassLoaderData
and tearing down the Metaspace after we started printing it in another
thread, right? Also, I do not see any fences.

So, I wonder whether PrintCLDMetaspaceInfoClosure should run in lock
protection. And then, I wonder if it would be good to separate data
gathering and printing. We print to a (at least in principle) unknown
output stream, which may be doing slow File IO or even Network IO. Doing
this under lock protection seems not a good idea (but I see other places
where this happens too).

For an example, see ChunkManager::get_statistic() vs
ChunkManager::print_statistic(). Could you do the same, have a data
gathering step where you collect your <capacity/used/free/waste> quadrupel
in a variable sized list of structures in memory, and print it out in a
separate step? I realize though that the effort would be larger than for
what we did in ChunkManager::get_statistic(), where we only fill a
structure.

------

vm_operations.hpp

- VM_PrintMetadata : do we still need _unit?




> Thanks,
>
> -Zhengyu
>


Thanks!

Thomas



>
>
> On 10/23/2017 11:08 AM, Thomas Stüfe wrote:
>
>>
>>
>> On Mon, Oct 23, 2017 at 5:03 PM, Zhengyu Gu <[hidden email] <mailto:
>> [hidden email]>> wrote:
>>
>>
>>
>>         Okay. So this is important for understanding cases where we have
>>         a large number of miniscule class loaders, each one only having
>>         a few numbers of chunks. In this case, would it be useful to
>>         have a running total of "free" and "waste", too? Or even better,
>>         also average and peak values of "free" and "waste" (to tell
>>         apart cases where you have one outlier vs cases where just all
>>         metaspaces waste a lot of memory).
>>         Just out of curiosity, I looked at
>>         http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>  and
>>         it seems that "free" is the problem, not "waste"? So I guess
>>         what happens is that all the little classloaders allocate just
>>         enough memory to push them over "_small_chunk_limit=4", so they
>>         allocate the first medium chunk, from which then they take a
>>         small bite and leave the rest laying around?
>>
>>     Yes. The free space of anonymous classes should be counted as waste,
>>     in my opinion. I am not sure if everyone agrees, so I took the
>>     summary line out of this patch.
>>
>>     I would be more than happy to restore the summary line, if you find
>>     it is useful :-)
>>
>>
>> I agree with you. Typically class loaders stop loading at some point,
>> especially these tine ones, and often will not resume loading. So,
>> effectivly, the space is wasted. It still helps to distinguish "free" in
>> the current chunks from "free" in the other chunks to tell this situation
>> apart from a situation where we have just a bug in metaspace chunk handling
>> preventing us to use up our chunks.
>>
>>
>>                  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
>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>>         <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?
>>
>>
>>              Make sense! Could you recommend what data and format you
>>         would like
>>              to see?
>>
>>
>>
>>         Would not ChunkManager::print_all_chunkmanagers() be sufficient?
>>
>>     Okay, I might need to tweak output format.
>>
>>
>> Fine by me. Nothing depends on it yet.
>>
>> Thanks, Thomas
>>
>>     Thanks,
>>
>>     -Zhengyu
>>
>>
>>
>>                  -------------
>>
>>                  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.We do need scale here, since nmt command
>>         line can set
>>                  scale and we do
>>
>>              have to honor that.
>>
>>              However, passing unit is ugly! I don't want to introduce
>>         inverse
>>              dependence (metaspace -> nmt), which is also ugly.
>>
>>
>>         Yes, that would be regrettable.
>>
>>              Probably should move scale mapping code from NMTUtil to a
>>         common util?
>>
>>
>>
>>         That would be possible, these functions (scale_from_name etc)
>>         are simple enough to be moved to a generic file. However, if you
>>         pass scale, deducing the string that goes with it is trivial and
>>         I would have no qualms doing this by hand in metaspace.cpp. But
>>         it is a matter of taste.
>>
>>
>>                  I did not look closely at locking issues, I assume
>>                  MetaspaceAux::print_metadata() is supposed to run only
>> in
>>                  Safepoints?
>>
>>
>>              No. sum_xxx_xxx_in_use queries are guarded by locks.
>>
>>              Thanks,
>>
>>              -Zhengyu
>>
>>
>>         Thanks, Thomas
>>
>>
>>
>>                  Thanks & Kind Regards, Thomas
>>
>>
>>
>>
>>
>>
>>                  On Fri, Oct 20, 2017 at 4:00 PM, Zhengyu Gu
>>         <[hidden email] <mailto:[hidden email]>
>>                  <mailto:[hidden email] <mailto:[hidden email]>>
>>         <mailto:[hidden email] <mailto:[hidden email]>
>>
>>                  <mailto:[hidden email] <mailto:[hidden email]>>>> 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
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>                       <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>                       Webrev:
>>         http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>                                     <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>>
>>                       Test:
>>
>>                          hotspot_tier1_runtime, fastdebug and release on
>>         x64 Linux.
>>
>>                       Thanks,
>>
>>                       -Zhengyu
>>
>>
>>
>>
>>
>>
Reply | Threaded
Open this post in threaded view
|

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

Thomas Stüfe-2
In reply to this post by Andrew Dinn
Hi Andrew,

On Mon, Oct 23, 2017 at 5:29 PM, Andrew Dinn <[hidden email]> wrote:

> On 23/10/17 16:03, Thomas Stüfe wrote:
> > I am confused about the sizes though . Are we talking about the chunk
> > sizes in metaspace.cpp? Because they are way smaller, with medium chunks
> > being 32/64K? Or are these large chunks a RedHat addition?
> Oops, apologies -- wrong units!
>
> Yes, this is indeed chunk sizes but I meant to say 1KB and 4KB chunks
> (MBs would be truly dreadful :-).


That makes more sense :)



> The relevant definitions in
> metaspace.cpp are:
>
> enum ChunkSizes {    // in words.
>   ClassSpecializedChunk = 128,
>   SpecializedChunk = 128,
>   ClassSmallChunk = 256,
>   SmallChunk = 512,
>   ClassMediumChunk = 4 * K,
>   MediumChunk = 8 * K
> };
>
> Anonymous classes (used for the anon classes used to back lambdas)
> initially allocate a ClassSpecializedChunk and a SpecializedChunk i.e. 2
> x 128 * 8 bytes = 2 x 1KB. If you have something that goes beyond the
> most basic lambda then you may well also allocate an extra SmallChunk =
> 512 * 8 bytes = 4KB. We experimented (with both real and synthetic
> examples) and found a high proportion of cases where 6KB chunks was
> being allocated but around 50% was left unused as free tail ends of
> these 2/3 chunks.
>
> regards,
>
>
>
Last year I analyzed some OOM-in-classspace situations. I saw this problem,
but was
disregarding it as something which would have diminishing effect with
increased number
of loaded classes. But now I see that this is indeed a real world problem.

(The actual problem in my case turned out to be the class space being
flooded with small chunks,
which were free because its creating class loaders have been unloaded, but
due to the unability
of the metaspace coding to reuse chunks in a different size were not
reused. We added a patch
in our fork to split-and-merge free metaspace chunks on demand to deal with
this problem.)

My first impulse was to say "Well, lets make the _small_chunk_limit
configurable via command line",
but I think that is a short sighted stop gap. A better solution may be to
make _small_chunk_limit  a
per-classloader-setting, so that class loaders which we know will only be
used for some anonymous
classes can be started with a high _small_chunk_limit  to never let them
allocate medium chunks.

I wonder if another solution could be some sort of stealing mechanism:
Periodically walk all class loaders,
and, if they have half-eaten medium chunks laying around they did not touch
for a while, split that chunk into
n small chunks and return n-1 chunks to the free list. This only works
though if the "used" portion of the
medium chunk does not surpass the size of one small chunk, because the
portion in use cannot be split
into multiple chunks. Considering that the size ratio small:medium is 1:16,
this may not be that useful. As
you say, usually the medium chunks are used up about half way.

Kind Regards, Thomas


> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>
Reply | Threaded
Open this post in threaded view
|

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

Zhengyu Gu-2
In reply to this post by Thomas Stüfe-2
Hi Thomas,

>
> Not sure whether we misunderstood each other. I was thinking of
> something in the line of:
>
> <<<<
> ....
> ClassLoader: jdk/internal/reflect/DelegatingClassLoader
>    Metadata:
>       capacity:     5.00KB used:     3.32KB free:     1.68KB waste: 0.00KB
>    Class data:
>       capacity:     1.00KB used:     0.54KB free:     0.46KB waste: 0.00KB
>
> ClassLoader: for anonymous class
>    Metadata:
>       capacity:     1.00KB used:     1.00KB free:     0.00KB waste: 0.00KB
>    Class data:
>       capacity:     1.00KB used:     0.64KB free:     0.36KB waste: 0.00KB
> ....
>
> Summary:
> XX class loaders encountered, total capacity: xx, total waste: xx.
>
> Peak usage by class loader: xxxx
>  >>>>
Added summary lines:

   Total class loaders=    56 capacity=   6378.00KB used=   6205.08KB
free=    172.92KB waste=      1.44KB
For anonymous classes=    54 capacity=    212.00KB used=     96.95KB
free=    115.05KB waste=      0.94KB



>
> These numbers would not be interpretation, just a convenience to the
> reader to get a clear picture.
>
> It even may be useful to separate the output between basic and detailed
> mode, and in basic mode omit all the single class loaders and just print
> the summary line.
>
>     Updated webrev:
>     http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html
>     <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>
>
>
>
> metaspace.hpp:
>
> I would have preferred to keep scale_unit file local static instead of
> exposing it via MetaspaceAux, because it does not really have anything
> to do with metaspace.
Fixed

>
> Otherwise ok.
>
> ---
>
> metaspace.cpp
>
> - ChunkManager::print_statistics(): thanks for reusing this function!
>      -> Scale can only be ever 1, K, M, G, yes? So, could you add an
> assert at the start of the function, and a comment at the prototype or
> function head?
>      -> Also, now that ChunkManager::print_statistics() takes a scale,
> could you please change the printout to use floats like you did in
> PrintCLDMetaspaceInfoClosure::print_metaspace()?
Done.


Chunkmanager (non-class):
   0 specialized (128 bytes) chunks, total 0.00KB
   0 small (512 bytes) chunks, total 0.00KB
   0 medium (8192 bytes) chunks, total 0.00KB
   0 humongous chunks, total 0.00KB
   total size: 0.00KB.
Chunkmanager (class):
   0 specialized (128 bytes) chunks, total 0.00KB
   0 small (256 bytes) chunks, total 0.00KB
   0 medium (4096 bytes) chunks, total 0.00KB
   0 humongous chunks, total 0.00KB
   total size: 0.00KB.

>
> - I am concerned about races in
> PrintCLDMetaspaceInfoClosure::do_cld(ClassLoaderData* cld). Maybe my
> understanding is limited. We bail if cld->is_unloading. But nothing
> prevents a ClassLoaderDataGraph from starting to unload a
> ClassLoaderData and tearing down the Metaspace after we started printing
> it in another thread, right? Also, I do not see any fences.
>
> So, I wonder whether PrintCLDMetaspaceInfoClosure should run in lock
> protection. And then, I wonder if it would be good to separate data
> gathering and printing. We print to a (at least in principle) unknown
> output stream, which may be doing slow File IO or even Network IO. Doing
> this under lock protection seems not a good idea (but I see other places
> where this happens too).
>
> For an example, see ChunkManager::get_statistic() vs
> ChunkManager::print_statistic(). Could you do the same, have a data
> gathering step where you collect your <capacity/used/free/waste>
> quadrupel in a variable sized list of structures in memory, and print it
> out in a separate step? I realize though that the effort would be larger
> than for what we did in ChunkManager::get_statistic(), where we only
> fill a structure.
>

Unlike other NMT queries, this query is executed at a safepoint by
VMThread, so it should be okay.

Updated webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.02/

Thanks,

-Zhengyu


> ------
>
> vm_operations.hpp
>
> - VM_PrintMetadata : do we still need _unit?
>
>
>     Thanks,
>
>     -Zhengyu
>
>
>
> Thanks!
>
> Thomas
>
>
>
>     On 10/23/2017 11:08 AM, Thomas Stüfe wrote:
>
>
>
>         On Mon, Oct 23, 2017 at 5:03 PM, Zhengyu Gu <[hidden email]
>         <mailto:[hidden email]> <mailto:[hidden email]
>         <mailto:[hidden email]>>> wrote:
>
>
>
>                  Okay. So this is important for understanding cases
>         where we have
>                  a large number of miniscule class loaders, each one
>         only having
>                  a few numbers of chunks. In this case, would it be
>         useful to
>                  have a running total of "free" and "waste", too? Or
>         even better,
>                  also average and peak values of "free" and "waste" (to tell
>                  apart cases where you have one outlier vs cases where
>         just all
>                  metaspaces waste a lot of memory).
>                  Just out of curiosity, I looked at
>         http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>                
>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>  and
>                  it seems that "free" is the problem, not "waste"? So I
>         guess
>                  what happens is that all the little classloaders
>         allocate just
>                  enough memory to push them over "_small_chunk_limit=4",
>         so they
>                  allocate the first medium chunk, from which then they
>         take a
>                  small bite and leave the rest laying around?
>
>              Yes. The free space of anonymous classes should be counted
>         as waste,
>              in my opinion. I am not sure if everyone agrees, so I took the
>              summary line out of this patch.
>
>              I would be more than happy to restore the summary line, if
>         you find
>              it is useful :-)
>
>
>         I agree with you. Typically class loaders stop loading at some
>         point, especially these tine ones, and often will not resume
>         loading. So, effectivly, the space is wasted. It still helps to
>         distinguish "free" in the current chunks from "free" in the
>         other chunks to tell this situation apart from a situation where
>         we have just a bug in metaspace chunk handling preventing us to
>         use up our chunks.
>
>
>                           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
>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>         <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>                        
>           <https://bugs.openjdk.java.net/browse/JDK-8166690
>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>         <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?
>
>
>                       Make sense! Could you recommend what data and
>         format you
>                  would like
>                       to see?
>
>
>
>                  Would not ChunkManager::print_all_chunkmanagers() be
>         sufficient?
>
>              Okay, I might need to tweak output format.
>
>
>         Fine by me. Nothing depends on it yet.
>
>         Thanks, Thomas
>
>              Thanks,
>
>              -Zhengyu
>
>
>
>                           -------------
>
>                           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.We do need scale here, since nmt
>         command
>                  line can set
>                           scale and we do
>
>                       have to honor that.
>
>                       However, passing unit is ugly! I don't want to
>         introduce
>                  inverse
>                       dependence (metaspace -> nmt), which is also ugly.
>
>
>                  Yes, that would be regrettable.
>
>                       Probably should move scale mapping code from
>         NMTUtil to a
>                  common util?
>
>
>
>                  That would be possible, these functions
>         (scale_from_name etc)
>                  are simple enough to be moved to a generic file.
>         However, if you
>                  pass scale, deducing the string that goes with it is
>         trivial and
>                  I would have no qualms doing this by hand in
>         metaspace.cpp. But
>                  it is a matter of taste.
>
>
>                           I did not look closely at locking issues, I assume
>                           MetaspaceAux::print_metadata() is supposed to
>         run only in
>                           Safepoints?
>
>
>                       No. sum_xxx_xxx_in_use queries are guarded by locks.
>
>                       Thanks,
>
>                       -Zhengyu
>
>
>                  Thanks, Thomas
>
>
>
>                           Thanks & Kind Regards, Thomas
>
>
>
>
>
>
>                           On Fri, Oct 20, 2017 at 4:00 PM, Zhengyu Gu
>                  <[hidden email] <mailto:[hidden email]>
>         <mailto:[hidden email] <mailto:[hidden email]>>
>                           <mailto:[hidden email] <mailto:[hidden email]>
>         <mailto:[hidden email] <mailto:[hidden email]>>>
>                  <mailto:[hidden email] <mailto:[hidden email]>
>         <mailto:[hidden email] <mailto:[hidden email]>>
>
>                           <mailto:[hidden email] <mailto:[hidden email]>
>         <mailto:[hidden email] <mailto:[hidden email]>>>>> 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
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                        
>           <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>                              
>         <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                        
>           <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>
>                                Webrev:
>         http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>                                            
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>
>
>                                Test:
>
>                                   hotspot_tier1_runtime, fastdebug and
>         release on
>                  x64 Linux.
>
>                                Thanks,
>
>                                -Zhengyu
>
>
>
>
>
>
Reply | Threaded
Open this post in threaded view
|

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

Thomas Stüfe-2
Hi Zhengyu,

- VM_PrintMetadata still has the _unit member, but I see it nowhere used.

- I would have split the summary between class and non-class area, that may
have been more useful. But this is a matter of taste, I leave it up to you.

All else looks fine to me now. I do not need another review.

Thanks for your work, this will come in handy!

..Thomas

On Tue, Oct 24, 2017 at 5:08 PM, Zhengyu Gu <[hidden email]> wrote:

> Hi Thomas,
>
>
>> Not sure whether we misunderstood each other. I was thinking of something
>> in the line of:
>>
>> <<<<
>> ....
>> ClassLoader: jdk/internal/reflect/DelegatingClassLoader
>>    Metadata:
>>       capacity:     5.00KB used:     3.32KB free:     1.68KB waste: 0.00KB
>>    Class data:
>>       capacity:     1.00KB used:     0.54KB free:     0.46KB waste: 0.00KB
>>
>> ClassLoader: for anonymous class
>>    Metadata:
>>       capacity:     1.00KB used:     1.00KB free:     0.00KB waste: 0.00KB
>>    Class data:
>>       capacity:     1.00KB used:     0.64KB free:     0.36KB waste: 0.00KB
>> ....
>>
>> Summary:
>> XX class loaders encountered, total capacity: xx, total waste: xx.
>>
>> Peak usage by class loader: xxxx
>>  >>>>
>>
> Added summary lines:
>
>   Total class loaders=    56 capacity=   6378.00KB used=   6205.08KB
> free=    172.92KB waste=      1.44KB
> For anonymous classes=    54 capacity=    212.00KB used=     96.95KB
> free=    115.05KB waste=      0.94KB
>
>
>
>
>> These numbers would not be interpretation, just a convenience to the
>> reader to get a clear picture.
>>
>> It even may be useful to separate the output between basic and detailed
>> mode, and in basic mode omit all the single class loaders and just print
>> the summary line.
>>
>>     Updated webrev:
>>     http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html
>>     <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>
>>
>>
>>
>> metaspace.hpp:
>>
>> I would have preferred to keep scale_unit file local static instead of
>> exposing it via MetaspaceAux, because it does not really have anything to
>> do with metaspace.
>>
> Fixed
>
>
>> Otherwise ok.
>>
>> ---
>>
>> metaspace.cpp
>>
>> - ChunkManager::print_statistics(): thanks for reusing this function!
>>      -> Scale can only be ever 1, K, M, G, yes? So, could you add an
>> assert at the start of the function, and a comment at the prototype or
>> function head?
>>      -> Also, now that ChunkManager::print_statistics() takes a scale,
>> could you please change the printout to use floats like you did in
>> PrintCLDMetaspaceInfoClosure::print_metaspace()?
>>
> Done.
>
>
> Chunkmanager (non-class):
>   0 specialized (128 bytes) chunks, total 0.00KB
>   0 small (512 bytes) chunks, total 0.00KB
>   0 medium (8192 bytes) chunks, total 0.00KB
>   0 humongous chunks, total 0.00KB
>   total size: 0.00KB.
> Chunkmanager (class):
>   0 specialized (128 bytes) chunks, total 0.00KB
>   0 small (256 bytes) chunks, total 0.00KB
>   0 medium (4096 bytes) chunks, total 0.00KB
>   0 humongous chunks, total 0.00KB
>   total size: 0.00KB.
>
>
>> - I am concerned about races in PrintCLDMetaspaceInfoClosure::do_cld(ClassLoaderData*
>> cld). Maybe my understanding is limited. We bail if cld->is_unloading. But
>> nothing prevents a ClassLoaderDataGraph from starting to unload a
>> ClassLoaderData and tearing down the Metaspace after we started printing it
>> in another thread, right? Also, I do not see any fences.
>>
>> So, I wonder whether PrintCLDMetaspaceInfoClosure should run in lock
>> protection. And then, I wonder if it would be good to separate data
>> gathering and printing. We print to a (at least in principle) unknown
>> output stream, which may be doing slow File IO or even Network IO. Doing
>> this under lock protection seems not a good idea (but I see other places
>> where this happens too).
>>
>> For an example, see ChunkManager::get_statistic() vs
>> ChunkManager::print_statistic(). Could you do the same, have a data
>> gathering step where you collect your <capacity/used/free/waste> quadrupel
>> in a variable sized list of structures in memory, and print it out in a
>> separate step? I realize though that the effort would be larger than for
>> what we did in ChunkManager::get_statistic(), where we only fill a
>> structure.
>>
>>
> Unlike other NMT queries, this query is executed at a safepoint by
> VMThread, so it should be okay.
>
> Updated webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.02/
>
> Thanks,
>
> -Zhengyu
>
>
> ------
>>
>> vm_operations.hpp
>>
>> - VM_PrintMetadata : do we still need _unit?
>>
>>
>>     Thanks,
>>
>>     -Zhengyu
>>
>>
>>
>> Thanks!
>>
>> Thomas
>>
>>
>>
>>     On 10/23/2017 11:08 AM, Thomas Stüfe wrote:
>>
>>
>>
>>         On Mon, Oct 23, 2017 at 5:03 PM, Zhengyu Gu <[hidden email]
>>         <mailto:[hidden email]> <mailto:[hidden email]
>>         <mailto:[hidden email]>>> wrote:
>>
>>
>>
>>                  Okay. So this is important for understanding cases
>>         where we have
>>                  a large number of miniscule class loaders, each one
>>         only having
>>                  a few numbers of chunks. In this case, would it be
>>         useful to
>>                  have a running total of "free" and "waste", too? Or
>>         even better,
>>                  also average and peak values of "free" and "waste" (to
>> tell
>>                  apart cases where you have one outlier vs cases where
>>         just all
>>                  metaspaces waste a lot of memory).
>>                  Just out of curiosity, I looked at
>>         http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>                         <http://cr.openjdk.java.net/~z
>> gu/cld_metaspace/wildfly.txt
>>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>  and
>>                  it seems that "free" is the problem, not "waste"? So I
>>         guess
>>                  what happens is that all the little classloaders
>>         allocate just
>>                  enough memory to push them over "_small_chunk_limit=4",
>>         so they
>>                  allocate the first medium chunk, from which then they
>>         take a
>>                  small bite and leave the rest laying around?
>>
>>              Yes. The free space of anonymous classes should be counted
>>         as waste,
>>              in my opinion. I am not sure if everyone agrees, so I took
>> the
>>              summary line out of this patch.
>>
>>              I would be more than happy to restore the summary line, if
>>         you find
>>              it is useful :-)
>>
>>
>>         I agree with you. Typically class loaders stop loading at some
>>         point, especially these tine ones, and often will not resume
>>         loading. So, effectivly, the space is wasted. It still helps to
>>         distinguish "free" in the current chunks from "free" in the
>>         other chunks to tell this situation apart from a situation where
>>         we have just a bug in metaspace chunk handling preventing us to
>>         use up our chunks.
>>
>>
>>                           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
>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>                                   <https://bugs.openjdk.java.net
>> /browse/JDK-8166690
>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>>         <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?
>>
>>
>>                       Make sense! Could you recommend what data and
>>         format you
>>                  would like
>>                       to see?
>>
>>
>>
>>                  Would not ChunkManager::print_all_chunkmanagers() be
>>         sufficient?
>>
>>              Okay, I might need to tweak output format.
>>
>>
>>         Fine by me. Nothing depends on it yet.
>>
>>         Thanks, Thomas
>>
>>              Thanks,
>>
>>              -Zhengyu
>>
>>
>>
>>                           -------------
>>
>>                           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.We do need scale here, since nmt
>>         command
>>                  line can set
>>                           scale and we do
>>
>>                       have to honor that.
>>
>>                       However, passing unit is ugly! I don't want to
>>         introduce
>>                  inverse
>>                       dependence (metaspace -> nmt), which is also ugly.
>>
>>
>>                  Yes, that would be regrettable.
>>
>>                       Probably should move scale mapping code from
>>         NMTUtil to a
>>                  common util?
>>
>>
>>
>>                  That would be possible, these functions
>>         (scale_from_name etc)
>>                  are simple enough to be moved to a generic file.
>>         However, if you
>>                  pass scale, deducing the string that goes with it is
>>         trivial and
>>                  I would have no qualms doing this by hand in
>>         metaspace.cpp. But
>>                  it is a matter of taste.
>>
>>
>>                           I did not look closely at locking issues, I
>> assume
>>                           MetaspaceAux::print_metadata() is supposed to
>>         run only in
>>                           Safepoints?
>>
>>
>>                       No. sum_xxx_xxx_in_use queries are guarded by locks.
>>
>>                       Thanks,
>>
>>                       -Zhengyu
>>
>>
>>                  Thanks, Thomas
>>
>>
>>
>>                           Thanks & Kind Regards, Thomas
>>
>>
>>
>>
>>
>>
>>                           On Fri, Oct 20, 2017 at 4:00 PM, Zhengyu Gu
>>                  <[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>
>>                           <mailto:[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>>
>>                  <mailto:[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>
>>
>>                           <mailto:[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>>>> 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
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>                                   <https://bugs.openjdk.java.net
>> /browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>                                       <https://bugs.openjdk.java.net
>> /browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>                                   <https://bugs.openjdk.java.net
>> /browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>
>>                                Webrev:
>>         http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>                                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>>                                                     <
>> http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>                                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>
>>
>>                                Test:
>>
>>                                   hotspot_tier1_runtime, fastdebug and
>>         release on
>>                  x64 Linux.
>>
>>                                Thanks,
>>
>>                                -Zhengyu
>>
>>
>>
>>
>>
>>
>>
Reply | Threaded
Open this post in threaded view
|

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

Zhengyu Gu-2
Hi Thomas,

On 10/24/2017 12:08 PM, Thomas Stüfe wrote:
> Hi Zhengyu,
>
> - VM_PrintMetadata still has the _unit member, but I see it nowhere used.
>
> - I would have split the summary between class and non-class area, that
> may have been more useful. But this is a matter of taste, I leave it up
> to you.
>
Agree, should go little further.

Summary:
   Total class loaders=   754 capacity=  67528.00KB used=  61216.88KB
free=   9453.11KB waste=     38.73KB
                     Metadata capacity=  58780.00KB used=  54053.45KB
free=   4726.55KB waste=     36.38KB
                   Class data capacity=   8748.00KB used=   7163.43KB
free=   1584.57KB waste=      2.35KB

For anonymous classes=   580 capacity=   2732.00KB used=   1065.06KB
free=   1666.94KB waste=     16.27KB
                     Metadata capacity=   2152.00KB used=    738.46KB
free=   1413.54KB waste=     16.27KB
                   Class data capacity=    580.00KB used=    326.60KB
free=    253.40KB waste=      0.00KB


Updated webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.03/

Thanks,

-Zhengyu


> All else looks fine to me now. I do not need another review.
>
> Thanks for your work, this will come in handy!
>
> ..Thomas
>
> On Tue, Oct 24, 2017 at 5:08 PM, Zhengyu Gu <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi Thomas,
>
>
>         Not sure whether we misunderstood each other. I was thinking of
>         something in the line of:
>
>         <<<<
>         ....
>         ClassLoader: jdk/internal/reflect/DelegatingClassLoader
>             Metadata:
>                capacity:     5.00KB used:     3.32KB free:     1.68KB
>         waste: 0.00KB
>             Class data:
>                capacity:     1.00KB used:     0.54KB free:     0.46KB
>         waste: 0.00KB
>
>         ClassLoader: for anonymous class
>             Metadata:
>                capacity:     1.00KB used:     1.00KB free:     0.00KB
>         waste: 0.00KB
>             Class data:
>                capacity:     1.00KB used:     0.64KB free:     0.36KB
>         waste: 0.00KB
>         ....
>
>         Summary:
>         XX class loaders encountered, total capacity: xx, total waste: xx.
>
>         Peak usage by class loader: xxxx
>           >>>>
>
>     Added summary lines:
>
>        Total class loaders=    56 capacity=   6378.00KB used=
>       6205.08KB free=    172.92KB waste=      1.44KB
>     For anonymous classes=    54 capacity=    212.00KB used=     96.95KB
>     free=    115.05KB waste=      0.94KB
>
>
>
>
>         These numbers would not be interpretation, just a convenience to
>         the reader to get a clear picture.
>
>         It even may be useful to separate the output between basic and
>         detailed mode, and in basic mode omit all the single class
>         loaders and just print the summary line.
>
>              Updated webrev:
>         http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>
>            
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>>
>
>
>
>         metaspace.hpp:
>
>         I would have preferred to keep scale_unit file local static
>         instead of exposing it via MetaspaceAux, because it does not
>         really have anything to do with metaspace.
>
>     Fixed
>
>
>         Otherwise ok.
>
>         ---
>
>         metaspace.cpp
>
>         - ChunkManager::print_statistics(): thanks for reusing this
>         function!
>               -> Scale can only be ever 1, K, M, G, yes? So, could you
>         add an assert at the start of the function, and a comment at the
>         prototype or function head?
>               -> Also, now that ChunkManager::print_statistics() takes a
>         scale, could you please change the printout to use floats like
>         you did in PrintCLDMetaspaceInfoClosure::print_metaspace()?
>
>     Done.
>
>
>     Chunkmanager (non-class):
>        0 specialized (128 bytes) chunks, total 0.00KB
>        0 small (512 bytes) chunks, total 0.00KB
>        0 medium (8192 bytes) chunks, total 0.00KB
>        0 humongous chunks, total 0.00KB
>        total size: 0.00KB.
>     Chunkmanager (class):
>        0 specialized (128 bytes) chunks, total 0.00KB
>        0 small (256 bytes) chunks, total 0.00KB
>        0 medium (4096 bytes) chunks, total 0.00KB
>        0 humongous chunks, total 0.00KB
>        total size: 0.00KB.
>
>
>         - I am concerned about races in
>         PrintCLDMetaspaceInfoClosure::do_cld(ClassLoaderData* cld).
>         Maybe my understanding is limited. We bail if cld->is_unloading.
>         But nothing prevents a ClassLoaderDataGraph from starting to
>         unload a ClassLoaderData and tearing down the Metaspace after we
>         started printing it in another thread, right? Also, I do not see
>         any fences.
>
>         So, I wonder whether PrintCLDMetaspaceInfoClosure should run in
>         lock protection. And then, I wonder if it would be good to
>         separate data gathering and printing. We print to a (at least in
>         principle) unknown output stream, which may be doing slow File
>         IO or even Network IO. Doing this under lock protection seems
>         not a good idea (but I see other places where this happens too).
>
>         For an example, see ChunkManager::get_statistic() vs
>         ChunkManager::print_statistic(). Could you do the same, have a
>         data gathering step where you collect your
>         <capacity/used/free/waste> quadrupel in a variable sized list of
>         structures in memory, and print it out in a separate step? I
>         realize though that the effort would be larger than for what we
>         did in ChunkManager::get_statistic(), where we only fill a
>         structure.
>
>
>     Unlike other NMT queries, this query is executed at a safepoint by
>     VMThread, so it should be okay.
>
>     Updated webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.02/
>     <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/>
>
>     Thanks,
>
>     -Zhengyu
>
>
>         ------
>
>         vm_operations.hpp
>
>         - VM_PrintMetadata : do we still need _unit?
>
>
>              Thanks,
>
>              -Zhengyu
>
>
>
>         Thanks!
>
>         Thomas
>
>
>
>              On 10/23/2017 11:08 AM, Thomas Stüfe wrote:
>
>
>
>                  On Mon, Oct 23, 2017 at 5:03 PM, Zhengyu Gu
>         <[hidden email] <mailto:[hidden email]>
>                  <mailto:[hidden email] <mailto:[hidden email]>>
>         <mailto:[hidden email] <mailto:[hidden email]>
>                  <mailto:[hidden email] <mailto:[hidden email]>>>> wrote:
>
>
>
>                           Okay. So this is important for understanding cases
>                  where we have
>                           a large number of miniscule class loaders,
>         each one
>                  only having
>                           a few numbers of chunks. In this case, would it be
>                  useful to
>                           have a running total of "free" and "waste",
>         too? Or
>                  even better,
>                           also average and peak values of "free" and
>         "waste" (to tell
>                           apart cases where you have one outlier vs
>         cases where
>                  just all
>                           metaspaces waste a lot of memory).
>                           Just out of curiosity, I looked at
>         http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>                
>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>
>                                
>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>                
>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>>  and
>                           it seems that "free" is the problem, not
>         "waste"? So I
>                  guess
>                           what happens is that all the little classloaders
>                  allocate just
>                           enough memory to push them over
>         "_small_chunk_limit=4",
>                  so they
>                           allocate the first medium chunk, from which
>         then they
>                  take a
>                           small bite and leave the rest laying around?
>
>                       Yes. The free space of anonymous classes should be
>         counted
>                  as waste,
>                       in my opinion. I am not sure if everyone agrees,
>         so I took the
>                       summary line out of this patch.
>
>                       I would be more than happy to restore the summary
>         line, if
>                  you find
>                       it is useful :-)
>
>
>                  I agree with you. Typically class loaders stop loading
>         at some
>                  point, especially these tine ones, and often will not
>         resume
>                  loading. So, effectivly, the space is wasted. It still
>         helps to
>                  distinguish "free" in the current chunks from "free" in the
>                  other chunks to tell this situation apart from a
>         situation where
>                  we have just a bug in metaspace chunk handling
>         preventing us to
>                  use up our chunks.
>
>
>                                    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
>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>         <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>                        
>           <https://bugs.openjdk.java.net/browse/JDK-8166690
>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>         <https://bugs.openjdk.java.net/browse/JDK-8166690>>>
>                                          
>         <https://bugs.openjdk.java.net/browse/JDK-8166690
>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>         <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>                        
>           <https://bugs.openjdk.java.net/browse/JDK-8166690
>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>         <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?
>
>
>                                Make sense! Could you recommend what data and
>                  format you
>                           would like
>                                to see?
>
>
>
>                           Would not
>         ChunkManager::print_all_chunkmanagers() be
>                  sufficient?
>
>                       Okay, I might need to tweak output format.
>
>
>                  Fine by me. Nothing depends on it yet.
>
>                  Thanks, Thomas
>
>                       Thanks,
>
>                       -Zhengyu
>
>
>
>                                    -------------
>
>                                    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.We do need scale here,
>         since nmt
>                  command
>                           line can set
>                                    scale and we do
>
>                                have to honor that.
>
>                                However, passing unit is ugly! I don't
>         want to
>                  introduce
>                           inverse
>                                dependence (metaspace -> nmt), which is
>         also ugly.
>
>
>                           Yes, that would be regrettable.
>
>                                Probably should move scale mapping code from
>                  NMTUtil to a
>                           common util?
>
>
>
>                           That would be possible, these functions
>                  (scale_from_name etc)
>                           are simple enough to be moved to a generic file.
>                  However, if you
>                           pass scale, deducing the string that goes with
>         it is
>                  trivial and
>                           I would have no qualms doing this by hand in
>                  metaspace.cpp. But
>                           it is a matter of taste.
>
>
>                                    I did not look closely at locking
>         issues, I assume
>                                    MetaspaceAux::print_metadata() is
>         supposed to
>                  run only in
>                                    Safepoints?
>
>
>                                No. sum_xxx_xxx_in_use queries are
>         guarded by locks.
>
>                                Thanks,
>
>                                -Zhengyu
>
>
>                           Thanks, Thomas
>
>
>
>                                    Thanks & Kind Regards, Thomas
>
>
>
>
>
>
>                                    On Fri, Oct 20, 2017 at 4:00 PM,
>         Zhengyu Gu
>                           <[hidden email] <mailto:[hidden email]>
>         <mailto:[hidden email] <mailto:[hidden email]>>
>                  <mailto:[hidden email] <mailto:[hidden email]>
>         <mailto:[hidden email] <mailto:[hidden email]>>>
>                                    <mailto:[hidden email]
>         <mailto:[hidden email]> <mailto:[hidden email]
>         <mailto:[hidden email]>>
>                  <mailto:[hidden email] <mailto:[hidden email]>
>         <mailto:[hidden email] <mailto:[hidden email]>>>>
>                           <mailto:[hidden email] <mailto:[hidden email]>
>         <mailto:[hidden email] <mailto:[hidden email]>>
>                  <mailto:[hidden email] <mailto:[hidden email]>
>         <mailto:[hidden email] <mailto:[hidden email]>>>
>
>                                    <mailto:[hidden email]
>         <mailto:[hidden email]> <mailto:[hidden email]
>         <mailto:[hidden email]>>
>                  <mailto:[hidden email] <mailto:[hidden email]>
>         <mailto:[hidden email] <mailto:[hidden email]>>>>>> 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
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                        
>           <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>                                          
>         <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                        
>           <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>
>                                              
>         <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                        
>           <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>                                          
>         <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                        
>           <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>>
>                                         Webrev:
>         http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>                                                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>
>                                                            
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>                                                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>>
>
>                                         Test:
>
>                                            hotspot_tier1_runtime,
>         fastdebug and
>                  release on
>                           x64 Linux.
>
>                                         Thanks,
>
>                                         -Zhengyu
>
>
>
>
>
>
>
Reply | Threaded
Open this post in threaded view
|

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

Thomas Stüfe-2
Hi Zhengyu,

On Tue, Oct 24, 2017 at 8:04 PM, Zhengyu Gu <[hidden email]> wrote:

> Hi Thomas,
>
> On 10/24/2017 12:08 PM, Thomas Stüfe wrote:
>
>> Hi Zhengyu,
>>
>> - VM_PrintMetadata still has the _unit member, but I see it nowhere used.
>>
>> - I would have split the summary between class and non-class area, that
>> may have been more useful. But this is a matter of taste, I leave it up to
>> you.
>>
>> Agree, should go little further.
>
> Summary:
>   Total class loaders=   754 capacity=  67528.00KB used=  61216.88KB
> free=   9453.11KB waste=     38.73KB
>                     Metadata capacity=  58780.00KB used=  54053.45KB
> free=   4726.55KB waste=     36.38KB
>                   Class data capacity=   8748.00KB used=   7163.43KB
> free=   1584.57KB waste=      2.35KB
>
> For anonymous classes=   580 capacity=   2732.00KB used=   1065.06KB
> free=   1666.94KB waste=     16.27KB
>                     Metadata capacity=   2152.00KB used=    738.46KB
> free=   1413.54KB waste=     16.27KB
>                   Class data capacity=    580.00KB used=    326.60KB
> free=    253.40KB waste=      0.00KB
>
>
>
Nice, thanks :)


> Updated webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.03/
>
>
All is well. Note that you could reduce the footprint of your change
somewhat by defining a structure like this:

struct numbers_t { size_t cap; size_t used; size_t free; size_t waste; }

and replacing the many members in PrintCLDMetaspaceInfoClosure with
instances of this structure:

numbers_t total;
numbers_t total_class;
numbers_t total_anon;
numbers_t total_anon_class;

Depending on how far you go, your code could deflate quite a bit. Give the
structure a default ctor and your large initializer list goes away; give it
a printing function and you reduce print-summary() to four function calls;
with something like an numbers_t::add(size_t cap, size_t used, size_t free,
size_t waste) you can reduce the additions in
PrintCLDMetaspaceInfoClosure::print_metaspace() to four lines.

Just a suggestion, I leave it up to you if you do this.

Lines 3108 and 3129 miss each a space before BytesPerWord. Change looks
fine otherwise.

Thanks, Thomas


Thanks,

>
> -Zhengyu
>
>
> All else looks fine to me now. I do not need another review.
>>
>> Thanks for your work, this will come in handy!
>>
>> ..Thomas
>>
>> On Tue, Oct 24, 2017 at 5:08 PM, Zhengyu Gu <[hidden email] <mailto:
>> [hidden email]>> wrote:
>>
>>     Hi Thomas,
>>
>>
>>         Not sure whether we misunderstood each other. I was thinking of
>>         something in the line of:
>>
>>         <<<<
>>         ....
>>         ClassLoader: jdk/internal/reflect/DelegatingClassLoader
>>             Metadata:
>>                capacity:     5.00KB used:     3.32KB free:     1.68KB
>>         waste: 0.00KB
>>             Class data:
>>                capacity:     1.00KB used:     0.54KB free:     0.46KB
>>         waste: 0.00KB
>>
>>         ClassLoader: for anonymous class
>>             Metadata:
>>                capacity:     1.00KB used:     1.00KB free:     0.00KB
>>         waste: 0.00KB
>>             Class data:
>>                capacity:     1.00KB used:     0.64KB free:     0.36KB
>>         waste: 0.00KB
>>         ....
>>
>>         Summary:
>>         XX class loaders encountered, total capacity: xx, total waste: xx.
>>
>>         Peak usage by class loader: xxxx
>>           >>>>
>>
>>     Added summary lines:
>>
>>        Total class loaders=    56 capacity=   6378.00KB used=
>>  6205.08KB free=    172.92KB waste=      1.44KB
>>     For anonymous classes=    54 capacity=    212.00KB used=     96.95KB
>>     free=    115.05KB waste=      0.94KB
>>
>>
>>
>>
>>         These numbers would not be interpretation, just a convenience to
>>         the reader to get a clear picture.
>>
>>         It even may be useful to separate the output between basic and
>>         detailed mode, and in basic mode omit all the single class
>>         loaders and just print the summary line.
>>
>>              Updated webrev:
>>         http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>
>>                     <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.01/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>>
>>
>>
>>
>>         metaspace.hpp:
>>
>>         I would have preferred to keep scale_unit file local static
>>         instead of exposing it via MetaspaceAux, because it does not
>>         really have anything to do with metaspace.
>>
>>     Fixed
>>
>>
>>         Otherwise ok.
>>
>>         ---
>>
>>         metaspace.cpp
>>
>>         - ChunkManager::print_statistics(): thanks for reusing this
>>         function!
>>               -> Scale can only be ever 1, K, M, G, yes? So, could you
>>         add an assert at the start of the function, and a comment at the
>>         prototype or function head?
>>               -> Also, now that ChunkManager::print_statistics() takes a
>>         scale, could you please change the printout to use floats like
>>         you did in PrintCLDMetaspaceInfoClosure::print_metaspace()?
>>
>>     Done.
>>
>>
>>     Chunkmanager (non-class):
>>        0 specialized (128 bytes) chunks, total 0.00KB
>>        0 small (512 bytes) chunks, total 0.00KB
>>        0 medium (8192 bytes) chunks, total 0.00KB
>>        0 humongous chunks, total 0.00KB
>>        total size: 0.00KB.
>>     Chunkmanager (class):
>>        0 specialized (128 bytes) chunks, total 0.00KB
>>        0 small (256 bytes) chunks, total 0.00KB
>>        0 medium (4096 bytes) chunks, total 0.00KB
>>        0 humongous chunks, total 0.00KB
>>        total size: 0.00KB.
>>
>>
>>         - I am concerned about races in
>>         PrintCLDMetaspaceInfoClosure::do_cld(ClassLoaderData* cld).
>>         Maybe my understanding is limited. We bail if cld->is_unloading.
>>         But nothing prevents a ClassLoaderDataGraph from starting to
>>         unload a ClassLoaderData and tearing down the Metaspace after we
>>         started printing it in another thread, right? Also, I do not see
>>         any fences.
>>
>>         So, I wonder whether PrintCLDMetaspaceInfoClosure should run in
>>         lock protection. And then, I wonder if it would be good to
>>         separate data gathering and printing. We print to a (at least in
>>         principle) unknown output stream, which may be doing slow File
>>         IO or even Network IO. Doing this under lock protection seems
>>         not a good idea (but I see other places where this happens too).
>>
>>         For an example, see ChunkManager::get_statistic() vs
>>         ChunkManager::print_statistic(). Could you do the same, have a
>>         data gathering step where you collect your
>>         <capacity/used/free/waste> quadrupel in a variable sized list of
>>         structures in memory, and print it out in a separate step? I
>>         realize though that the effort would be larger than for what we
>>         did in ChunkManager::get_statistic(), where we only fill a
>>         structure.
>>
>>
>>     Unlike other NMT queries, this query is executed at a safepoint by
>>     VMThread, so it should be okay.
>>
>>     Updated webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.02/
>>     <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/>
>>
>>     Thanks,
>>
>>     -Zhengyu
>>
>>
>>         ------
>>
>>         vm_operations.hpp
>>
>>         - VM_PrintMetadata : do we still need _unit?
>>
>>
>>              Thanks,
>>
>>              -Zhengyu
>>
>>
>>
>>         Thanks!
>>
>>         Thomas
>>
>>
>>
>>              On 10/23/2017 11:08 AM, Thomas Stüfe wrote:
>>
>>
>>
>>                  On Mon, Oct 23, 2017 at 5:03 PM, Zhengyu Gu
>>         <[hidden email] <mailto:[hidden email]>
>>                  <mailto:[hidden email] <mailto:[hidden email]>>
>>         <mailto:[hidden email] <mailto:[hidden email]>
>>                  <mailto:[hidden email] <mailto:[hidden email]>>>> wrote:
>>
>>
>>
>>                           Okay. So this is important for understanding
>> cases
>>                  where we have
>>                           a large number of miniscule class loaders,
>>         each one
>>                  only having
>>                           a few numbers of chunks. In this case, would it
>> be
>>                  useful to
>>                           have a running total of "free" and "waste",
>>         too? Or
>>                  even better,
>>                           also average and peak values of "free" and
>>         "waste" (to tell
>>                           apart cases where you have one outlier vs
>>         cases where
>>                  just all
>>                           metaspaces waste a lot of memory).
>>                           Just out of curiosity, I looked at
>>         http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>                         <http://cr.openjdk.java.net/~z
>> gu/cld_metaspace/wildfly.txt
>>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>
>>                                         <http://cr.openjdk.java.net/~z
>> gu/cld_metaspace/wildfly.txt
>>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>                         <http://cr.openjdk.java.net/~z
>> gu/cld_metaspace/wildfly.txt
>>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>>
>> and
>>                           it seems that "free" is the problem, not
>>         "waste"? So I
>>                  guess
>>                           what happens is that all the little classloaders
>>                  allocate just
>>                           enough memory to push them over
>>         "_small_chunk_limit=4",
>>                  so they
>>                           allocate the first medium chunk, from which
>>         then they
>>                  take a
>>                           small bite and leave the rest laying around?
>>
>>                       Yes. The free space of anonymous classes should be
>>         counted
>>                  as waste,
>>                       in my opinion. I am not sure if everyone agrees,
>>         so I took the
>>                       summary line out of this patch.
>>
>>                       I would be more than happy to restore the summary
>>         line, if
>>                  you find
>>                       it is useful :-)
>>
>>
>>                  I agree with you. Typically class loaders stop loading
>>         at some
>>                  point, especially these tine ones, and often will not
>>         resume
>>                  loading. So, effectivly, the space is wasted. It still
>>         helps to
>>                  distinguish "free" in the current chunks from "free" in
>> the
>>                  other chunks to tell this situation apart from a
>>         situation where
>>                  we have just a bug in metaspace chunk handling
>>         preventing us to
>>                  use up our chunks.
>>
>>
>>                                    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
>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>                                   <https://bugs.openjdk.java.net
>> /browse/JDK-8166690
>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>>>
>>                                                   <
>> https://bugs.openjdk.java.net/browse/JDK-8166690
>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>                                   <https://bugs.openjdk.java.net
>> /browse/JDK-8166690
>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>>         <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?
>>
>>
>>                                Make sense! Could you recommend what data
>> and
>>                  format you
>>                           would like
>>                                to see?
>>
>>
>>
>>                           Would not
>>         ChunkManager::print_all_chunkmanagers() be
>>                  sufficient?
>>
>>                       Okay, I might need to tweak output format.
>>
>>
>>                  Fine by me. Nothing depends on it yet.
>>
>>                  Thanks, Thomas
>>
>>                       Thanks,
>>
>>                       -Zhengyu
>>
>>
>>
>>                                    -------------
>>
>>                                    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.We do need scale here,
>>         since nmt
>>                  command
>>                           line can set
>>                                    scale and we do
>>
>>                                have to honor that.
>>
>>                                However, passing unit is ugly! I don't
>>         want to
>>                  introduce
>>                           inverse
>>                                dependence (metaspace -> nmt), which is
>>         also ugly.
>>
>>
>>                           Yes, that would be regrettable.
>>
>>                                Probably should move scale mapping code
>> from
>>                  NMTUtil to a
>>                           common util?
>>
>>
>>
>>                           That would be possible, these functions
>>                  (scale_from_name etc)
>>                           are simple enough to be moved to a generic file.
>>                  However, if you
>>                           pass scale, deducing the string that goes with
>>         it is
>>                  trivial and
>>                           I would have no qualms doing this by hand in
>>                  metaspace.cpp. But
>>                           it is a matter of taste.
>>
>>
>>                                    I did not look closely at locking
>>         issues, I assume
>>                                    MetaspaceAux::print_metadata() is
>>         supposed to
>>                  run only in
>>                                    Safepoints?
>>
>>
>>                                No. sum_xxx_xxx_in_use queries are
>>         guarded by locks.
>>
>>                                Thanks,
>>
>>                                -Zhengyu
>>
>>
>>                           Thanks, Thomas
>>
>>
>>
>>                                    Thanks & Kind Regards, Thomas
>>
>>
>>
>>
>>
>>
>>                                    On Fri, Oct 20, 2017 at 4:00 PM,
>>         Zhengyu Gu
>>                           <[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>
>>                  <mailto:[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>>
>>                                    <mailto:[hidden email]
>>         <mailto:[hidden email]> <mailto:[hidden email]
>>         <mailto:[hidden email]>>
>>                  <mailto:[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>>>
>>                           <mailto:[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>
>>                  <mailto:[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>>
>>
>>                                    <mailto:[hidden email]
>>         <mailto:[hidden email]> <mailto:[hidden email]
>>         <mailto:[hidden email]>>
>>                  <mailto:[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>>>>> 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
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>                                   <https://bugs.openjdk.java.net
>> /browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>                                                   <
>> https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>                                   <https://bugs.openjdk.java.net
>> /browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>
>>                                                       <
>> https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>                                   <https://bugs.openjdk.java.net
>> /browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>                                                   <
>> https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>                                   <https://bugs.openjdk.java.net
>> /browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>>
>>                                         Webrev:
>>         http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>                                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>>                                                         <
>> http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>                                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>
>>                                                                     <
>> http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>                                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>>                                                         <
>> http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>                                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>                         <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.00/index.html
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>> >>>>>
>>
>>                                         Test:
>>
>>                                            hotspot_tier1_runtime,
>>         fastdebug and
>>                  release on
>>                           x64 Linux.
>>
>>                                         Thanks,
>>
>>                                         -Zhengyu
>>
>>
>>
>>
>>
>>
>>
>>
Reply | Threaded
Open this post in threaded view
|

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

Thomas Stüfe-2
Hi Zhengyu,

one more thing, your output does not seem to be included in the final NMT
report when the VM shuts down, could this be added too?

Thanks! Thomas

On Wed, Oct 25, 2017 at 7:00 AM, Thomas Stüfe <[hidden email]>
wrote:

> Hi Zhengyu,
>
> On Tue, Oct 24, 2017 at 8:04 PM, Zhengyu Gu <[hidden email]> wrote:
>
>> Hi Thomas,
>>
>> On 10/24/2017 12:08 PM, Thomas Stüfe wrote:
>>
>>> Hi Zhengyu,
>>>
>>> - VM_PrintMetadata still has the _unit member, but I see it nowhere used.
>>>
>>> - I would have split the summary between class and non-class area, that
>>> may have been more useful. But this is a matter of taste, I leave it up to
>>> you.
>>>
>>> Agree, should go little further.
>>
>> Summary:
>>   Total class loaders=   754 capacity=  67528.00KB used=  61216.88KB
>> free=   9453.11KB waste=     38.73KB
>>                     Metadata capacity=  58780.00KB used=  54053.45KB
>> free=   4726.55KB waste=     36.38KB
>>                   Class data capacity=   8748.00KB used=   7163.43KB
>> free=   1584.57KB waste=      2.35KB
>>
>> For anonymous classes=   580 capacity=   2732.00KB used=   1065.06KB
>> free=   1666.94KB waste=     16.27KB
>>                     Metadata capacity=   2152.00KB used=    738.46KB
>> free=   1413.54KB waste=     16.27KB
>>                   Class data capacity=    580.00KB used=    326.60KB
>> free=    253.40KB waste=      0.00KB
>>
>>
>>
> Nice, thanks :)
>
>
>> Updated webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.03/
>>
>>
> All is well. Note that you could reduce the footprint of your change
> somewhat by defining a structure like this:
>
> struct numbers_t { size_t cap; size_t used; size_t free; size_t waste; }
>
> and replacing the many members in PrintCLDMetaspaceInfoClosure with
> instances of this structure:
>
> numbers_t total;
> numbers_t total_class;
> numbers_t total_anon;
> numbers_t total_anon_class;
>
> Depending on how far you go, your code could deflate quite a bit. Give the
> structure a default ctor and your large initializer list goes away; give it
> a printing function and you reduce print-summary() to four function calls;
> with something like an numbers_t::add(size_t cap, size_t used, size_t free,
> size_t waste) you can reduce the additions in PrintCLDMetaspaceInfoClosure::print_metaspace()
> to four lines.
>
> Just a suggestion, I leave it up to you if you do this.
>
> Lines 3108 and 3129 miss each a space before BytesPerWord. Change looks
> fine otherwise.
>
> Thanks, Thomas
>
>
> Thanks,
>>
>> -Zhengyu
>>
>>
>> All else looks fine to me now. I do not need another review.
>>>
>>> Thanks for your work, this will come in handy!
>>>
>>> ..Thomas
>>>
>>> On Tue, Oct 24, 2017 at 5:08 PM, Zhengyu Gu <[hidden email] <mailto:
>>> [hidden email]>> wrote:
>>>
>>>     Hi Thomas,
>>>
>>>
>>>         Not sure whether we misunderstood each other. I was thinking of
>>>         something in the line of:
>>>
>>>         <<<<
>>>         ....
>>>         ClassLoader: jdk/internal/reflect/DelegatingClassLoader
>>>             Metadata:
>>>                capacity:     5.00KB used:     3.32KB free:     1.68KB
>>>         waste: 0.00KB
>>>             Class data:
>>>                capacity:     1.00KB used:     0.54KB free:     0.46KB
>>>         waste: 0.00KB
>>>
>>>         ClassLoader: for anonymous class
>>>             Metadata:
>>>                capacity:     1.00KB used:     1.00KB free:     0.00KB
>>>         waste: 0.00KB
>>>             Class data:
>>>                capacity:     1.00KB used:     0.64KB free:     0.36KB
>>>         waste: 0.00KB
>>>         ....
>>>
>>>         Summary:
>>>         XX class loaders encountered, total capacity: xx, total waste:
>>> xx.
>>>
>>>         Peak usage by class loader: xxxx
>>>           >>>>
>>>
>>>     Added summary lines:
>>>
>>>        Total class loaders=    56 capacity=   6378.00KB used=
>>>  6205.08KB free=    172.92KB waste=      1.44KB
>>>     For anonymous classes=    54 capacity=    212.00KB used=     96.95KB
>>>     free=    115.05KB waste=      0.94KB
>>>
>>>
>>>
>>>
>>>         These numbers would not be interpretation, just a convenience to
>>>         the reader to get a clear picture.
>>>
>>>         It even may be useful to separate the output between basic and
>>>         detailed mode, and in basic mode omit all the single class
>>>         loaders and just print the summary line.
>>>
>>>              Updated webrev:
>>>         http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>
>>>                     <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.01/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>>
>>>
>>>
>>>
>>>         metaspace.hpp:
>>>
>>>         I would have preferred to keep scale_unit file local static
>>>         instead of exposing it via MetaspaceAux, because it does not
>>>         really have anything to do with metaspace.
>>>
>>>     Fixed
>>>
>>>
>>>         Otherwise ok.
>>>
>>>         ---
>>>
>>>         metaspace.cpp
>>>
>>>         - ChunkManager::print_statistics(): thanks for reusing this
>>>         function!
>>>               -> Scale can only be ever 1, K, M, G, yes? So, could you
>>>         add an assert at the start of the function, and a comment at the
>>>         prototype or function head?
>>>               -> Also, now that ChunkManager::print_statistics() takes a
>>>         scale, could you please change the printout to use floats like
>>>         you did in PrintCLDMetaspaceInfoClosure::print_metaspace()?
>>>
>>>     Done.
>>>
>>>
>>>     Chunkmanager (non-class):
>>>        0 specialized (128 bytes) chunks, total 0.00KB
>>>        0 small (512 bytes) chunks, total 0.00KB
>>>        0 medium (8192 bytes) chunks, total 0.00KB
>>>        0 humongous chunks, total 0.00KB
>>>        total size: 0.00KB.
>>>     Chunkmanager (class):
>>>        0 specialized (128 bytes) chunks, total 0.00KB
>>>        0 small (256 bytes) chunks, total 0.00KB
>>>        0 medium (4096 bytes) chunks, total 0.00KB
>>>        0 humongous chunks, total 0.00KB
>>>        total size: 0.00KB.
>>>
>>>
>>>         - I am concerned about races in
>>>         PrintCLDMetaspaceInfoClosure::do_cld(ClassLoaderData* cld).
>>>         Maybe my understanding is limited. We bail if cld->is_unloading.
>>>         But nothing prevents a ClassLoaderDataGraph from starting to
>>>         unload a ClassLoaderData and tearing down the Metaspace after we
>>>         started printing it in another thread, right? Also, I do not see
>>>         any fences.
>>>
>>>         So, I wonder whether PrintCLDMetaspaceInfoClosure should run in
>>>         lock protection. And then, I wonder if it would be good to
>>>         separate data gathering and printing. We print to a (at least in
>>>         principle) unknown output stream, which may be doing slow File
>>>         IO or even Network IO. Doing this under lock protection seems
>>>         not a good idea (but I see other places where this happens too).
>>>
>>>         For an example, see ChunkManager::get_statistic() vs
>>>         ChunkManager::print_statistic(). Could you do the same, have a
>>>         data gathering step where you collect your
>>>         <capacity/used/free/waste> quadrupel in a variable sized list of
>>>         structures in memory, and print it out in a separate step? I
>>>         realize though that the effort would be larger than for what we
>>>         did in ChunkManager::get_statistic(), where we only fill a
>>>         structure.
>>>
>>>
>>>     Unlike other NMT queries, this query is executed at a safepoint by
>>>     VMThread, so it should be okay.
>>>
>>>     Updated webrev: http://cr.openjdk.java.net/~zgu/8189688/webrev.02/
>>>     <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/>
>>>
>>>     Thanks,
>>>
>>>     -Zhengyu
>>>
>>>
>>>         ------
>>>
>>>         vm_operations.hpp
>>>
>>>         - VM_PrintMetadata : do we still need _unit?
>>>
>>>
>>>              Thanks,
>>>
>>>              -Zhengyu
>>>
>>>
>>>
>>>         Thanks!
>>>
>>>         Thomas
>>>
>>>
>>>
>>>              On 10/23/2017 11:08 AM, Thomas Stüfe wrote:
>>>
>>>
>>>
>>>                  On Mon, Oct 23, 2017 at 5:03 PM, Zhengyu Gu
>>>         <[hidden email] <mailto:[hidden email]>
>>>                  <mailto:[hidden email] <mailto:[hidden email]>>
>>>         <mailto:[hidden email] <mailto:[hidden email]>
>>>                  <mailto:[hidden email] <mailto:[hidden email]>>>>
>>> wrote:
>>>
>>>
>>>
>>>                           Okay. So this is important for understanding
>>> cases
>>>                  where we have
>>>                           a large number of miniscule class loaders,
>>>         each one
>>>                  only having
>>>                           a few numbers of chunks. In this case, would
>>> it be
>>>                  useful to
>>>                           have a running total of "free" and "waste",
>>>         too? Or
>>>                  even better,
>>>                           also average and peak values of "free" and
>>>         "waste" (to tell
>>>                           apart cases where you have one outlier vs
>>>         cases where
>>>                  just all
>>>                           metaspaces waste a lot of memory).
>>>                           Just out of curiosity, I looked at
>>>         http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>>                         <http://cr.openjdk.java.net/~z
>>> gu/cld_metaspace/wildfly.txt
>>>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>
>>>                                         <http://cr.openjdk.java.net/~z
>>> gu/cld_metaspace/wildfly.txt
>>>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>>                         <http://cr.openjdk.java.net/~z
>>> gu/cld_metaspace/wildfly.txt
>>>         <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>>
>>> and
>>>                           it seems that "free" is the problem, not
>>>         "waste"? So I
>>>                  guess
>>>                           what happens is that all the little
>>> classloaders
>>>                  allocate just
>>>                           enough memory to push them over
>>>         "_small_chunk_limit=4",
>>>                  so they
>>>                           allocate the first medium chunk, from which
>>>         then they
>>>                  take a
>>>                           small bite and leave the rest laying around?
>>>
>>>                       Yes. The free space of anonymous classes should be
>>>         counted
>>>                  as waste,
>>>                       in my opinion. I am not sure if everyone agrees,
>>>         so I took the
>>>                       summary line out of this patch.
>>>
>>>                       I would be more than happy to restore the summary
>>>         line, if
>>>                  you find
>>>                       it is useful :-)
>>>
>>>
>>>                  I agree with you. Typically class loaders stop loading
>>>         at some
>>>                  point, especially these tine ones, and often will not
>>>         resume
>>>                  loading. So, effectivly, the space is wasted. It still
>>>         helps to
>>>                  distinguish "free" in the current chunks from "free" in
>>> the
>>>                  other chunks to tell this situation apart from a
>>>         situation where
>>>                  we have just a bug in metaspace chunk handling
>>>         preventing us to
>>>                  use up our chunks.
>>>
>>>
>>>                                    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
>>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>>                                   <https://bugs.openjdk.java.net
>>> /browse/JDK-8166690
>>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>>>
>>>                                                   <
>>> https://bugs.openjdk.java.net/browse/JDK-8166690
>>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>>                                   <https://bugs.openjdk.java.net
>>> /browse/JDK-8166690
>>>         <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8166690
>>>         <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?
>>>
>>>
>>>                                Make sense! Could you recommend what data
>>> and
>>>                  format you
>>>                           would like
>>>                                to see?
>>>
>>>
>>>
>>>                           Would not
>>>         ChunkManager::print_all_chunkmanagers() be
>>>                  sufficient?
>>>
>>>                       Okay, I might need to tweak output format.
>>>
>>>
>>>                  Fine by me. Nothing depends on it yet.
>>>
>>>                  Thanks, Thomas
>>>
>>>                       Thanks,
>>>
>>>                       -Zhengyu
>>>
>>>
>>>
>>>                                    -------------
>>>
>>>                                    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.We do need scale here,
>>>         since nmt
>>>                  command
>>>                           line can set
>>>                                    scale and we do
>>>
>>>                                have to honor that.
>>>
>>>                                However, passing unit is ugly! I don't
>>>         want to
>>>                  introduce
>>>                           inverse
>>>                                dependence (metaspace -> nmt), which is
>>>         also ugly.
>>>
>>>
>>>                           Yes, that would be regrettable.
>>>
>>>                                Probably should move scale mapping code
>>> from
>>>                  NMTUtil to a
>>>                           common util?
>>>
>>>
>>>
>>>                           That would be possible, these functions
>>>                  (scale_from_name etc)
>>>                           are simple enough to be moved to a generic
>>> file.
>>>                  However, if you
>>>                           pass scale, deducing the string that goes with
>>>         it is
>>>                  trivial and
>>>                           I would have no qualms doing this by hand in
>>>                  metaspace.cpp. But
>>>                           it is a matter of taste.
>>>
>>>
>>>                                    I did not look closely at locking
>>>         issues, I assume
>>>                                    MetaspaceAux::print_metadata() is
>>>         supposed to
>>>                  run only in
>>>                                    Safepoints?
>>>
>>>
>>>                                No. sum_xxx_xxx_in_use queries are
>>>         guarded by locks.
>>>
>>>                                Thanks,
>>>
>>>                                -Zhengyu
>>>
>>>
>>>                           Thanks, Thomas
>>>
>>>
>>>
>>>                                    Thanks & Kind Regards, Thomas
>>>
>>>
>>>
>>>
>>>
>>>
>>>                                    On Fri, Oct 20, 2017 at 4:00 PM,
>>>         Zhengyu Gu
>>>                           <[hidden email] <mailto:[hidden email]>
>>>         <mailto:[hidden email] <mailto:[hidden email]>>
>>>                  <mailto:[hidden email] <mailto:[hidden email]>
>>>         <mailto:[hidden email] <mailto:[hidden email]>>>
>>>                                    <mailto:[hidden email]
>>>         <mailto:[hidden email]> <mailto:[hidden email]
>>>         <mailto:[hidden email]>>
>>>                  <mailto:[hidden email] <mailto:[hidden email]>
>>>         <mailto:[hidden email] <mailto:[hidden email]>>>>
>>>                           <mailto:[hidden email] <mailto:[hidden email]>
>>>         <mailto:[hidden email] <mailto:[hidden email]>>
>>>                  <mailto:[hidden email] <mailto:[hidden email]>
>>>         <mailto:[hidden email] <mailto:[hidden email]>>>
>>>
>>>                                    <mailto:[hidden email]
>>>         <mailto:[hidden email]> <mailto:[hidden email]
>>>         <mailto:[hidden email]>>
>>>                  <mailto:[hidden email] <mailto:[hidden email]>
>>>         <mailto:[hidden email] <mailto:[hidden email]>>>>>> 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
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>>                                   <https://bugs.openjdk.java.net
>>> /browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>>                                                   <
>>> https://bugs.openjdk.java.net/browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>>                                   <https://bugs.openjdk.java.net
>>> /browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>
>>>                                                       <
>>> https://bugs.openjdk.java.net/browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>>                                   <https://bugs.openjdk.java.net
>>> /browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>>                                                   <
>>> https://bugs.openjdk.java.net/browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>>                                   <https://bugs.openjdk.java.net
>>> /browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>>                  <https://bugs.openjdk.java.net/browse/JDK-8189688
>>>         <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>>
>>>                                         Webrev:
>>>         http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>>                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>>                                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>>                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>>>                                                         <
>>> http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>>                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>>                                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>>                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>> >>>>
>>>                                                                     <
>>> http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>>                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>>                                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>>                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>>>                                                         <
>>> http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>>                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>>                                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>>                         <http://cr.openjdk.java.net/~z
>>> gu/8189688/webrev.00/index.html
>>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>> >>>>>
>>>
>>>                                         Test:
>>>
>>>                                            hotspot_tier1_runtime,
>>>         fastdebug and
>>>                  release on
>>>                           x64 Linux.
>>>
>>>                                         Thanks,
>>>
>>>                                         -Zhengyu
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

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

Zhengyu Gu-2

On 10/26/2017 08:53 AM, Thomas Stüfe wrote:
> Hi Zhengyu,
>
> one more thing, your output does not seem to be included in the final
> NMT report when the VM shuts down, could this be added too?
>
This turns out to be a tricky one.

During VM exit, we are not supposed to request a safepoint, correct?
Without a safepoint, it is unsafe to walk class loaders.

Thanks,

-Zhengyu


> Thanks! Thomas
>
> On Wed, Oct 25, 2017 at 7:00 AM, Thomas Stüfe <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi Zhengyu,
>
>     On Tue, Oct 24, 2017 at 8:04 PM, Zhengyu Gu <[hidden email]
>     <mailto:[hidden email]>> wrote:
>
>         Hi Thomas,
>
>         On 10/24/2017 12:08 PM, Thomas Stüfe wrote:
>
>             Hi Zhengyu,
>
>             - VM_PrintMetadata still has the _unit member, but I see it
>             nowhere used.
>
>             - I would have split the summary between class and non-class
>             area, that may have been more useful. But this is a matter
>             of taste, I leave it up to you.
>
>         Agree, should go little further.
>
>         Summary:
>            Total class loaders=   754 capacity=  67528.00KB used=
>         61216.88KB free=   9453.11KB waste=     38.73KB
>                              Metadata capacity=  58780.00KB used=
>         54053.45KB free=   4726.55KB waste=     36.38KB
>                            Class data capacity=   8748.00KB used=
>           7163.43KB free=   1584.57KB waste=      2.35KB
>
>         For anonymous classes=   580 capacity=   2732.00KB used=
>           1065.06KB free=   1666.94KB waste=     16.27KB
>                              Metadata capacity=   2152.00KB used=  
>         738.46KB free=   1413.54KB waste=     16.27KB
>                            Class data capacity=    580.00KB used=  
>         326.60KB free=    253.40KB waste=      0.00KB
>
>
>
>     Nice, thanks :)
>
>         Updated webrev:
>         http://cr.openjdk.java.net/~zgu/8189688/webrev.03/
>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.03/>
>
>
>     All is well. Note that you could reduce the footprint of your change
>     somewhat by defining a structure like this:
>
>     struct numbers_t { size_t cap; size_t used; size_t free; size_t waste; }
>
>     and replacing the many members in PrintCLDMetaspaceInfoClosure with
>     instances of this structure:
>
>     numbers_t total;
>     numbers_t total_class;
>     numbers_t total_anon;
>     numbers_t total_anon_class;
>     Depending on how far you go, your code could deflate quite a bit.
>     Give the structure a default ctor and your large initializer list
>     goes away; give it a printing function and you reduce
>     print-summary() to four function calls; with something like an
>     numbers_t::add(size_t cap, size_t used, size_t free, size_t waste)
>     you can reduce the additions in
>     PrintCLDMetaspaceInfoClosure::print_metaspace() to four lines.
>
>     Just a suggestion, I leave it up to you if you do this.
>
>     Lines 3108 and 3129 miss each a space before BytesPerWord. Change
>     looks fine otherwise.
>
>     Thanks, Thomas
>
>
>         Thanks,
>
>         -Zhengyu
>
>
>             All else looks fine to me now. I do not need another review.
>
>             Thanks for your work, this will come in handy!
>
>             ..Thomas
>
>             On Tue, Oct 24, 2017 at 5:08 PM, Zhengyu Gu <[hidden email]
>             <mailto:[hidden email]> <mailto:[hidden email]
>             <mailto:[hidden email]>>> wrote:
>
>                  Hi Thomas,
>
>
>                      Not sure whether we misunderstood each other. I was
>             thinking of
>                      something in the line of:
>
>                      <<<<
>                      ....
>                      ClassLoader: jdk/internal/reflect/DelegatingClassLoader
>                          Metadata:
>                             capacity:     5.00KB used:     3.32KB free:
>                 1.68KB
>                      waste: 0.00KB
>                          Class data:
>                             capacity:     1.00KB used:     0.54KB free:
>                 0.46KB
>                      waste: 0.00KB
>
>                      ClassLoader: for anonymous class
>                          Metadata:
>                             capacity:     1.00KB used:     1.00KB free:
>                 0.00KB
>                      waste: 0.00KB
>                          Class data:
>                             capacity:     1.00KB used:     0.64KB free:
>                 0.36KB
>                      waste: 0.00KB
>                      ....
>
>                      Summary:
>                      XX class loaders encountered, total capacity: xx,
>             total waste: xx.
>
>                      Peak usage by class loader: xxxx
>                        >>>>
>
>                  Added summary lines:
>
>                     Total class loaders=    56 capacity=   6378.00KB
>             used=       6205.08KB free=    172.92KB waste=      1.44KB
>                  For anonymous classes=    54 capacity=    212.00KB
>             used=     96.95KB
>                  free=    115.05KB waste=      0.94KB
>
>
>
>
>                      These numbers would not be interpretation, just a
>             convenience to
>                      the reader to get a clear picture.
>
>                      It even may be useful to separate the output
>             between basic and
>                      detailed mode, and in basic mode omit all the
>             single class
>                      loaders and just print the summary line.
>
>                           Updated webrev:
>             http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>>
>                                
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>>>
>
>
>
>                      metaspace.hpp:
>
>                      I would have preferred to keep scale_unit file
>             local static
>                      instead of exposing it via MetaspaceAux, because it
>             does not
>                      really have anything to do with metaspace.
>
>                  Fixed
>
>
>                      Otherwise ok.
>
>                      ---
>
>                      metaspace.cpp
>
>                      - ChunkManager::print_statistics(): thanks for
>             reusing this
>                      function!
>                            -> Scale can only be ever 1, K, M, G, yes?
>             So, could you
>                      add an assert at the start of the function, and a
>             comment at the
>                      prototype or function head?
>                            -> Also, now that
>             ChunkManager::print_statistics() takes a
>                      scale, could you please change the printout to use
>             floats like
>                      you did in
>             PrintCLDMetaspaceInfoClosure::print_metaspace()?
>
>                  Done.
>
>
>                  Chunkmanager (non-class):
>                     0 specialized (128 bytes) chunks, total 0.00KB
>                     0 small (512 bytes) chunks, total 0.00KB
>                     0 medium (8192 bytes) chunks, total 0.00KB
>                     0 humongous chunks, total 0.00KB
>                     total size: 0.00KB.
>                  Chunkmanager (class):
>                     0 specialized (128 bytes) chunks, total 0.00KB
>                     0 small (256 bytes) chunks, total 0.00KB
>                     0 medium (4096 bytes) chunks, total 0.00KB
>                     0 humongous chunks, total 0.00KB
>                     total size: 0.00KB.
>
>
>                      - I am concerned about races in
>                    
>             PrintCLDMetaspaceInfoClosure::do_cld(ClassLoaderData* cld).
>                      Maybe my understanding is limited. We bail if
>             cld->is_unloading.
>                      But nothing prevents a ClassLoaderDataGraph from
>             starting to
>                      unload a ClassLoaderData and tearing down the
>             Metaspace after we
>                      started printing it in another thread, right? Also,
>             I do not see
>                      any fences.
>
>                      So, I wonder whether PrintCLDMetaspaceInfoClosure
>             should run in
>                      lock protection. And then, I wonder if it would be
>             good to
>                      separate data gathering and printing. We print to a
>             (at least in
>                      principle) unknown output stream, which may be
>             doing slow File
>                      IO or even Network IO. Doing this under lock
>             protection seems
>                      not a good idea (but I see other places where this
>             happens too).
>
>                      For an example, see ChunkManager::get_statistic() vs
>                      ChunkManager::print_statistic(). Could you do the
>             same, have a
>                      data gathering step where you collect your
>                      <capacity/used/free/waste> quadrupel in a variable
>             sized list of
>                      structures in memory, and print it out in a
>             separate step? I
>                      realize though that the effort would be larger than
>             for what we
>                      did in ChunkManager::get_statistic(), where we only
>             fill a
>                      structure.
>
>
>                  Unlike other NMT queries, this query is executed at a
>             safepoint by
>                  VMThread, so it should be okay.
>
>                  Updated webrev:
>             http://cr.openjdk.java.net/~zgu/8189688/webrev.02/
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/>
>                  <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/>>
>
>                  Thanks,
>
>                  -Zhengyu
>
>
>                      ------
>
>                      vm_operations.hpp
>
>                      - VM_PrintMetadata : do we still need _unit?
>
>
>                           Thanks,
>
>                           -Zhengyu
>
>
>
>                      Thanks!
>
>                      Thomas
>
>
>
>                           On 10/23/2017 11:08 AM, Thomas Stüfe wrote:
>
>
>
>                               On Mon, Oct 23, 2017 at 5:03 PM, Zhengyu Gu
>                      <[hidden email] <mailto:[hidden email]>
>             <mailto:[hidden email] <mailto:[hidden email]>>
>                               <mailto:[hidden email]
>             <mailto:[hidden email]> <mailto:[hidden email]
>             <mailto:[hidden email]>>>
>                      <mailto:[hidden email] <mailto:[hidden email]>
>             <mailto:[hidden email] <mailto:[hidden email]>>
>                               <mailto:[hidden email]
>             <mailto:[hidden email]> <mailto:[hidden email]
>             <mailto:[hidden email]>>>>> wrote:
>
>
>
>                                        Okay. So this is important for
>             understanding cases
>                               where we have
>                                        a large number of miniscule class
>             loaders,
>                      each one
>                               only having
>                                        a few numbers of chunks. In this
>             case, would it be
>                               useful to
>                                        have a running total of "free"
>             and "waste",
>                      too? Or
>                               even better,
>                                        also average and peak values of
>             "free" and
>                      "waste" (to tell
>                                        apart cases where you have one
>             outlier vs
>                      cases where
>                               just all
>                                        metaspaces waste a lot of memory).
>                                        Just out of curiosity, I looked at
>             http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>                    
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>
>                                    
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>                    
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>>
>                                                    
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>                    
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>
>                                    
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>                    
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>>>
>             and
>                                        it seems that "free" is the
>             problem, not
>                      "waste"? So I
>                               guess
>                                        what happens is that all the
>             little classloaders
>                               allocate just
>                                        enough memory to push them over
>                      "_small_chunk_limit=4",
>                               so they
>                                        allocate the first medium chunk,
>             from which
>                      then they
>                               take a
>                                        small bite and leave the rest
>             laying around?
>
>                                    Yes. The free space of anonymous
>             classes should be
>                      counted
>                               as waste,
>                                    in my opinion. I am not sure if
>             everyone agrees,
>                      so I took the
>                                    summary line out of this patch.
>
>                                    I would be more than happy to restore
>             the summary
>                      line, if
>                               you find
>                                    it is useful :-)
>
>
>                               I agree with you. Typically class loaders
>             stop loading
>                      at some
>                               point, especially these tine ones, and
>             often will not
>                      resume
>                               loading. So, effectivly, the space is
>             wasted. It still
>                      helps to
>                               distinguish "free" in the current chunks
>             from "free" in the
>                               other chunks to tell this situation apart
>             from a
>                      situation where
>                               we have just a bug in metaspace chunk handling
>                      preventing us to
>                               use up our chunks.
>
>
>                                                 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
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>>
>                                              
>             <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>>>
>                                                              
>             <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>>
>                                              
>             <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>             <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?
>
>
>                                             Make sense! Could you
>             recommend what data and
>                               format you
>                                        would like
>                                             to see?
>
>
>
>                                        Would not
>                      ChunkManager::print_all_chunkmanagers() be
>                               sufficient?
>
>                                    Okay, I might need to tweak output
>             format.
>
>
>                               Fine by me. Nothing depends on it yet.
>
>                               Thanks, Thomas
>
>                                    Thanks,
>
>                                    -Zhengyu
>
>
>
>                                                 -------------
>
>                                                 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.We do need
>             scale here,
>                      since nmt
>                               command
>                                        line can set
>                                                 scale and we do
>
>                                             have to honor that.
>
>                                             However, passing unit is
>             ugly! I don't
>                      want to
>                               introduce
>                                        inverse
>                                             dependence (metaspace ->
>             nmt), which is
>                      also ugly.
>
>
>                                        Yes, that would be regrettable.
>
>                                             Probably should move scale
>             mapping code from
>                               NMTUtil to a
>                                        common util?
>
>
>
>                                        That would be possible, these
>             functions
>                               (scale_from_name etc)
>                                        are simple enough to be moved to
>             a generic file.
>                               However, if you
>                                        pass scale, deducing the string
>             that goes with
>                      it is
>                               trivial and
>                                        I would have no qualms doing this
>             by hand in
>                               metaspace.cpp. But
>                                        it is a matter of taste.
>
>
>                                                 I did not look closely
>             at locking
>                      issues, I assume
>                                              
>               MetaspaceAux::print_metadata() is
>                      supposed to
>                               run only in
>                                                 Safepoints?
>
>
>                                             No. sum_xxx_xxx_in_use
>             queries are
>                      guarded by locks.
>
>                                             Thanks,
>
>                                             -Zhengyu
>
>
>                                        Thanks, Thomas
>
>
>
>                                                 Thanks & Kind Regards,
>             Thomas
>
>
>
>
>
>
>                                                 On Fri, Oct 20, 2017 at
>             4:00 PM,
>                      Zhengyu Gu
>                                        <[hidden email]
>             <mailto:[hidden email]> <mailto:[hidden email]
>             <mailto:[hidden email]>>
>                      <mailto:[hidden email] <mailto:[hidden email]>
>             <mailto:[hidden email] <mailto:[hidden email]>>>
>                               <mailto:[hidden email]
>             <mailto:[hidden email]> <mailto:[hidden email]
>             <mailto:[hidden email]>>
>                      <mailto:[hidden email] <mailto:[hidden email]>
>             <mailto:[hidden email] <mailto:[hidden email]>>>>
>                                                 <mailto:[hidden email]
>             <mailto:[hidden email]>
>                      <mailto:[hidden email] <mailto:[hidden email]>>
>             <mailto:[hidden email] <mailto:[hidden email]>
>                      <mailto:[hidden email] <mailto:[hidden email]>>>
>                               <mailto:[hidden email]
>             <mailto:[hidden email]> <mailto:[hidden email]
>             <mailto:[hidden email]>>
>                      <mailto:[hidden email] <mailto:[hidden email]>
>             <mailto:[hidden email] <mailto:[hidden email]>>>>>
>                                        <mailto:[hidden email]
>             <mailto:[hidden email]> <mailto:[hidden email]
>             <mailto:[hidden email]>>
>                      <mailto:[hidden email] <mailto:[hidden email]>
>             <mailto:[hidden email] <mailto:[hidden email]>>>
>                               <mailto:[hidden email]
>             <mailto:[hidden email]> <mailto:[hidden email]
>             <mailto:[hidden email]>>
>                      <mailto:[hidden email] <mailto:[hidden email]>
>             <mailto:[hidden email] <mailto:[hidden email]>>>>
>
>                                                 <mailto:[hidden email]
>             <mailto:[hidden email]>
>                      <mailto:[hidden email] <mailto:[hidden email]>>
>             <mailto:[hidden email] <mailto:[hidden email]>
>                      <mailto:[hidden email] <mailto:[hidden email]>>>
>                               <mailto:[hidden email]
>             <mailto:[hidden email]> <mailto:[hidden email]
>             <mailto:[hidden email]>>
>                      <mailto:[hidden email] <mailto:[hidden email]>
>             <mailto:[hidden email] <mailto:[hidden email]>>>>>>> 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
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>                                              
>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>
>                                                              
>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>                                              
>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>>
>                                                                  
>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>                                              
>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>
>                                                              
>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>                                              
>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>                            
>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>>>
>                                                      Webrev:
>             http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>                                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>
>                                                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>                                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>>
>                                                                        
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>                                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>
>                                                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>                                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>                                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>                    
>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>>>
>
>                                                      Test:
>
>                                                      
>               hotspot_tier1_runtime,
>                      fastdebug and
>                               release on
>                                        x64 Linux.
>
>                                                      Thanks,
>
>                                                      -Zhengyu
>
>
>
>
>
>
>
>
>
Reply | Threaded
Open this post in threaded view
|

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

Thomas Stüfe-2
On Fri, Oct 27, 2017 at 10:34 PM, Zhengyu Gu <[hidden email]> wrote:

>
> On 10/26/2017 08:53 AM, Thomas Stüfe wrote:
>
>> Hi Zhengyu,
>>
>> one more thing, your output does not seem to be included in the final NMT
>> report when the VM shuts down, could this be added too?
>>
>> This turns out to be a tricky one.
>
> During VM exit, we are not supposed to request a safepoint, correct?
> Without a safepoint, it is unsafe to walk class loaders.
>
> Thanks,
>
> -Zhengyu
>
>
Yes, this is not trivial. I hacked it in (because I wanted to see your
output at the end of my tests) and had to disable the assert. I never ran
into problems. Should java threads not all be stopped at that point?

But this also can be done in a follow up issue.

Thanks, Thomas


>
> Thanks! Thomas
>>
>> On Wed, Oct 25, 2017 at 7:00 AM, Thomas Stüfe <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>>     Hi Zhengyu,
>>
>>     On Tue, Oct 24, 2017 at 8:04 PM, Zhengyu Gu <[hidden email]
>>     <mailto:[hidden email]>> wrote:
>>
>>         Hi Thomas,
>>
>>         On 10/24/2017 12:08 PM, Thomas Stüfe wrote:
>>
>>             Hi Zhengyu,
>>
>>             - VM_PrintMetadata still has the _unit member, but I see it
>>             nowhere used.
>>
>>             - I would have split the summary between class and non-class
>>             area, that may have been more useful. But this is a matter
>>             of taste, I leave it up to you.
>>
>>         Agree, should go little further.
>>
>>         Summary:
>>            Total class loaders=   754 capacity=  67528.00KB used=
>>  61216.88KB free=   9453.11KB waste=     38.73KB
>>                              Metadata capacity=  58780.00KB used=
>>  54053.45KB free=   4726.55KB waste=     36.38KB
>>                            Class data capacity=   8748.00KB used=
>>    7163.43KB free=   1584.57KB waste=      2.35KB
>>
>>         For anonymous classes=   580 capacity=   2732.00KB used=
>>  1065.06KB free=   1666.94KB waste=     16.27KB
>>                              Metadata capacity=   2152.00KB used=
>>    738.46KB free=   1413.54KB waste=     16.27KB
>>                            Class data capacity=    580.00KB used=
>>    326.60KB free=    253.40KB waste=      0.00KB
>>
>>
>>
>>     Nice, thanks :)
>>
>>         Updated webrev:
>>         http://cr.openjdk.java.net/~zgu/8189688/webrev.03/
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.03/>
>>
>>
>>     All is well. Note that you could reduce the footprint of your change
>>     somewhat by defining a structure like this:
>>
>>     struct numbers_t { size_t cap; size_t used; size_t free; size_t
>> waste; }
>>
>>     and replacing the many members in PrintCLDMetaspaceInfoClosure with
>>     instances of this structure:
>>
>>     numbers_t total;
>>     numbers_t total_class;
>>     numbers_t total_anon;
>>     numbers_t total_anon_class;
>>     Depending on how far you go, your code could deflate quite a bit.
>>     Give the structure a default ctor and your large initializer list
>>     goes away; give it a printing function and you reduce
>>     print-summary() to four function calls; with something like an
>>     numbers_t::add(size_t cap, size_t used, size_t free, size_t waste)
>>     you can reduce the additions in
>>     PrintCLDMetaspaceInfoClosure::print_metaspace() to four lines.
>>
>>     Just a suggestion, I leave it up to you if you do this.
>>
>>     Lines 3108 and 3129 miss each a space before BytesPerWord. Change
>>     looks fine otherwise.
>>
>>     Thanks, Thomas
>>
>>
>>         Thanks,
>>
>>         -Zhengyu
>>
>>
>>             All else looks fine to me now. I do not need another review.
>>
>>             Thanks for your work, this will come in handy!
>>
>>             ..Thomas
>>
>>             On Tue, Oct 24, 2017 at 5:08 PM, Zhengyu Gu <[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>> wrote:
>>
>>                  Hi Thomas,
>>
>>
>>                      Not sure whether we misunderstood each other. I was
>>             thinking of
>>                      something in the line of:
>>
>>                      <<<<
>>                      ....
>>                      ClassLoader: jdk/internal/reflect/Delegatin
>> gClassLoader
>>                          Metadata:
>>                             capacity:     5.00KB used:     3.32KB free:
>>                1.68KB
>>                      waste: 0.00KB
>>                          Class data:
>>                             capacity:     1.00KB used:     0.54KB free:
>>                0.46KB
>>                      waste: 0.00KB
>>
>>                      ClassLoader: for anonymous class
>>                          Metadata:
>>                             capacity:     1.00KB used:     1.00KB free:
>>                0.00KB
>>                      waste: 0.00KB
>>                          Class data:
>>                             capacity:     1.00KB used:     0.64KB free:
>>                0.36KB
>>                      waste: 0.00KB
>>                      ....
>>
>>                      Summary:
>>                      XX class loaders encountered, total capacity: xx,
>>             total waste: xx.
>>
>>                      Peak usage by class loader: xxxx
>>                        >>>>
>>
>>                  Added summary lines:
>>
>>                     Total class loaders=    56 capacity=   6378.00KB
>>             used=       6205.08KB free=    172.92KB waste=      1.44KB
>>                  For anonymous classes=    54 capacity=    212.00KB
>>             used=     96.95KB
>>                  free=    115.05KB waste=      0.94KB
>>
>>
>>
>>
>>                      These numbers would not be interpretation, just a
>>             convenience to
>>                      the reader to get a clear picture.
>>
>>                      It even may be useful to separate the output
>>             between basic and
>>                      detailed mode, and in basic mode omit all the
>>             single class
>>                      loaders and just print the summary line.
>>
>>                           Updated webrev:
>>             http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html
>>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html
>> >
>>                                 <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.01/index.html <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.01/index.html>>
>>                                             <
>> http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html <
>> http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>
>>                                 <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.01/index.html <http://cr.openjdk.java.net/~z
>> gu/8189688/webrev.01/index.html>>>
>>
>>
>>
>>                      metaspace.hpp:
>>
>>                      I would have preferred to keep scale_unit file
>>             local static
>>                      instead of exposing it via MetaspaceAux, because it
>>             does not
>>                      really have anything to do with metaspace.
>>
>>                  Fixed
>>
>>
>>                      Otherwise ok.
>>
>>                      ---
>>
>>                      metaspace.cpp
>>
>>                      - ChunkManager::print_statistics(): thanks for
>>             reusing this
>>                      function!
>>                            -> Scale can only be ever 1, K, M, G, yes?
>>             So, could you
>>                      add an assert at the start of the function, and a
>>             comment at the
>>                      prototype or function head?
>>                            -> Also, now that
>>             ChunkManager::print_statistics() takes a
>>                      scale, could you please change the printout to use
>>             floats like
>>                      you did in
>>             PrintCLDMetaspaceInfoClosure::print_metaspace()?
>>
>>                  Done.
>>
>>
>>                  Chunkmanager (non-class):
>>                     0 specialized (128 bytes) chunks, total 0.00KB
>>                     0 small (512 bytes) chunks, total 0.00KB
>>                     0 medium (8192 bytes) chunks, total 0.00KB
>>                     0 humongous chunks, total 0.00KB
>>                     total size: 0.00KB.
>>                  Chunkmanager (class):
>>                     0 specialized (128 bytes) chunks, total 0.00KB
>>                     0 small (256 bytes) chunks, total 0.00KB
>>                     0 medium (4096 bytes) chunks, total 0.00KB
>>                     0 humongous chunks, total 0.00KB
>>                     total size: 0.00KB.
>>
>>
>>                      - I am concerned about races in
>>                                 PrintCLDMetaspaceInfoClosure::do_cld(ClassLoaderData*
>> cld).
>>                      Maybe my understanding is limited. We bail if
>>             cld->is_unloading.
>>                      But nothing prevents a ClassLoaderDataGraph from
>>             starting to
>>                      unload a ClassLoaderData and tearing down the
>>             Metaspace after we
>>                      started printing it in another thread, right? Also,
>>             I do not see
>>                      any fences.
>>
>>                      So, I wonder whether PrintCLDMetaspaceInfoClosure
>>             should run in
>>                      lock protection. And then, I wonder if it would be
>>             good to
>>                      separate data gathering and printing. We print to a
>>             (at least in
>>                      principle) unknown output stream, which may be
>>             doing slow File
>>                      IO or even Network IO. Doing this under lock
>>             protection seems
>>                      not a good idea (but I see other places where this
>>             happens too).
>>
>>                      For an example, see ChunkManager::get_statistic() vs
>>                      ChunkManager::print_statistic(). Could you do the
>>             same, have a
>>                      data gathering step where you collect your
>>                      <capacity/used/free/waste> quadrupel in a variable
>>             sized list of
>>                      structures in memory, and print it out in a
>>             separate step? I
>>                      realize though that the effort would be larger than
>>             for what we
>>                      did in ChunkManager::get_statistic(), where we only
>>             fill a
>>                      structure.
>>
>>
>>                  Unlike other NMT queries, this query is executed at a
>>             safepoint by
>>                  VMThread, so it should be okay.
>>
>>                  Updated webrev:
>>             http://cr.openjdk.java.net/~zgu/8189688/webrev.02/
>>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/>
>>                  <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/
>>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/>>
>>
>>                  Thanks,
>>
>>                  -Zhengyu
>>
>>
>>                      ------
>>
>>                      vm_operations.hpp
>>
>>                      - VM_PrintMetadata : do we still need _unit?
>>
>>
>>                           Thanks,
>>
>>                           -Zhengyu
>>
>>
>>
>>                      Thanks!
>>
>>                      Thomas
>>
>>
>>
>>                           On 10/23/2017 11:08 AM, Thomas Stüfe wrote:
>>
>>
>>
>>                               On Mon, Oct 23, 2017 at 5:03 PM, Zhengyu Gu
>>                      <[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>>>> wrote:
>>
>>
>>
>>                                        Okay. So this is important for
>>             understanding cases
>>                               where we have
>>                                        a large number of miniscule class
>>             loaders,
>>                      each one
>>                               only having
>>                                        a few numbers of chunks. In this
>>             case, would it be
>>                               useful to
>>                                        have a running total of "free"
>>             and "waste",
>>                      too? Or
>>                               even better,
>>                                        also average and peak values of
>>             "free" and
>>                      "waste" (to tell
>>                                        apart cases where you have one
>>             outlier vs
>>                      cases where
>>                               just all
>>                                        metaspaces waste a lot of memory).
>>                                        Just out of curiosity, I looked at
>>             http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>                                 <http://cr.openjdk.java.net/~z
>> gu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>
>>                                                 <
>> http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>                                 <http://cr.openjdk.java.net/~z
>> gu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>>
>>                                                                 <
>> http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>                                 <http://cr.openjdk.java.net/~z
>> gu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>
>>                                                 <
>> http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>                                 <http://cr.openjdk.java.net/~z
>> gu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>>>
>>            and
>>                                        it seems that "free" is the
>>             problem, not
>>                      "waste"? So I
>>                               guess
>>                                        what happens is that all the
>>             little classloaders
>>                               allocate just
>>                                        enough memory to push them over
>>                      "_small_chunk_limit=4",
>>                               so they
>>                                        allocate the first medium chunk,
>>             from which
>>                      then they
>>                               take a
>>                                        small bite and leave the rest
>>             laying around?
>>
>>                                    Yes. The free space of anonymous
>>             classes should be
>>                      counted
>>                               as waste,
>>                                    in my opinion. I am not sure if
>>             everyone agrees,
>>                      so I took the
>>                                    summary line out of this patch.
>>
>>                                    I would be more than happy to restore
>>             the summary
>>                      line, if
>>                               you find
>>                                    it is useful :-)
>>
>>
>>                               I agree with you. Typically class loaders
>>             stop loading
>>                      at some
>>                               point, especially these tine ones, and
>>             often will not
>>                      resume
>>                               loading. So, effectivly, the space is
>>             wasted. It still
>>                      helps to
>>                               distinguish "free" in the current chunks
>>             from "free" in the
>>                               other chunks to tell this situation apart
>>             from a
>>                      situation where
>>                               we have just a bug in metaspace chunk
>> handling
>>                      preventing us to
>>                               use up our chunks.
>>
>>
>>                                                 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
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>                                           <https://bugs.openjdk.java.net
>> /browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>>
>>                                                           <
>> https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>                                           <https://bugs.openjdk.java.net
>> /browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>>>
>>
>> <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>                                           <https://bugs.openjdk.java.net
>> /browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>>
>>                                                           <
>> https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>                                           <https://bugs.openjdk.java.net
>> /browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <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?
>>
>>
>>                                             Make sense! Could you
>>             recommend what data and
>>                               format you
>>                                        would like
>>                                             to see?
>>
>>
>>
>>                                        Would not
>>                      ChunkManager::print_all_chunkmanagers() be
>>                               sufficient?
>>
>>                                    Okay, I might need to tweak output
>>             format.
>>
>>
>>                               Fine by me. Nothing depends on it yet.
>>
>>                               Thanks, Thomas
>>
>>                                    Thanks,
>>
>>                                    -Zhengyu
>>
>>
>>
>>                                                 -------------
>>
>>                                                 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.We do need
>>             scale here,
>>                      since nmt
>>                               command
>>                                        line can set
>>                                                 scale and we do
>>
>>                                             have to honor that.
>>
>>                                             However, passing unit is
>>             ugly! I don't
>>                      want to
>>                               introduce
>>                                        inverse
>>                                             dependence (metaspace ->
>>             nmt), which is
>>                      also ugly.
>>
>>
>>                                        Yes, that would be regrettable.
>>
>>                                             Probably should move scale
>>             mapping code from
>>                               NMTUtil to a
>>                                        common util?
>>
>>
>>
>>                                        That would be possible, these
>>             functions
>>                               (scale_from_name etc)
>>                                        are simple enough to be moved to
>>             a generic file.
>>                               However, if you
>>                                        pass scale, deducing the string
>>             that goes with
>>                      it is
>>                               trivial and
>>                                        I would have no qualms doing this
>>             by hand in
>>                               metaspace.cpp. But
>>                                        it is a matter of taste.
>>
>>
>>                                                 I did not look closely
>>             at locking
>>                      issues, I assume
>>
>> MetaspaceAux::print_metadata() is
>>                      supposed to
>>                               run only in
>>                                                 Safepoints?
>>
>>
>>                                             No. sum_xxx_xxx_in_use
>>             queries are
>>                      guarded by locks.
>>
>>                                             Thanks,
>>
>>                                             -Zhengyu
>>
>>
>>                                        Thanks, Thomas
>>
>>
>>
>>                                                 Thanks & Kind Regards,
>>             Thomas
>>
>>
>>
>>
>>
>>
>>                                                 On Fri, Oct 20, 2017 at
>>             4:00 PM,
>>                      Zhengyu Gu
>>                                        <[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>>
>>                                                 <mailto:[hidden email]
>>             <mailto:[hidden email]>
>>                      <mailto:[hidden email] <mailto:[hidden email]>>
>>             <mailto:[hidden email] <mailto:[hidden email]>
>>                      <mailto:[hidden email] <mailto:[hidden email]>>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>>>
>>                                        <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>>
>>
>>                                                 <mailto:[hidden email]
>>             <mailto:[hidden email]>
>>                      <mailto:[hidden email] <mailto:[hidden email]>>
>>             <mailto:[hidden email] <mailto:[hidden email]>
>>                      <mailto:[hidden email] <mailto:[hidden email]>>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>>>>> 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
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>                                           <https://bugs.openjdk.java.net
>> /browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>                                                           <
>> https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>                                           <https://bugs.openjdk.java.net
>> /browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>
>>
>> <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>                                           <https://bugs.openjdk.java.net
>> /browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>                                                           <
>> https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>                                           <https://bugs.openjdk.java.net
>> /browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>>
>>
>>     <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>
>
Reply | Threaded
Open this post in threaded view
|

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

David Holmes
In reply to this post by Zhengyu Gu-2
On 28/10/2017 6:34 AM, Zhengyu Gu wrote:

>
> On 10/26/2017 08:53 AM, Thomas Stüfe wrote:
>> Hi Zhengyu,
>>
>> one more thing, your output does not seem to be included in the final
>> NMT report when the VM shuts down, could this be added too?
>>
> This turns out to be a tricky one.
>
> During VM exit, we are not supposed to request a safepoint, correct?

VM exit uses a safepoint - can you use it do gather the data?

David

> Without a safepoint, it is unsafe to walk class loaders.
>
> Thanks,
>
> -Zhengyu
>
>
>> Thanks! Thomas
>>
>> On Wed, Oct 25, 2017 at 7:00 AM, Thomas Stüfe <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>>     Hi Zhengyu,
>>
>>     On Tue, Oct 24, 2017 at 8:04 PM, Zhengyu Gu <[hidden email]
>>     <mailto:[hidden email]>> wrote:
>>
>>         Hi Thomas,
>>
>>         On 10/24/2017 12:08 PM, Thomas Stüfe wrote:
>>
>>             Hi Zhengyu,
>>
>>             - VM_PrintMetadata still has the _unit member, but I see it
>>             nowhere used.
>>
>>             - I would have split the summary between class and non-class
>>             area, that may have been more useful. But this is a matter
>>             of taste, I leave it up to you.
>>
>>         Agree, should go little further.
>>
>>         Summary:
>>            Total class loaders=   754 capacity=  67528.00KB used=
>>         61216.88KB free=   9453.11KB waste=     38.73KB
>>                              Metadata capacity=  58780.00KB used=
>>         54053.45KB free=   4726.55KB waste=     36.38KB
>>                            Class data capacity=   8748.00KB used=
>>           7163.43KB free=   1584.57KB waste=      2.35KB
>>
>>         For anonymous classes=   580 capacity=   2732.00KB used=
>>           1065.06KB free=   1666.94KB waste=     16.27KB
>>                              Metadata capacity=   2152.00KB used=
>>         738.46KB free=   1413.54KB waste=     16.27KB
>>                            Class data capacity=    580.00KB used=
>>         326.60KB free=    253.40KB waste=      0.00KB
>>
>>
>>
>>     Nice, thanks :)
>>
>>         Updated webrev:
>>         http://cr.openjdk.java.net/~zgu/8189688/webrev.03/
>>         <http://cr.openjdk.java.net/~zgu/8189688/webrev.03/>
>>
>>
>>     All is well. Note that you could reduce the footprint of your change
>>     somewhat by defining a structure like this:
>>
>>     struct numbers_t { size_t cap; size_t used; size_t free; size_t
>> waste; }
>>
>>     and replacing the many members in PrintCLDMetaspaceInfoClosure with
>>     instances of this structure:
>>
>>     numbers_t total;
>>     numbers_t total_class;
>>     numbers_t total_anon;
>>     numbers_t total_anon_class;
>>     Depending on how far you go, your code could deflate quite a bit.
>>     Give the structure a default ctor and your large initializer list
>>     goes away; give it a printing function and you reduce
>>     print-summary() to four function calls; with something like an
>>     numbers_t::add(size_t cap, size_t used, size_t free, size_t waste)
>>     you can reduce the additions in
>>     PrintCLDMetaspaceInfoClosure::print_metaspace() to four lines.
>>
>>     Just a suggestion, I leave it up to you if you do this.
>>
>>     Lines 3108 and 3129 miss each a space before BytesPerWord. Change
>>     looks fine otherwise.
>>
>>     Thanks, Thomas
>>
>>
>>         Thanks,
>>
>>         -Zhengyu
>>
>>
>>             All else looks fine to me now. I do not need another review.
>>
>>             Thanks for your work, this will come in handy!
>>
>>             ..Thomas
>>
>>             On Tue, Oct 24, 2017 at 5:08 PM, Zhengyu Gu <[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>> wrote:
>>
>>                  Hi Thomas,
>>
>>
>>                      Not sure whether we misunderstood each other. I was
>>             thinking of
>>                      something in the line of:
>>
>>                      <<<<
>>                      ....
>>                      ClassLoader:
>> jdk/internal/reflect/DelegatingClassLoader
>>                          Metadata:
>>                             capacity:     5.00KB used:     3.32KB
>> free:                 1.68KB
>>                      waste: 0.00KB
>>                          Class data:
>>                             capacity:     1.00KB used:     0.54KB
>> free:                 0.46KB
>>                      waste: 0.00KB
>>
>>                      ClassLoader: for anonymous class
>>                          Metadata:
>>                             capacity:     1.00KB used:     1.00KB
>> free:                 0.00KB
>>                      waste: 0.00KB
>>                          Class data:
>>                             capacity:     1.00KB used:     0.64KB
>> free:                 0.36KB
>>                      waste: 0.00KB
>>                      ....
>>
>>                      Summary:
>>                      XX class loaders encountered, total capacity: xx,
>>             total waste: xx.
>>
>>                      Peak usage by class loader: xxxx
>>                        >>>>
>>
>>                  Added summary lines:
>>
>>                     Total class loaders=    56 capacity=   6378.00KB
>>             used=       6205.08KB free=    172.92KB waste=      1.44KB
>>                  For anonymous classes=    54 capacity=    212.00KB
>>             used=     96.95KB
>>                  free=    115.05KB waste=      0.94KB
>>
>>
>>
>>
>>                      These numbers would not be interpretation, just a
>>             convenience to
>>                      the reader to get a clear picture.
>>
>>                      It even may be useful to separate the output
>>             between basic and
>>                      detailed mode, and in basic mode omit all the
>>             single class
>>                      loaders and just print the summary line.
>>
>>                           Updated webrev:
>>             http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.01/index.html>>>
>>
>>
>>
>>                      metaspace.hpp:
>>
>>                      I would have preferred to keep scale_unit file
>>             local static
>>                      instead of exposing it via MetaspaceAux, because it
>>             does not
>>                      really have anything to do with metaspace.
>>
>>                  Fixed
>>
>>
>>                      Otherwise ok.
>>
>>                      ---
>>
>>                      metaspace.cpp
>>
>>                      - ChunkManager::print_statistics(): thanks for
>>             reusing this
>>                      function!
>>                            -> Scale can only be ever 1, K, M, G, yes?
>>             So, could you
>>                      add an assert at the start of the function, and a
>>             comment at the
>>                      prototype or function head?
>>                            -> Also, now that
>>             ChunkManager::print_statistics() takes a
>>                      scale, could you please change the printout to use
>>             floats like
>>                      you did in
>>             PrintCLDMetaspaceInfoClosure::print_metaspace()?
>>
>>                  Done.
>>
>>
>>                  Chunkmanager (non-class):
>>                     0 specialized (128 bytes) chunks, total 0.00KB
>>                     0 small (512 bytes) chunks, total 0.00KB
>>                     0 medium (8192 bytes) chunks, total 0.00KB
>>                     0 humongous chunks, total 0.00KB
>>                     total size: 0.00KB.
>>                  Chunkmanager (class):
>>                     0 specialized (128 bytes) chunks, total 0.00KB
>>                     0 small (256 bytes) chunks, total 0.00KB
>>                     0 medium (4096 bytes) chunks, total 0.00KB
>>                     0 humongous chunks, total 0.00KB
>>                     total size: 0.00KB.
>>
>>
>>                      - I am concerned about races in
>>             PrintCLDMetaspaceInfoClosure::do_cld(ClassLoaderData* cld).
>>                      Maybe my understanding is limited. We bail if
>>             cld->is_unloading.
>>                      But nothing prevents a ClassLoaderDataGraph from
>>             starting to
>>                      unload a ClassLoaderData and tearing down the
>>             Metaspace after we
>>                      started printing it in another thread, right? Also,
>>             I do not see
>>                      any fences.
>>
>>                      So, I wonder whether PrintCLDMetaspaceInfoClosure
>>             should run in
>>                      lock protection. And then, I wonder if it would be
>>             good to
>>                      separate data gathering and printing. We print to a
>>             (at least in
>>                      principle) unknown output stream, which may be
>>             doing slow File
>>                      IO or even Network IO. Doing this under lock
>>             protection seems
>>                      not a good idea (but I see other places where this
>>             happens too).
>>
>>                      For an example, see ChunkManager::get_statistic() vs
>>                      ChunkManager::print_statistic(). Could you do the
>>             same, have a
>>                      data gathering step where you collect your
>>                      <capacity/used/free/waste> quadrupel in a variable
>>             sized list of
>>                      structures in memory, and print it out in a
>>             separate step? I
>>                      realize though that the effort would be larger than
>>             for what we
>>                      did in ChunkManager::get_statistic(), where we only
>>             fill a
>>                      structure.
>>
>>
>>                  Unlike other NMT queries, this query is executed at a
>>             safepoint by
>>                  VMThread, so it should be okay.
>>
>>                  Updated webrev:
>>             http://cr.openjdk.java.net/~zgu/8189688/webrev.02/
>>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/>
>>                  <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/
>>             <http://cr.openjdk.java.net/~zgu/8189688/webrev.02/>>
>>
>>                  Thanks,
>>
>>                  -Zhengyu
>>
>>
>>                      ------
>>
>>                      vm_operations.hpp
>>
>>                      - VM_PrintMetadata : do we still need _unit?
>>
>>
>>                           Thanks,
>>
>>                           -Zhengyu
>>
>>
>>
>>                      Thanks!
>>
>>                      Thomas
>>
>>
>>
>>                           On 10/23/2017 11:08 AM, Thomas Stüfe wrote:
>>
>>
>>
>>                               On Mon, Oct 23, 2017 at 5:03 PM, Zhengyu Gu
>>                      <[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>>>> wrote:
>>
>>
>>
>>                                        Okay. So this is important for
>>             understanding cases
>>                               where we have
>>                                        a large number of miniscule class
>>             loaders,
>>                      each one
>>                               only having
>>                                        a few numbers of chunks. In this
>>             case, would it be
>>                               useful to
>>                                        have a running total of "free"
>>             and "waste",
>>                      too? Or
>>                               even better,
>>                                        also average and peak values of
>>             "free" and
>>                      "waste" (to tell
>>                                        apart cases where you have one
>>             outlier vs
>>                      cases where
>>                               just all
>>                                        metaspaces waste a lot of memory).
>>                                        Just out of curiosity, I looked at
>>             http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>>
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>
>>             <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt
>>            
>> <http://cr.openjdk.java.net/~zgu/cld_metaspace/wildfly.txt>>>>
>>             and
>>                                        it seems that "free" is the
>>             problem, not
>>                      "waste"? So I
>>                               guess
>>                                        what happens is that all the
>>             little classloaders
>>                               allocate just
>>                                        enough memory to push them over
>>                      "_small_chunk_limit=4",
>>                               so they
>>                                        allocate the first medium chunk,
>>             from which
>>                      then they
>>                               take a
>>                                        small bite and leave the rest
>>             laying around?
>>
>>                                    Yes. The free space of anonymous
>>             classes should be
>>                      counted
>>                               as waste,
>>                                    in my opinion. I am not sure if
>>             everyone agrees,
>>                      so I took the
>>                                    summary line out of this patch.
>>
>>                                    I would be more than happy to restore
>>             the summary
>>                      line, if
>>                               you find
>>                                    it is useful :-)
>>
>>
>>                               I agree with you. Typically class loaders
>>             stop loading
>>                      at some
>>                               point, especially these tine ones, and
>>             often will not
>>                      resume
>>                               loading. So, effectivly, the space is
>>             wasted. It still
>>                      helps to
>>                               distinguish "free" in the current chunks
>>             from "free" in the
>>                               other chunks to tell this situation apart
>>             from a
>>                      situation where
>>                               we have just a bug in metaspace chunk
>> handling
>>                      preventing us to
>>                               use up our chunks.
>>
>>
>>                                                 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
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>>
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>>>
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>>
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <https://bugs.openjdk.java.net/browse/JDK-8166690>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8166690
>>             <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?
>>
>>
>>                                             Make sense! Could you
>>             recommend what data and
>>                               format you
>>                                        would like
>>                                             to see?
>>
>>
>>
>>                                        Would not
>>                      ChunkManager::print_all_chunkmanagers() be
>>                               sufficient?
>>
>>                                    Okay, I might need to tweak output
>>             format.
>>
>>
>>                               Fine by me. Nothing depends on it yet.
>>
>>                               Thanks, Thomas
>>
>>                                    Thanks,
>>
>>                                    -Zhengyu
>>
>>
>>
>>                                                 -------------
>>
>>                                                 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.We do need
>>             scale here,
>>                      since nmt
>>                               command
>>                                        line can set
>>                                                 scale and we do
>>
>>                                             have to honor that.
>>
>>                                             However, passing unit is
>>             ugly! I don't
>>                      want to
>>                               introduce
>>                                        inverse
>>                                             dependence (metaspace ->
>>             nmt), which is
>>                      also ugly.
>>
>>
>>                                        Yes, that would be regrettable.
>>
>>                                             Probably should move scale
>>             mapping code from
>>                               NMTUtil to a
>>                                        common util?
>>
>>
>>
>>                                        That would be possible, these
>>             functions
>>                               (scale_from_name etc)
>>                                        are simple enough to be moved to
>>             a generic file.
>>                               However, if you
>>                                        pass scale, deducing the string
>>             that goes with
>>                      it is
>>                               trivial and
>>                                        I would have no qualms doing this
>>             by hand in
>>                               metaspace.cpp. But
>>                                        it is a matter of taste.
>>
>>
>>                                                 I did not look closely
>>             at locking
>>                      issues, I assume
>>               MetaspaceAux::print_metadata() is
>>                      supposed to
>>                               run only in
>>                                                 Safepoints?
>>
>>
>>                                             No. sum_xxx_xxx_in_use
>>             queries are
>>                      guarded by locks.
>>
>>                                             Thanks,
>>
>>                                             -Zhengyu
>>
>>
>>                                        Thanks, Thomas
>>
>>
>>
>>                                                 Thanks & Kind Regards,
>>             Thomas
>>
>>
>>
>>
>>
>>
>>                                                 On Fri, Oct 20, 2017 at
>>             4:00 PM,
>>                      Zhengyu Gu
>>                                        <[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>>
>>                                                 <mailto:[hidden email]
>>             <mailto:[hidden email]>
>>                      <mailto:[hidden email] <mailto:[hidden email]>>
>>             <mailto:[hidden email] <mailto:[hidden email]>
>>                      <mailto:[hidden email] <mailto:[hidden email]>>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>>>
>>                                        <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>>
>>
>>                                                 <mailto:[hidden email]
>>             <mailto:[hidden email]>
>>                      <mailto:[hidden email] <mailto:[hidden email]>>
>>             <mailto:[hidden email] <mailto:[hidden email]>
>>                      <mailto:[hidden email] <mailto:[hidden email]>>>
>>                               <mailto:[hidden email]
>>             <mailto:[hidden email]> <mailto:[hidden email]
>>             <mailto:[hidden email]>>
>>                      <mailto:[hidden email] <mailto:[hidden email]>
>>             <mailto:[hidden email] <mailto:[hidden email]>>>>>>> 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
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>>
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>
>>               <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8189688
>>             <https://bugs.openjdk.java.net/browse/JDK-8189688>>>>>>
>>                                                      Webrev:
>>             http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>
>>            
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~zgu/8189688/webrev.00/index.html>>>>>>
>>
>>                                                      Test:
>>
>>               hotspot_tier1_runtime,
>>                      fastdebug and
>>                               release on
>>                                        x64 Linux.
>>
>>                                                      Thanks,
>>
>>                                                      -Zhengyu
>>
>>
>>
>>
>>
>>
>>
>>
>>
12