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

[Yasumasa Suenaga]

PING: Could you review them?
We need a second reviewer.

>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.02/hotspot/
>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.02/jdk/


Thanks,

Yasumasa


On 2016/05/07 8:26, Jon Masamitsu wrote:
>
>
> On 5/5/2016 5:52 PM, Yasumasa Suenaga wrote:
>> Hi Jon,
>>
>> Thank you for your comment.
>> I uploaded new webrev:
>>
>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.02/hotspot/
>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.02/jdk/
>>
>>
>>> Could this assertion fail because of rounding error?  Or if GC time is very small?
>>>
>>> + assertThat(CGCT > 0, "Number of full GC events is " + CGC + ", but CGCT is 0");
>>
>> Full GC time is evaluated in the same way.
>> So I think it is not ciritical.
>> STW in CGC might be less than FGC time. So if we have concern for it, I will remove it.
>> (If we remove it, should we remove assertion for FGCT?)
>
> All right.  Leave it.  If it fails in the future, we can remove it then.
>
> Jon
>
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2016/05/06 7:30, Jon Masamitsu wrote:
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.01/hotspot/test/serviceability/tmtools/jstat/utils/JstatGcCapacityResults.java.udiff.html
>>>
>>> + * CGC Number of STW of Concurrent GC Events.
>>>
>>> I think a better comment would be "Number of STW events during Concurrent GC's"
>>> if that is accurate.
>>>
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.01/hotspot/test/serviceability/tmtools/jstat/utils/JstatGcCauseResults.java.udiff.html
>>>
>>> + * CGC Number of STW of Concurrent GC.
>>>
>>> Number of STW events during Concurrent GC's.
>>>
>>> + * CGCT STW of Concurrent GC time.
>>> STW time during Concurrent GC's
>>>
>>>
>>> Could this assertion fail because of rounding error?  Or if GC time is very small?
>>>
>>> + assertThat(CGCT > 0, "Number of full GC events is " + CGC + ", but CGCT is 0");
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.01/hotspot/test/serviceability/tmtools/jstat/utils/JstatGcResults.java.udiff.html
>>>
>>> Also fix
>>>
>>> + * CGC Number of STW of Concurrent GC events.
>>> + * CGCT STW of Concurrent garbage collection time.
>>>
>>> consider
>>>
>>>
>>> + assertThat(CGCT > 0, "Number of full GC events is " + CGC + ", but CGCT is 0");
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.01/hotspot/test/serviceability/tmtools/jstat/utils/JstatResults.java.udiff.html
>>>
>>> + assertThat(increasedFGC || increasedCGC, "FGC or CGC didn't increase between measurements 2 and 3");
>>>
>>> Comment should be "FGC and CGC both don't increase between measurents 2 and 3".
>>>
>>>
>>> Rest look good.
>>>
>>> Jon
>>>
>>> On 05/04/2016 10:00 PM, Yasumasa Suenaga wrote:
>>>> Hi all,
>>>>
>>>> We modified our patches for this enhancement.
>>>> They work fine subset of JPRT tests which are related to GCs.
>>>> (Many thanks, Jon!)
>>>>
>>>> I uploaded a webrev. Could you review again?
>>>>
>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.01/hotspot/
>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.01/jdk/
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2016/04/14 23:46, Jon Masamitsu wrote:
>>>>> Yasmasa,
>>>>>
>>>>> I have your patch in a repository and will push it early next week.
>>>>> Some repositories are being merged in the next few days and I'd
>>>>> prefer to push after that is complete.
>>>>>
>>>>> Jon
>>>>>
>>>>> On 04/11/2016 07:30 AM, Yasumasa Suenaga wrote:
>>>>>> PING: Could you review and sponsor it?
>>>>>>
>>>>>>>>>       - hotspot
>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.00/hotspot/
>>>>>>>>>
>>>>>>>>>       - jdk
>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.00/jdk/
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2016/04/05 8:19, Yasumasa Suenaga wrote:
>>>>>>> Daniel-san,
>>>>>>>
>>>>>>> Sorry for my changeset.
>>>>>>>
>>>>>>>> We're still sifting through the various failures and figuring out where they came from.
>>>>>>> Okay, I will ping in next week.
>>>>>>> (I sent several review request for hs-rt. I will ping them in next week too.)
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2016/04/05 2:38, Daniel D. Daugherty wrote:
>>>>>>>> Yasumasa-san,
>>>>>>>>
>>>>>>>> Please hold off on redoing this changeset. Friday (04.01) was quite
>>>>>>>> a train wreck for our JDK9-hs-rt internal testing. We're still sifting
>>>>>>>> through the various failures and figuring out where they came from.
>>>>>>>>
>>>>>>>> Dan
>>>>>>>>
>>>>>>>>
>>>>>>>> On 4/2/16 7:41 AM, Yasumasa Suenaga wrote:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> JDK-8151674 has been backouted.
>>>>>>>>> However, I want to try to merge this enhancement to hs-rt repos.
>>>>>>>>>
>>>>>>>>> I updated testcase in:
>>>>>>>>>
>>>>>>>>>       - hotspot/test/serviceability/tmtools/jstat
>>>>>>>>>       - jdk/test/sun/tools/jstat sun/tools/jstatd
>>>>>>>>> I ran them with jtreg as below, they works fine.
>>>>>>>>>
>>>>>>>>>       - hotspot
>>>>>>>>>           $JT_HOME/bin/jtreg -ignore:quiet serviceability/tmtools/jstat
>>>>>>>>>
>>>>>>>>>       - jdk
>>>>>>>>>           $JT_HOME/bin/jtreg -ignore:quiet sun/tools/jstat sun/tools/jstatd
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I uploaded a webrev.
>>>>>>>>> Could you review it?
>>>>>>>>>
>>>>>>>>>       - hotspot
>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.00/hotspot/
>>>>>>>>>
>>>>>>>>>       - jdk
>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.00/jdk/
>>>>>>>>>
>>>>>>>>> Note: I did not modify changes in hotspot and jstat_options in jdk repos.
>>>>>>>>>           Changes from JDK-8151674 is for testcase only.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I cannot access JPRT.
>>>>>>>>> So I need a sponsor.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>
>