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

[Erik Österlund]

Hi Per,

Looks reasonable.

Thanks,
/Erik

On 2018-08-07 12:36, Per Liden wrote:
> 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
>>>>>
>>>