RFR (M): 8112746: Followup to JDK-8059557 (JEP 245: Validate JVM Command-Line Flag Arguments)

[gerard ziemski]

hi Dmitry, Kim,

Agree, the value looks ridiculous, though 
test/runtime/CommandLine/OptionsValidation test seems to work just fine 
with it.

So I originally had G1ConcMarkStepDurationMillis range defined as 
"range(0, (double)max_uintx)", but the request was to use mathematically 
correct bound, ie. DBL_MAX, as any other value would have to be 
carefully determined as it's a user limiting change.

Sangheon, is currently working on JDK-8059555, so maybe I should leave 
this flag to him, since it's a GC flag and he already indicated that he 
thinks he will need DBL_MAX for other flags? It looks like Sangheon will 
be faced with the same kind of issue for other flags, so maybe we should 
leave this issue up to him and have a single discussion about this and 
other flags at the same time?

Sanghon, is it OK if I take this particular change out and you and GC 
team will handle it as you find appropriate while you work on JDK-8059555?


cheers


On 08/03/2015 07:08 AM, Dmitry Dmitriev wrote:
> Hello Gerard,
>
> As Kim mention, 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 31.07.2015 21:04, gerard ziemski wrote:
>> Please review this webrev 2 of the follow-up fixes including:
>>
>> - Adding DBL_MAX (requested by Kim Barrett)
>>
>> - Passing values by value, not pointer, to constraint functions. It was thought originally that me way want to “fix” the values, but it was decided to be out of scope for the JEP. (requested by Coleen)
>>
>> - Add print_error_as_needed() utility function to range/constraint code to improve code robustness (requested by Coleen)
>>
>> - Remove debug code (requested by Kim Barrett)
>>
>> - Implement Flag::flag_error_str in .cpp file (requested by Kim Barrett)
>>
>> - Only check constraint if range passes (requested by Kim Barrett)
>>
>> - Merged with Sangheon Kim fix for "Add additional validation after heap creation” (JDK-8130459)
>>
>>
>> This webrev passes "JPRT -testset hotspot” and “RBT vm.quick testlist”
>>
>>
>> References:
>>
>>     JEP 245:https://bugs.openjdk.java.net/browse/JDK-8122937
>>         bug:https://bugs.openjdk.java.net/browse/JDK-8112746
>>      webrev:http://cr.openjdk.java.net/~gziemski/8112746_rev2/
>>
>