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

Tue Feb 6 05:10:05 UTC 2018

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/

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?

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
>>>>>>>>>
>>>
>