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 <yasue...@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 >>> :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.johans...@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 >>>>>>>>>>>> >> >
