RFR(s): 8222015: Small VM.metaspace improvements

[Thomas Stüfe]

Thank you Jiangli!

Your recommendations make sense, I'll work them into the next webrev.

Best Regards, Thomas

On Mon, Apr 8, 2019 at 5:30 AM Jiangli Zhou <jianglizhou at google.com> wrote:

> Hi Thomas,
>
> This seems good to me. I have a few minor suggestions below, but please
> feel free to keep your existing code without changing.
>
> - For consistency with the existing code and VM.metaspace output, it might
> be worth renaming _num_classes_cds to _num_classes_shared, and
> _num_classes_cds_by_spacetype to _num_classes_shared_by_spacetype.
>
> - src/hotspot/share/memory/metaspace/printCLDMetaspaceInfoClosure.cpp
> You could replace the following MetaspaceShared::is_in_shared_metaspace(k)
> call with k->is_shared() if 'k' is guaranteed to be a valid Klass.
>
>   58   void do_klass(Klass* k) {  59     _num_classes ++;  60     if (MetaspaceShared::is_in_shared_metaspace(k)) {  61       _num_classes_cds ++;  62     }  63   }
>
> - src/hotspot/share/memory/metaspace/printMetaspaceInfoKlassClosure.cpp
>
>   46   // Print a 's' for shared classes  47   _out->put(MetaspaceShared::is_in_shared_metaspace(k) ? 's': ' ');  48
>
> Same suggestion as the above.
>
> Thanks and regards,
> Jiangli
>
>
> On Fri, Apr 5, 2019 at 3:07 AM Thomas Stüfe <thomas.stuefe at gmail.com>
> wrote:
>
>> Hi all,
>>
>> may I have please a review for this collection of small improvements to
>> the
>> VM.metaspace diagnostic command?
>>
>> - it clearly marks now classes whose metadata reside in cds
>> - it shows the number of classes loaded, incl. those from cds, in the
>> overviews too.
>>
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8222015
>> cr:
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8222015--small-vm.metaspace-improvements/webrev.00/webrev/
>>
>> Example output:
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8222015--small-vm.metaspace-improvements/example-by-spacetype.txt
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8222015--small-vm.metaspace-improvements/example-showloaders.txt
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8222015--small-vm.metaspace-improvements/example-showloaders-showclasses.txt
>> (scroll
>> down -> cds classes in <bootstrap> are now marked with 's')
>>
>> Thank you,
>>
>> Thomas
>>
>