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

Yasumasa Suenaga yasuenag at gmail.com
Tue May 1 04:07:26 UTC 2018


Hi David,

On 2018/05/01 10:36, David Holmes wrote:
> 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.

I got two reviewers.
According to [1], we need to get 1 reviewer and 1 committer or reviewer before pushing. I got them. [2] [3]


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

Okay, I will try it, but I do not have OS X.
Can you share .jtr file? Please attach it to JBS if you can.


Thanks,

Yasumasa


[1] http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-March/030656.html
[2] http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-March/023419.html
[3] http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-April/023590.html


> 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