PING: RFR: 8199519: Several GC tests fails with: java.lang.NumberFormatException: Unparseable number: "-"
David Holmes
david.holmes at oracle.com
Tue May 1 01:36:11 UTC 2018
Hi Yasumasa,
This appears to have been pushed without the second review occurring.
Chris's email appears to be from after when this was pushed.
This change also seems to be causing, or contributing to a failure in
serviceability/tmtools/jstat/GcTest01.java:
java.lang.RuntimeException: Number of concurrent GC events is 1, but
CGCT is 0
I will file a bug for that and assign to you for evaluation.
Thanks,
David
On 25/04/2018 10:48 PM, Yasumasa Suenaga wrote:
> PING: Could you review this change?
> I've sent review request about a month ago, but I do not yet get second
> reviewer.
>
>>>>> > http://cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.03/
>
>
> Yasumasa
>
>
> On 2018/04/10 20:10, Yasumasa Suenaga wrote:
>> PING: Could you review it?
>> We need one more reviewer.
>>
>>>>> > http://cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.03/
>>
>>
>> Yasumasa
>>
>>
>> On 2018/04/03 21:37, Yasumasa Suenaga wrote:
>>> 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