PING: RFR: JDK-8153333: [REDO] STW phases at Concurrent GC should count in PerfCounter

Tue Feb 6 13:33:26 UTC 2018

Hi Stefan,

> This looks good to me, will do some more testing while waiting for a second reviewer and I can sponsor the change once it's ready to go.

Thanks! I'm waiting for second reviewer.

>> What should I do to get CSR approve?
> In the bug system under "More" you can choose "Create CSR" which is the first step. More information can be found on the wiki:
> https://wiki.openjdk.java.net/display/csr/CSR+FAQs

I filed new CSR:
   https://bugs.openjdk.java.net/browse/JDK-8196862

Yasumasa

On 2018/02/06 21:55, Stefan Johansson wrote:
> 
> 
> On 2018-02-06 06:10, Yasumasa Suenaga wrote:
>> Hi Stefan,
>>
>>> I agree, for G1 this should not be controlled. Maybe I was a bit unclear, I
>>> was wondering why we want to control it for CMS.
>> I said to remove -XX:EnableConcGCPerfCounter in two years ago. I've
>> missed it :-)
>>
>>    http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2016-March/017125.html
>>
>> So I uploaded new webrev. This change includes copyright year updates.
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.06/
> Thanks Yasumasa,
> 
> This looks good to me, will do some more testing while waiting for a second reviewer and I can sponsor the change once it's ready to go.
>>
>> This change passes all tests on submit repo, and
>> :hotspot_serviceability :jdk_tools tests on my laptop.
>>
>>    http://java.se.oracle.com:10065/mdash/jobs/mach5-one-ysuenaga-JDK-8153333-20180206-0222-10428
>>
>>
>>> If we do the change for CMS, we should
>>> probably also do a CSR, but that should be fairly straight forward.
>> What should I do to get CSR approve?
> In the bug system under "More" you can choose "Create CSR" which is the first step. More information can be found on the wiki:
> https://wiki.openjdk.java.net/display/csr/CSR+FAQs
> 
> Cheers,
> Stefan
> 
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> 2018-02-06 0:33 GMT+09:00 Stefan Johansson <stefan.johansson at oracle.com>:
>>>
>>> On 2018-02-03 06:40, Yasumasa Suenaga wrote:
>>>> On 2018/02/02 23:38, Stefan Johansson wrote:
>>>>> Hi Yasumasa,
>>>>>
>>>>> The changes doesn't apply clean on the latest jdk/hs, can you provide an
>>>>> updated webrev?
>>>>
>>>> I uploaded webrev for jdk-hs:
>>>>    cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.05/
>>>>
>>> Thanks, I've kicked off a testing job now to verify nothing unexpected
>>> fails.
>>>>
>>>>> The testing done by the submit repo doesn't cover the tests you have
>>>>> update so I plan to take the change for a spin and make sure the correct
>>>>> tests are run and verified in Mach 5.
>>>>
>>>> I've also tested hotspot/jtreg/:hotspot_serviceability and jdk/:jdk_tools
>>>> on my laptop.
>>>> I did not see any errors / failures which are related to this change.
>>> I also ran some local tests on this and it looks good.
>>>>
>>>>
>>>>> Also a question about the change. Why do we need a special flag for CMS?
>>>>> I see that the original bug report refers to the flag as being a way to turn
>>>>> on and off the feature but the current implementation only consider the flag
>>>>> for CMS.
>>>>
>>>>
>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2016-March/016774.html
>>>>
>>>> Originally, STW phases (Remark and Cleanup) at G1 are not counted in jstat
>>>> FGC column.
>>>> So I think we need not to control the behavior of PerfCounter for G1.
>>>>
>>> I agree, for G1 this should not be controlled. Maybe I was a bit unclear, I
>>> was wondering why we want to control it for CMS. I think either we should
>>> change the behavior without guarding it by a flag or just skip updating CMS
>>> (and leave the pauses in FGC). If we do the change for CMS, we should
>>> probably also do a CSR, but that should be fairly straight forward.
>>>
>>> I also found the old review thread where Jon M had the same comment
>>> (removing the flag) and it looks like all agreed on that:
>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2016-March/017118.html
>>>
>>> Thanks,
>>> Stefan
>>>
>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>> Thanks,
>>>>> Stefan
>>>>>
>>>>> On 2018-02-01 14:58, Yasumasa Suenaga wrote:
>>>>>> PING: Could you review and sponsor it?
>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.04/
>>>>>>
>>>>>> This change has been passed Mach 5 via submit repo:
>>>>>>
>>>>>> http://java.se.oracle.com:10065/mdash/jobs/mach5-one-ysuenaga-JDK-8153333-20180201-0805-10101
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2017/11/01 22:02, Yasumasa Suenaga wrote:
>>>>>>> PING: Could you review and sponsor it?
>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.04/
>>>>>>>
>>>>>>> Also I need JPRT results of this change.
>>>>>>> Could you cooperate?
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2017/09/27 0:08, Yasumasa Suenaga wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> I uploaded new webrev to be adapted to jdk10/hs:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.04/
>>>>>>>>
>>>>>>>> I want to check this patch via JPRT, but I cannot access it.
>>>>>>>> Could you cooperate?
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2017/09/21 7:46, Yasumasa Suenaga wrote:
>>>>>>>>> PING:
>>>>>>>>>
>>>>>>>>> Have you checked this issue?
>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.03/hotspot/
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.03/jdk/
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2017/07/01 23:44, Yasumasa Suenaga wrote:
>>>>>>>>>> PING:
>>>>>>>>>>
>>>>>>>>>> Have you checked this issue?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2017/06/14 13:22, Yasumasa Suenaga wrote:
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> I changed PerfCounter to show CGC STW phase in jstat in
>>>>>>>>>>> JDK-8151674.
>>>>>>>>>>> However, it occurred several jtreg test failure, so it was
>>>>>>>>>>> back-outed.
>>>>>>>>>>>
>>>>>>>>>>> I want to resume to work for this issue.
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.03/hotspot/
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.03/jdk/
>>>>>>>>>>>
>>>>>>>>>>> These changes are work fine on jtreg test as below:
>>>>>>>>>>>
>>>>>>>>>>>     hotspot/test/serviceability/tmtools/jstat
>>>>>>>>>>>     jdk/test/sun/tools
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Since JDK 9, default GC algorithm is set to G1.
>>>>>>>>>>> So I think this change is useful to watch GC behavior through
>>>>>>>>>>> jstat.
>>>>>>>>>>>
>>>>>>>>>>> I cannot access JPRT. Could you help?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
> 