PING: RFR: 8199519: Several GC tests fails with: java.lang.NumberFormatException: Unparseable number: "-"

Yasumasa Suenaga yasuenag at gmail.com
Tue Apr 3 12:37:21 UTC 2018


PING: Could you review it?
This change has been passed Mach5 test.

>>     > http://cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.03/


Thanks,

Yasumasa


On 2018/03/28 22:38, Stefan Johansson wrote:
> Mach5 testing looks good.
> 
> Can someone in the serviceability team do the second review?
> 
> Cheers,
> Stefan
> 
> On 2018-03-28 13:32, Yasumasa Suenaga wrote:
>> Thanks Stefan,
>> I'm waiting for second reviewer.
>>
>>
>> Yasumasa
>>
>>
>> 2018年3月28日(水) 18:36 Stefan Johansson <stefan.johansson at oracle.com <mailto:stefan.johansson at oracle.com>>:
>>
>>     Hi Yasumasa,
>>
>>     Local testing looks good and I've kicked of some additional Mach5
>>     testing that will include these tests on all platforms.
>>
>>     Cheers,
>>     Stefan
>>
>>     On 2018-03-28 06:04, Yasumasa Suenaga wrote:
>>     > Hi Stefan,
>>     >
>>     > Thank you for sharing your report!
>>     > I could reproduce them on my VM.
>>     >
>>     > I've fixed them in new webrev, and it works fine on my environment.
>>     > Could you check again?
>>     >
>>     > http://cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.03/ <http://cr.openjdk.java.net/%7Eysuenaga/JDK-8199519/webrev.03/>
>>     >
>>     >
>>     > Thanks,
>>     >
>>     > Yasumasa
>>     >
>>     >
>>     >
>>     > 2018-03-28 0:29 GMT+09:00 Stefan Johansson <stefan.johansson at oracle.com <mailto:stefan.johansson at oracle.com>>:
>>     >>
>>     >> On 2018-03-27 16:44, Yasumasa Suenaga wrote:
>>     >>> Hi Stefan,
>>     >>>
>>     >>> On 2018/03/27 22:45, Stefan Johansson wrote:
>>     >>>> Hi Yasumasa,
>>     >>>>
>>     >>>> On 2018-03-27 10:56, Yasumasa Suenaga wrote:
>>     >>>>> Hi Stefan,
>>     >>>>>
>>     >>>>> Thank you for your comment.
>>     >>>>> I updated webrev:
>>     >>>>>
>>     >>>>>     webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.01/ <http://cr.openjdk.java.net/%7Eysuenaga/JDK-8199519/webrev.01/>
>>     >>>> I think the usage of Optional in Expression.setRequired(bool) is a bit
>>     >>>> unnecessary. It will create temporary objects and there is no benefit from
>>     >>>> just doing two simple if-statements.
>>     >>>
>>     >>> I fixed it in new webrev:
>>     >>> http://cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.02/ <http://cr.openjdk.java.net/%7Eysuenaga/JDK-8199519/webrev.02/>
>>     >>>
>>     >>>
>>     >>>> I also ran this patch (and the one using forcibly) on my single core VM
>>     >>>> and realized that this fix will have to include some awk-file updates to
>>     >>>> make the test in test/jdk/sun/tools/jstat pass when Serial in chosen as the
>>     >>>> default collector. The tests in test/jdk/sun/tools/jstatd/ are fine.
>>     >>>
>>     >>> Can you share the failure report?
>>     >> It relates to all tests that display the the CGC and the CGCT columns, for
>>     >> example in jstatGCOutput1.sh:
>>     >>   S0C    S1C    S0U    S1U      EC       EU OC  OU       MC     MU
>>     >> CCSC   CCSU   YGC     YGCT FGC    FGCT    CGC CGCT     GCT
>>     >> 256.0  256.0  254.0   0.0    2176.0   1025.0 5504.0 920.5    7168.0
>>     >> 6839.7 768.0  602.8       2    0.007   0 0.000   -       -    0.007
>>     >>
>>     >> The awk regex needs to be updated to handle '-' for these tests:
>>     >> test: sun/tools/jstat/jstatGcCapacityOutput1.sh
>>     >> Failed. Execution failed: exit code 1
>>     >>
>>     >> test: sun/tools/jstat/jstatGcMetaCapacityOutput1.sh
>>     >> Failed. Execution failed: exit code 1
>>     >>
>>     >> test: sun/tools/jstat/jstatGcNewCapacityOutput1.sh
>>     >> Failed. Execution failed: exit code 1
>>     >>
>>     >> test: sun/tools/jstat/jstatGcOldCapacityOutput1.sh
>>     >> Failed. Execution failed: exit code 1
>>     >>
>>     >> test: sun/tools/jstat/jstatGcOldOutput1.sh
>>     >> Failed. Execution failed: exit code 1
>>     >>
>>     >> test: sun/tools/jstat/jstatGcOutput1.sh
>>     >> Failed. Execution failed: exit code 1
>>     >>
>>     >>
>>     >>> If it occurs in jstatClassloadOutput1.sh, it relates to JDK-8173942.
>>     >>>
>>     >>>
>>     >>> Thanks,
>>     >>>
>>     >>> Yasumasa
>>     >>>
>>     >>>
>>     >>>> Thanks,
>>     >>>> Stefan
>>     >>>>>     submit-hs: mach5-one-ysuenaga-JDK-8199519-20180327-0652-16322
>>     >>>>>
>>     >>>>>
>>     >>>>> Thanks,
>>     >>>>>
>>     >>>>> Yasumasa
>>     >>>>>
>>     >>>>>
>>     >>>>>
>>     >>>>> 2018-03-27 0:03 GMT+09:00 Stefan Johansson
>>     >>>>> <stefan.johansson at oracle.com <mailto:stefan.johansson at oracle.com>>:
>>     >>>>>> Hi Yasumasa,
>>     >>>>>>
>>     >>>>>> On 2018-03-22 11:35, Yasumasa Suenaga wrote:
>>     >>>>>>> Hi all,
>>     >>>>>>>
>>     >>>>>>> Please review this change:
>>     >>>>>>>
>>     >>>>>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8199519
>>     >>>>>>> webrev: cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.00/ <http://cr.openjdk.java.net/%7Eysuenaga/JDK-8199519/webrev.00/>
>>     >>>>>> The fix seems to make things to work as expected. Manually tested it
>>     >>>>>> and
>>     >>>>>> Mach5 also looks good.
>>     >>>>>>
>>     >>>>>> I have some comments regarding the patch. I think 'forcibly' should be
>>     >>>>>> rename to something more descriptive. Naming is never easy but I think
>>     >>>>>> 'required' would be better, as in, this column is required and not
>>     >>>>>> allowed
>>     >>>>>> to print '-'. That would also render the code in
>>     >>>>>> ExpressionResolver.java to
>>     >>>>>> be:
>>     >>>>>>     return new Literal(isRequired ? 0.0d : Double.NaN);
>>     >>>>>> I think that also better explains why we return 0 instead of NaN.
>>     >>>>>>
>>     >>>>>> I would also like to see the forcibly/required state moved into the
>>     >>>>>> Expression it self, that way we don't have to pass it around but can
>>     >>>>>> instead
>>     >>>>>> do:
>>     >>>>>>     return new Literal(e.isRequired() ? 0.0d : Double.NaN);
>>     >>>>>>
>>     >>>>>> Thanks,
>>     >>>>>> Stefan
>>     >>>>>>
>>     >>>>>>
>>     >>>>>>> After JDK-8153333, some jstat tests are failed because GCT in jstat
>>     >>>>>>> output
>>     >>>>>>> is dash (-) if garbage collector is not concurrent collector e.g.
>>     >>>>>>> Serial GC.
>>     >>>>>>> I fixed that GCT can be calculated correctly.
>>     >>>>>>>
>>     >>>>>>> This change has been tested on Mach5 by Stefan.
>>     >>>>>>>
>>     >>>>>>>
>>     >>>>>>> Thanks,
>>     >>>>>>>
>>     >>>>>>> Yasumasa
>>     >>>>>>
>>
> 


More information about the serviceability-dev mailing list