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

[Eric Caspole]

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.

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/