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

[Stefan Johansson]

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
>>>>>>