[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8199519: Several GC tests fails with: java.lang.NumberFormatException: Unparseable number:
From: Stefan Johansson <stefan.johansson () oracle ! com>
Date: 2018-03-28 13:38:01
Message-ID: 5c1975cd-1080-652e-c23a-abd693cc0095 () oracle ! com
[Download RAW message or body]
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@oracle.com
> <mailto:stefan.johansson@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@oracle.com <mailto:stefan.johansson@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@oracle.com
> <mailto:stefan.johansson@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
> >>>>>>
>
[Attachment #3 (text/html)]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<tt>Mach5 testing looks good.<br>
<br>
Can someone in the serviceability team do the second review? <br>
<br>
Cheers,<br>
Stefan<br>
</tt><br>
<div class="moz-cite-prefix">On 2018-03-28 13:32, Yasumasa Suenaga
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAGFVN2BvFBDo_BfhgDN0Q4JW+2d6VsL2+6MWTKK_ChT9we9FAw@mail.gmail.com">
<div dir="auto">Thanks Stefan,
<div dir="auto">I'm waiting for second reviewer.</div>
<div dir="auto"><br>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">Yasumasa</div>
<div dir="auto"><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">2018年3月28日(水) 18:36 Stefan Johansson <<a
href="mailto:stefan.johansson@oracle.com"
moz-do-not-send="true">stefan.johansson@oracle.com</a>>:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Yasumasa,<br>
<br>
Local testing looks good and I've kicked of some additional
Mach5<br>
testing that will include these tests on all platforms.<br>
<br>
Cheers,<br>
Stefan<br>
<br>
On 2018-03-28 06:04, Yasumasa Suenaga wrote:<br>
> Hi Stefan,<br>
><br>
> Thank you for sharing your report!<br>
> I could reproduce them on my VM.<br>
><br>
> I've fixed them in new webrev, and it works fine on my
environment.<br>
> Could you check again?<br>
><br>
> <a
href="http://cr.openjdk.java.net/%7Eysuenaga/JDK-8199519/webrev.03/"
rel="noreferrer noreferrer" target="_blank"
moz-do-not-send="true">http://cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.03/</a><br>
><br>
><br>
> Thanks,<br>
><br>
> Yasumasa<br>
><br>
><br>
><br>
> 2018-03-28 0:29 GMT+09:00 Stefan Johansson <<a
href="mailto:stefan.johansson@oracle.com" target="_blank"
rel="noreferrer" moz-do-not-send="true">stefan.johansson@oracle.com</a>>:<br>
>><br>
>> On 2018-03-27 16:44, Yasumasa Suenaga wrote:<br>
>>> Hi Stefan,<br>
>>><br>
>>> On 2018/03/27 22:45, Stefan Johansson wrote:<br>
>>>> Hi Yasumasa,<br>
>>>><br>
>>>> On 2018-03-27 10:56, Yasumasa Suenaga wrote:<br>
>>>>> Hi Stefan,<br>
>>>>><br>
>>>>> Thank you for your comment.<br>
>>>>> I updated webrev:<br>
>>>>><br>
>>>>> webrev: <a
href="http://cr.openjdk.java.net/%7Eysuenaga/JDK-8199519/webrev.01/"
rel="noreferrer noreferrer" target="_blank"
moz-do-not-send="true">http://cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.01/</a><br>
>>>> I think the usage of Optional in
Expression.setRequired(bool) is a bit<br>
>>>> unnecessary. It will create temporary objects
and there is no benefit from<br>
>>>> just doing two simple if-statements.<br>
>>><br>
>>> I fixed it in new webrev:<br>
>>> <a
href="http://cr.openjdk.java.net/%7Eysuenaga/JDK-8199519/webrev.02/"
rel="noreferrer noreferrer" target="_blank"
moz-do-not-send="true">http://cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.02/</a><br>
>>><br>
>>><br>
>>>> I also ran this patch (and the one using
forcibly) on my single core VM<br>
>>>> and realized that this fix will have to
include some awk-file updates to<br>
>>>> make the test in test/jdk/sun/tools/jstat
pass when Serial in chosen as the<br>
>>>> default collector. The tests in
test/jdk/sun/tools/jstatd/ are fine.<br>
>>><br>
>>> Can you share the failure report?<br>
>> It relates to all tests that display the the CGC and
the CGCT columns, for<br>
>> example in jstatGCOutput1.sh:<br>
>> S0C S1C S0U S1U EC EU OC
OU MC MU<br>
>> CCSC CCSU YGC YGCT FGC FGCT CGC
CGCT GCT<br>
>> 256.0 256.0 254.0 0.0 2176.0 1025.0
5504.0 920.5 7168.0<br>
>> 6839.7 768.0 602.8 2 0.007 0 0.000 -
- 0.007<br>
>><br>
>> The awk regex needs to be updated to handle '-' for
these tests:<br>
>> test: sun/tools/jstat/jstatGcCapacityOutput1.sh<br>
>> Failed. Execution failed: exit code 1<br>
>><br>
>> test: sun/tools/jstat/jstatGcMetaCapacityOutput1.sh<br>
>> Failed. Execution failed: exit code 1<br>
>><br>
>> test: sun/tools/jstat/jstatGcNewCapacityOutput1.sh<br>
>> Failed. Execution failed: exit code 1<br>
>><br>
>> test: sun/tools/jstat/jstatGcOldCapacityOutput1.sh<br>
>> Failed. Execution failed: exit code 1<br>
>><br>
>> test: sun/tools/jstat/jstatGcOldOutput1.sh<br>
>> Failed. Execution failed: exit code 1<br>
>><br>
>> test: sun/tools/jstat/jstatGcOutput1.sh<br>
>> Failed. Execution failed: exit code 1<br>
>><br>
>><br>
>>> If it occurs in jstatClassloadOutput1.sh, it
relates to JDK-8173942.<br>
>>><br>
>>><br>
>>> Thanks,<br>
>>><br>
>>> Yasumasa<br>
>>><br>
>>><br>
>>>> Thanks,<br>
>>>> Stefan<br>
>>>>> submit-hs:
mach5-one-ysuenaga-JDK-8199519-20180327-0652-16322<br>
>>>>><br>
>>>>><br>
>>>>> Thanks,<br>
>>>>><br>
>>>>> Yasumasa<br>
>>>>><br>
>>>>><br>
>>>>><br>
>>>>> 2018-03-27 0:03 GMT+09:00 Stefan
Johansson<br>
>>>>> <<a
href="mailto:stefan.johansson@oracle.com" target="_blank"
rel="noreferrer" moz-do-not-send="true">stefan.johansson@oracle.com</a>>:<br>
>>>>>> Hi Yasumasa,<br>
>>>>>><br>
>>>>>> On 2018-03-22 11:35, Yasumasa Suenaga
wrote:<br>
>>>>>>> Hi all,<br>
>>>>>>><br>
>>>>>>> Please review this change:<br>
>>>>>>><br>
>>>>>>> JBS: <a
href="https://bugs.openjdk.java.net/browse/JDK-8199519"
rel="noreferrer noreferrer" target="_blank"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8199519</a><br>
>>>>>>> webrev: <a
href="http://cr.openjdk.java.net/%7Eysuenaga/JDK-8199519/webrev.00/"
rel="noreferrer noreferrer" target="_blank"
moz-do-not-send="true">cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.00/</a><br>
>>>>>> The fix seems to make things to work
as expected. Manually tested it<br>
>>>>>> and<br>
>>>>>> Mach5 also looks good.<br>
>>>>>><br>
>>>>>> I have some comments regarding the
patch. I think 'forcibly' should be<br>
>>>>>> rename to something more descriptive.
Naming is never easy but I think<br>
>>>>>> 'required' would be better, as in,
this column is required and not<br>
>>>>>> allowed<br>
>>>>>> to print '-'. That would also render
the code in<br>
>>>>>> ExpressionResolver.java to<br>
>>>>>> be:<br>
>>>>>> return new Literal(isRequired ?
0.0d : Double.NaN);<br>
>>>>>> I think that also better explains why
we return 0 instead of NaN.<br>
>>>>>><br>
>>>>>> I would also like to see the
forcibly/required state moved into the<br>
>>>>>> Expression it self, that way we don't
have to pass it around but can<br>
>>>>>> instead<br>
>>>>>> do:<br>
>>>>>> return new Literal(e.isRequired()
? 0.0d : Double.NaN);<br>
>>>>>><br>
>>>>>> Thanks,<br>
>>>>>> Stefan<br>
>>>>>><br>
>>>>>><br>
>>>>>>> After JDK-8153333, some jstat
tests are failed because GCT in jstat<br>
>>>>>>> output<br>
>>>>>>> is dash (-) if garbage collector
is not concurrent collector e.g.<br>
>>>>>>> Serial GC.<br>
>>>>>>> I fixed that GCT can be
calculated correctly.<br>
>>>>>>><br>
>>>>>>> This change has been tested on
Mach5 by Stefan.<br>
>>>>>>><br>
>>>>>>><br>
>>>>>>> Thanks,<br>
>>>>>>><br>
>>>>>>> Yasumasa<br>
>>>>>><br>
<br>
</blockquote>
</div>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic