ZGC: RFR: 8207756: jstat should show CGC STW phases of ZGC

[Per Liden]

Hi,

On 07/27/2018 04:31 PM, Yasumasa Suenaga wrote:
> Hi Erik, Per,
> 
> Thank you for your comment.
> I uploaded new webrev. It has CGC counter in ZServiceability.
> 
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8207756/webrev.01/
> 
> I know ZGC has three STW phases. They are called by VM_ZOperation. So I 
> had added CGC counter in it at first. However I agree with you if 
> ZServiceability should have it.

In general, I think jstat is in such a bad state that I'm hoping we long 
term can either deprecate/remove it or replace it with a better tool. As 
you know, jstat just makes too many assumptions and is a really bad fit 
for concurrent collectors in general. Anyway, let's just play along and 
try to make jstat happy for now.

I took the liberty of making some cleanups and adjustments to your patch.

http://cr.openjdk.java.net/~pliden/8207756/webrev.0

I'm currently running it through our testing.

cheers,
Per

> 
> 
> Yasumasa
> 
> 
> On 2018/07/26 23:19, Erik Österlund wrote:
>> Hi Yasumasa,
>>
>> There is only one global instance of CollectorCounters. Do we really 
>> have to pass it around into constructors and dump it in ZDriver?
>> I don't see why it would be in any way problematic to just put this 
>> global instance of counters in ZServiceabilityCounters. And put the 
>> TraceCollectorStats tcs(_cgc_counters); stuff in 
>> ZServiceabilityCountersTracer or something like that.
>>
>> Also I saw this: _cgc_counters = new CollectorCounters("Z 
>> stop-the-world phases", 2);
>>
>> Does this assume there are two pauses for a ZGC collection cycle? 
>> There are at least 3, but possibly more:
>>
>> 1) Mark start
>> 2) Mark end (this could recur multiple times due to unlucky races with 
>> mutators resurrecting weak references right before hitting mark end, 
>> causing a new concurrent phase if a large object graph was reachable 
>> from this weak ref). So we really can't know for sure how many pauses 
>> there will be.
>> 3) Relocate start (done after reference processing and concurrent 
>> selection of relocaiton set)
>>
>> Thanks,
>> /Erik
>>
>> On 2018-07-26 15:44, Yasumasa Suenaga wrote:
>>> CC'ing to hotspot-gc-dev
>>>
>>> Hi Per,
>>>
>>> I've looked at ZServiceabilityCounters in zServiceability.cpp .
>>> IMHO it is unsuitable because CGC counter means STW in concurrent GC. 
>>> So I think it is suitable to implement in VMOperation for ZGC.
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> On 2018/07/26 22:30, Per Lidén wrote:
>>>> Hi,
>>>>
>>>> Please have a look at the zServiceability class, which is where code 
>>>> like this is intended to live.
>>>>
>>>> /Per
>>>>
>>>>> On 26 Jul 2018, at 14:30, Yasumasa Suenaga <yasuenag at gmail.com> wrote:
>>>>>
>>>>> PING: Could you review it?
>>>>>
>>>>>>    webrev: 
>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8207756/webrev.00/
>>>>>
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>> On 2018/07/18 11:59, Yasumasa Suenaga wrote:
>>>>>> Hi all,
>>>>>> Please review this change:
>>>>>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8207756
>>>>>>    webrev: 
>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8207756/webrev.00/
>>>>>> jstat shows CGC STW phases as CGC and CGCT fields since JDK-8153333 .
>>>>>> However, ZGC does not adapt to it yet as following:
>>>>>> ```
>>>>>> $ jstat -gc 1234 1 1
>>>>>>   S0C S1C S0U S1U EC EU OC OU MC MU CCSC CCSU YGC YGCT FGC FGCT 
>>>>>> CGC CGCT GCT
>>>>>>    - - - - - - 514048.0 12288.0 4352.0 3731.2 0.0 0.0 - - - - - - 
>>>>>> 0.000
>>>>>> ```
>>>>>> I uploaded webrev which is for jdk/jdk repo. Please tell me if it
>>>>>> should be for zgc repo.
>>>>>> I'm not an author of ZGC. So I need a sponsor.
>>>>>> (I'm a Reviewer of jdk. If this change should be pushed to jdk/jdk,
>>>>>> please tell me. I will push it after reviewing.)
>>>>>> Thanks,
>>>>>> Yasumasa
>>>>
>>