RFR (XS) 8222818: NMT summary could show the GC in use

[David Holmes]

On 24/04/2019 6:50 am, Zhengyu Gu wrote:
> 
> 
> On 4/23/19 2:53 PM, Eric Caspole wrote:
>> Hi Zhengyu,
>> Hopefully this email comes through in monospace, the alignment is OK 
>> for me:
>>
>>
>> currently:
>>
>> -                        GC (reserved=379056KB, committed=93220KB)
>>                              (malloc=39184KB #2159)
>>                              (mmap: reserved=339872KB, committed=54036KB)
>>
>>
>> My version:
>>
>>
>> -                GC - g1 gc (reserved=379090KB, committed=93254KB)
>>                              (malloc=39218KB #2194)
>>                              (mmap: reserved=339872KB, committed=54036KB)
>>
>>
>> so it is aligned going to the left off the parenthesis like the 
>> current version. Is that what you mean? I like the way the GC stands 
>> out like this but it is OK to put it in the parentheses on the right.
> 
> Different GC has different name, it is hard to get them all aligned 
> right, and it does not worth the effort.

AFAICS The code already "reserves" 26 characters just to print "GC", 
which is right-aligned. So all this does is take some of the 24 existing 
spaces and fill them in with the GC name so you end up with:

                 GC - g1 gc (reserved=379056KB, committed=93220KB)
                            (malloc=39184KB #2159)
                            (mmap: reserved=339872KB, committed=54036KB)

or
         GC - shenandoah gc (reserved=379056KB, committed=93220KB)
                            (malloc=39184KB #2159)
                            (mmap: reserved=339872KB, committed=54036KB)

etc. Only nit is that 26 seems to small for "concurrent mark sweep gc". 
Also the alignment of 26 could be specified dynamically based on the 
length of the hs_err_name() string if needed.

David
-----

> 
> So, my suggestion is to place GC name inside parentheses, and you don't 
> have to deal with indents to the left.
> 
> e.g.
> 
> 
> -                     GC (reserved=379056KB, committed=93220KB by g1 gc)
>                            (malloc=39184KB #2159)
>                            (mmap: reserved=339872KB, committed=54036KB)
> 
> 
> Thanks,
> 
> -Zhengyu
> 
>>
>> Thanks,
>> Eric
>>
>>
>>
>> On 4/22/19 21:57, Zhengyu Gu wrote:
>>>
>>>
>>> On 4/22/19 8:19 PM, David Holmes wrote:
>>>> Hi Eric,
>>>>
>>>> On 23/04/2019 8:13 am, Eric Caspole wrote:
>>>>> Hi, could I have reviews and any opinions on this little change to 
>>>>> show the GC name in the NMT output, as this helps us to more easily 
>>>>> triage performance data.
>>>>
>>>> The idea seems fine.
>>>
>>>>
>>>> For the implementation wouldn't it be simpler to do something like:
>>>>
>>>> if (flag == mtGC) {
>>>>    out->print("%s - %s (", NMTUtil::flag_to_name(flag),
>>>>                            GCConfig::hs_err_name());
>>>> } else {
>>>>    out->print("-%26s (", NMTUtil::flag_to_name(flag));
>>>> }
>>>>
>>> Yes, this is simpler.
>>>
>>> I don't like where the name is placed, it screws up section 
>>> alignments. I would prefer to place name inside parenthesis. e.g.
>>>
>>> - GC (g1 gc reserved=379056KB, committed=93220KB)
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>>> and skip the need for a local buffer and snprintf?
>>>>
>>>> Aside: it's probably used in enough different contexts that 
>>>> GCConfig::hs_err_name should be renamed.
>>>>
>>>> Also if the VM terminates during initialization is it possible for 
>>>> this code to be executed before the GCConfig has been setup? And if 
>>>> so how will it behave?
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> This passed tier 1 and 2.
>>>>> Thanks,
>>>>> Eric
>>>>>
>>>>>
>>>>> JBS:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8222818
>>>>>
>>>>> webrev:
>>>>> http://cr.openjdk.java.net/~ecaspole/JDK-8222818/02/webrev/