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

[Stefan Karlsson]

On 2019-04-24 00:17, David Holmes wrote:
> 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.


FYI, the name of the function hs_err_name() was chosen to deter people 
from using it in other places. Maybe it's time to add another function 
in GCConfig that return better names? Maybe use names that match our 
-XX:Use<Name>GC flags?

If you would find that useful, I've created a patch that exposes three 
new functions that return the flag names, or parts of it:
http://cr.openjdk.java.net/~stefank/8222818/gcFlagNames/webrev.01/

GCConfig::flag_name() gives:
  UseConcMarkSweepGC
  UseEpsilonGC
  UseG1GC
  UseParallelGC
  UseSerialGC
  UseShenandoahGC
  UseZGC

GCConfig::flag_name_no_use() gives:
  ConcMarkSweepGC
  EpsilonGC
  G1GC
  ParallelGC
  SerialGC
  ShenandoahGC
  ZGC

GCConfig::flag_name_no_use_no_gc() gives:
  ConcMarkSweep
  Epsilon
  G1
  Parallel
  Serial
  Shenandoah
  Z

The universe.cpp changes, are only temporary changes to test the output.

StefanK

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