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