RFR (XS) 8222818: NMT summary could show the GC in use
Eric Caspole
eric.caspole at oracle.com
Mon Apr 29 22:07:17 UTC 2019
Hi everybody,
I got busy with other minor emergencies and lost track of this
discussion about what the gc names should be and where it should go, so
I'll cancel this PR and let someone else take it up later if there is
interest.
Eric
On 4/24/19 05:25, Stefan Karlsson wrote:
> On 2019-04-24 11:18, David Holmes wrote:
>> Hi Stefan,
>>
>> On 24/04/2019 6:47 pm, Stefan Karlsson wrote:
>>>
>>>
>>> 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?
>>
>> I was thinking more simply that we might rename hs_err_name() to
>> name() and make the strings a little nicer e.g. "G1 GC" instead of "g1
>> gc". But if you want to add more general GC functionality here that's
>> up to you :)
>
> Either way works for me.
>
> We tried to change the name before, but it was met with resistance:
> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2018-April/021937.html
>
>
> StefanK
>
>>
>> Cheers,
>> David
>>
>>> 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/
More information about the hotspot-runtime-dev
mailing list