RFR: 8222460: -XX:+PrintFlagsRanges prints incorrect range

[Per Liden]

Hi Gerard,

On 04/15/2019 05:29 PM, gerard ziemski wrote:
> 
> 
> On 4/15/19 12:58 AM, Per Liden wrote:
>> When testing JDK-8222145 (Add -XX:SoftMaxHeapSize flag), I ran into 
>> this issue.
>>
>> -XX:+PrintFlagsRanges prints incorrect value range for flags that are 
>> associated with a constraint function. Instead of printing an empty 
>> range "[ ...                           ]" is prints the default value 
>> range for the type. It should do the opposite.
>>
>> The fix in jvmFlagRangeList.cpp is trivial. However, it has the side 
>> effect that some options that were previously not tested are now 
>> tested, and vice versa. As a result I had to exclude testing the max 
>> range of two problematic options (ActiveProcessorCount and 
>> G1PeriodicGCInterval). It might also be the cases that some of the 
>> previously excluded options no longer need to be excluded, but I 
>> didn't want to fiddle with that in this patch. If this is the case, I 
>> suggest that is fixed in a follow up RFE.
>>
>> Testing: tier1-3 in all Oracle platforms, tier1-7 on Linux/x86_64
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8222460
>> Webrev: http://cr.openjdk.java.net/~pliden/8222460/webrev.0
>>
>> /Per
>>
> 
> hi Per,
> 
> I'm sorry that this caused an issue for you.

No problem.

> 
> I think the original idea here was that for those flags with a 
> constraint, we wanted to show the possible range, from which the 
> constraint will further restrict the final value. However, that is 
> tricky to test without exposing the constraint function, as evidenced by 
> the exclusion list in the test.
> 
> For those flags without range and constraint, the implicit range is the 
> max range of the flag's type, so the idea here was that such flag was 
> "untestable" for practical purposes, so we print an empty range.

But that's not very useful to an actual user, who want's to know what 
the range is (even if every value allowed by the type is valid). We 
can't expect a user to know what the range of a specific type is on 
every platform.

> 
> I believe that a better fix here might be to print an empty range in 
> both cases.

But why print an empty range when the range is well known? We don't have 
to make -XX:+PrintFlagsRanges dumber than it needs to be. The only time 
we don't know the range is what there's a constraint function associated 
with the flag.

Frankly, to me this looks like the original intent of this code, but a 
simple mistake snuck in which inverted the if-else statement.

cheers,
Per

> 
> The cleanup of the exclusion flags in the test (and removal of default 
> print range functions, if not needed anymore) can be handled by a 
> followup issue.
> 
> 
> cheers
>