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

[Per Liden]

On 04/15/2019 07:16 PM, gerard ziemski wrote:
> On 4/15/19 10:58 AM, Per Liden wrote:
>>>
>>> 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.
> 
> I was wrong in my initial reply.
> 
> There are flags with both range and constraint - the entire point of 
> printing the range to the user is to help define valid values, and as 
> you say "We can't expect a user to know what the range of a specific 
> type is on every platform", which applies to both cases.
> 
> If the test has an issue with the flag, then the test itself needs to be 
> fixed, by excluding the troublesome flag.
> 
> I believe that we should print out the default range in both cases (done 
> in a followup) and we should modify the test to exclude hard to test 
> (troublesome) flags as needed.

But for flags with constrains functions we have no way of knowing the 
valid range, right? So, the question becomes, is it better for a user to 
get no information (we an empty range) instead of the wrong information 
(we print the default range)? I tend to lean toward printing no 
information. A third alternative would be printing the default range for 
the type, but also add something in the printout to indicate that there 
might be other constraints too.

> 
> Can't we just exclude "SoftMaxHeapSize" from the test here?

Sure, I've updated that patch for JDK-8222145 "Add -XX:SoftMaxHeapSize 
flag" to exclude it from testing. Webrev here:

http://cr.openjdk.java.net/~pliden/8222145/webrev.2

Looks ok?

I'll withdraw the review for JDK-8222460, and re-assign the bug to you, 
and you can update it as you see fit, ok?

cheers,
Per