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

[Yasumasa Suenaga]

Thanks Stefan,
I'm waiting for second reviewer.


Yasumasa


2018年3月28日(水) 18:36 Stefan Johansson <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/
> >
> >
> > Thanks,
> >
> > Yasumasa
> >
> >
> >
> > 2018-03-28 0:29 GMT+09:00 Stefan Johansson <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/
> >>>> 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/
> >>>
> >>>
> >>>> 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>:
> >>>>>> 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/
> >>>>>> 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
> >>>>>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180328/db41abc2/attachment.html>