PING: RFR: JDK-8151674: STW phases at Concurrent GC should count in PerfCounter

[Yasumasa Suenaga]

PING:

We need one more reviewer for HotSpot changes.
Please review.


Thanks,

Yasumasa


On 2016/03/19 4:45, Jon Masamitsu wrote:
>
>
> On 3/18/2016 7:14 AM, Yasumasa Suenaga wrote:
>> Hi,
>>
>>> I added a comment to JBS about G1 initial mark.
>>> I will upload new webrev after discussing for it.
>>
>> We discussed about it on JBS, and we decided not to count initial mark on G1
>> to CGC counter. [1]
>>
>> I uploaded new webrev. Could you review it?
>
> Changes look good.   We'll need one more review of the hotspot changes.
>
> Jon
>
>>
>>   hotspot: http://cr.openjdk.java.net/~ysuenaga/JDK-8151674/webrev.01/hotspot/
>>       jdk: http://cr.openjdk.java.net/~ysuenaga/JDK-8151674/webrev.01/jdk/
>>
>> jdk part is the same with the patch which is reviewed by Staffan Larsen. [2]
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8151674?focusedCommentId=13914167&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13914167
>> [2] http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2016-March/016911.html
>>
>>
>> On 2016/03/17 8:23, Yasumasa Suenaga wrote:
>>> Hi Jon,
>>>
>>> 2016/03/17 3:25 "Jon Masamitsu" <jon.masamitsu at oracle.com <mailto:jon.masamitsu at oracle.com>>:
>>>  >
>>>  >
>>>  >
>>>  > On 03/10/2016 07:59 PM, Yasumasa Suenaga wrote:
>>>  >>
>>>  >> Hi all,
>>>  >>
>>>  >> This review request continues from:
>>>  >> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2016-March/016774.html
>>>  >> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2016-March/016896.html
>>>  >>
>>>  >> I wonder that STW phases (Remark and Cleanup) at G1 are not counted in jstat FGC column.
>>>  >> For example, Initial Mark and Remark at CMS are counted as FGC.
>>>  >> We discussed on hotspot-gc-dev for this issue, and I proposed to add new PerfCounter
>>>  >> for CGC STW phases.
>>>  >>
>>>  >> I uploaded webrev. Could you review it?
>>>  >>      hotspot: http://cr.openjdk.java.net/~ysuenaga/JDK-8151674/webrev.00/hotspot/
>>>  >>          jdk: http://cr.openjdk.java.net/~ysuenaga/JDK-8151674/webrev.00/jdk/
>>>  >>
>>>  >> For compatibility, this patch works the same as the current by default.
>>>  >> If you set -XX:+EnableConcGCPerfCounter, CGC counter will work fine.
>>>  >> (I want to set +EnableConcGCPerfCounter by default)
>>>  >
>>>  >
>>>  > I would prefer not to have the flag (EnableConcGCPerfCounte)
>>>  > and just make the change for CMS.  This is a change for a major
>>>  > release and I think we can accept the incompatibility.
>>>
>>> Okay, I will remove this flag.
>>>
>>> I added a comment to JBS about G1 initial mark.
>>> I will upload new webrev after discussing for it.
>>>
>>>  > http://cr.openjdk.java.net/~ysuenaga/JDK-8151674/webrev.00/hotspot/src/share/vm/gc/g1/g1MonitoringSupport.cpp.udiff.html
>>>  >
>>>  > + // name "collector.2". In a generational collector this would be the
>>>  > + // STW phases in concurrent collection.
>>>  > + _conc_collection_counters =
>>>  > + new CollectorCounters("G1 stop-the-world phases", 2);
>>>  >
>>>  >
>>>  > Is there a hard connection between these phases and a generational
>>>  > collector (i.e., should the "In a generational collector this would be the"
>>>  > be dropped in the comment)?
>>>  >
>>>  > A comment such as
>>>  >
>>>  > // name "collector.2". STW phases as part of a concurrent collection.
>>>
>>> Thanks, I will fix.
>>>
>>>  > A set of JDK changes had been previously been reviewed (not by me). Are these
>>>  > the same?
>>>  >
>>>  > http://cr.openjdk.java.net/~ysuenaga/JDK-8151674/webrev.00/jdk/
>>>
>>> Yes, same patch.
>>>
>>> Yasumasa
>>>
>>>  > Jon
>>>  >
>>>  >
>>>  >
>>>  >>
>>>  >>
>>>  >> I cannot access JPRT.
>>>  >> So I need a sponsor.
>>>  >>
>>>  >>
>>>  >> Thanks,
>>>  >>
>>>  >> Yasumasa
>>>  >
>>>  >
>>>
>