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

Yasumasa Suenaga yasuenag at gmail.com
Wed Mar 7 12🔞41 UTC 2018

PING: Could you review it?

http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.08/

JBS: https://bugs.openjdk.java.net/browse/JDK-8153333 CSR: https://bugs.openjdk.java.net/browse/JDK-8196862

This change has passed Mach5 on submit repo. Also it has passed hotspot/jtreg/:hotspot_serviceability and jdk/:jdk_tools jtreg tests.

We need one more reviewer.

Thanks,

Yasumasa

On 2018/02/21 21:14, Yasumasa Suenaga wrote:

PING: Could you review it?

http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.07/ JBS: https://bugs.openjdk.java.net/browse/JDK-8153333 CSR: https://bugs.openjdk.java.net/browse/JDK-8196862 Yasumasa On 2018/02/15 10:23, Yasumasa Suenaga wrote: Hi all,

CSR for this issue [1] has been approved. This webrev has been reviewed by Stefan, but we need one more reviewer. Could you review it? http://cr.openjdk.java.net/~ysuenaga/JDK-8153333/webrev.07/

Thanks, Yasumasa [1] https://bugs.openjdk.java.net/browse/JDK-8196862

2018-02-06 22:33 GMT+09:00 Yasumasa Suenaga <yasuenag at gmail.com>: 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 :hotspotserviceability :jdktools 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/:hotspotserviceability and jdk/:jdktools 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

More information about the serviceability-dev mailing list