RFR(S) 8193184: NMT: Report array class count in NMT summary

[Zhengyu Gu]

Hi Coleen,

Yumin also reviewed, Please find attached patch.

Thanks for sponsoring!

-Zhengyu



On 02/05/2018 11:29 PM, yumin qi wrote:
> Hi, Zhengyu
> 
>    Looks good.
> 
> Thanks
> Yumin
> (open id: minqi)
> 
> 
> On Mon, Feb 5, 2018 at 5:45 PM, Zhengyu Gu <zgu at redhat.com 
> <mailto:zgu at redhat.com>> wrote:
> 
> 
>     Updated: http://cr.openjdk.java.net/~zgu/8193184/webrev.02/
>     <http://cr.openjdk.java.net/~zgu/8193184/webrev.02/>
> 
>     Thanks,
> 
>     -Zhengyu
> 
> 
>     On 02/05/2018 05:17 PM, coleen.phillimore at oracle.com
>     <mailto:coleen.phillimore at oracle.com> wrote:
> 
> 
>         Yes, this looks good.  A couple of small issues.  Where you
>         include classLoaderData.inline.hpp, you don't have to include
>         classLoaderData.hpp because it's already included in the
>         inlne.hpp file.
>         Also, you are supposed to put the Oracle copyright at the top
>         before the RedHat one in the test.
> 
>         When you get another reviewer, I'll sponsor it for you.
>         thanks!
>         Coleen
> 
>         On 2/5/18 5:11 PM, Zhengyu Gu wrote:
> 
>             Hi Coleen,
> 
>             Thanks for the review and suggestions.
> 
>             Webrev is updated according to your comments.
> 
>             Webrev:
>             http://cr.openjdk.java.net/~zgu/8193184/webrev.01/index.html
>             <http://cr.openjdk.java.net/~zgu/8193184/webrev.01/index.html>
> 
>             Reran hotspot_runtime test on Linux 64 (fastdebug and release)
> 
>             -Zhengyu
> 
> 
>             On 02/05/2018 01:15 PM, coleen.phillimore at oracle.com
>             <mailto:coleen.phillimore at oracle.com> wrote:
> 
> 
>                 Hi Zhengyu,
> 
>                 This looks really good.  The instanceKlass count does
>                 seem to belong to ClassLoaderData.
> 
>                 http://cr.openjdk.java.net/~zgu/8193184/webrev.00/src/hotspot/share/classfile/classLoaderData.hpp.udiff.html
>                 <http://cr.openjdk.java.net/~zgu/8193184/webrev.00/src/hotspot/share/classfile/classLoaderData.hpp.udiff.html>
> 
> 
>                 + static void inc_instance_classes(size_t count) {
>                 + Atomic::add(count, &_num_instance_classes);
>                 + }
>                 +
>                 + static void dec_instance_classes(size_t count) {
>                 + assert(count <= _num_instance_classes, "Sanity");
>                 + Atomic::sub(count, &_num_instance_classes);
>                 + }
>                 +
>                 + static void inc_array_classes(size_t count) {
>                 + Atomic::add(count, &_num_array_classes);
>                 + }
>                 +
>                 + static void dec_array_classes(size_t count) {
>                 + assert(count <= _num_array_classes, "Sanity");
>                 + Atomic::sub(count, &_num_array_classes);
>                 + }
> 
> 
>                 Can you move these functions to classLoaderData.inline.hpp?
> 
>                 Maybe you should add an NMT test for this to verify the
>                 output?
> 
>                 Thanks,
>                 Coleen
> 
> 
>                 On 2/2/18 3:28 PM, Zhengyu Gu wrote:
> 
>                     Please review this patch that adds array class count
>                     to NMT report, so the class count can closely match
>                     the output from GC.class_stats.
> 
>                     For example:
>                     Summary:
>                     -                     Class (reserved=1069781KB,
>                     committed=1069269KB)
>                                                  (classes #3362)
>                                                  (  instance classes#
>                     3105, array classes#257)
> 
>                     Summary diff:
>                     -                     Class (reserved=1071843KB
>                     +2061KB, committed=1070051KB +781KB)
>                                                  (classes #3405 +43)
>                                                  (  instance classes#
>                     3143 +38, array classes#262 +5)
> 
>                     Bug:
>                     https://bugs.openjdk.java.net/browse/JDK-8193184
>                     <https://bugs.openjdk.java.net/browse/JDK-8193184>
>                     Webrev:
>                     http://cr.openjdk.java.net/~zgu/8193184/webrev.00/
>                     <http://cr.openjdk.java.net/~zgu/8193184/webrev.00/>
> 
>                     Test:
>                        hotspot_tier1_runtime on Linux 64 (fastdebug and
>                     release)
> 
>                     Thanks,
> 
>                     -Zhengyu
> 
> 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 8193184.patch
Type: text/x-patch
Size: 19689 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20180206/940f91ee/8193184-0001.patch>