Rev1 RFR (S): 8112746 Followup to JEP 245: Validate JVM Command-Line Flag Arguments

[Dmitry Dmitriev]

Hello Gerard,

This revision looks good. But I have one concern about new upper range 
for G1ConcMarkStepDurationMillis which now equal to "DBL_MAX". This 
value can be very huge. For example, I check it on my Linux-x64 and got 
value which is more than 300 characters long. Moreover we can not pass 
that huge value to the JVM since JVM can process only values which less 
than 256 characters long. Otherwise it will report that option is 
improperly specified. And also output of "-XX:+PrintFlagsRanges" looks 
strange with this very long value :)
I think that upper range can be lowered for that flag without impacting 
users. For example we can use very big value for that option, e.g. 
10000000000000000000.0 (i.e. something between (double)max_intx and 
(double)max_uintx). G1ConcMarkStepDurationMillis is a duration in 
milliseconds, so max value 10000000000000000000.0 will represent 
duration equal to (((10000000000000000000.0 / 1000) / 3600 seconds) / 24 
hours) / 365 days = ~317097919 years. I think that this will be enough 
for all :)

Regards,
Dmitry

On 26.06.2015 0:58, Gerard Ziemski wrote:
> hi all,
>
> This webrev gets the macro and quotes fix right.
>
> Please review this followup to my recent JEP 245 checkin. It addresses 
> the issues raised by Coleen, Dmitry and Kim during webrev.
>
> You can see https://bugs.openjdk.java.net/browse/JDK-8112746 for 
> details, but the most important change here is that we only check 
> constraint if the range check passes first.
>
> To quickly recap: I changed that part of the code when David pointed 
> out that I had to modify 2 tests in a way that looked like a 
> regression - I removed some test cases. However, Kim, later pointed 
> out that the original code had the advantage of having constraints 
> guaranteed that the flag values were within ranges.
>
> I checked in the code with ranges and constraints being checked both 
> regardless of each other, but this followup restores the original 
> behavior (and simplifies the code), where we first check ranges and 
> only check constraints if range passes.
>
> The 2 tests (ObjectAlignment.java and Options.java) seem to loose some 
> test cases, but those paths are still tested (though with different 
> values), so we in fact do not loose anything from test coverage point 
> of view.
>
> The change passes JPRT (hotspot) and RBT (vm.quick.testlist)
>
>
> References:
>
>     webrev:http://cr.openjdk.java.net/~gziemski/8112746_rev1
> this issue:https://bugs.openjdk.java.net/browse/JDK-8112746
>    JEP 245:https://bugs.openjdk.java.net/browse/JDK-8059557
>
>
> hg stat:
> # hg_stat
>   src/share/vm/gc/g1/g1_globals.hpp                     |    4 +-
>   src/share/vm/runtime/commandLineFlagConstraintsGC.cpp |    5 +-
>   src/share/vm/runtime/commandLineFlagRangeList.cpp     |   58 ++++------
>   src/share/vm/runtime/globals.cpp                      |  129 +++++++++++++----------
>   src/share/vm/runtime/globals.hpp                      |   17 +--
>   test/runtime/CompressedOops/ObjectAlignment.java      |    3 +-
>   test/runtime/contended/Options.java                   |    2 -
>   7 files changed, 103 insertions(+), 115 deletions(-)
>
>
>
>
>