JVM Flag ergonomics and constraints

[Ioi Lam]

On 2/3/2023 12:40 AM, David Holmes wrote:
> On 3/02/2023 5:12 pm, Ioi Lam wrote:
>>
>>
>> On 2/1/2023 10:37 PM, David Holmes wrote:
>>> <trimming>
>>>
>>> On 2/02/2023 3:25 pm, Thomas Stüfe wrote:
>>>> On Thu, Feb 2, 2023 at 5:30 AM David Holmes 
>>>> <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>>>>
>>>>     That would have to be determined case-by-case. For each ergo 
>>>> flag we
>>>>     would have to decide what value it is calculating and based on 
>>>> what,
>>>>     and
>>>>     determine whether an out-of-range value is an error or 
>>>> something to be
>>>>     wary of.
>>>>
>>>> Sure, what I meant was how to control it? Two sets of FLAT_SET_... 
>>>> macros, one that asserts on constraint violation, one that caps?
>>>
>>> No I expect to just see the places where FLAG_SET_ERGO is called do 
>>> the right thing by checking the return value and proceeding 
>>> accordingly.
>>
>> David, are you proposing this?
>>
>>     int value = calculate_ergo();
>>     if (FLAG_SET_ERGO (TheFlag, value) == error) {
>>       value = do_it_again()
>>       if (FLAG_SET_ERGO (TheFlag, value) == error) {
>>          huh?
>>       }
>>     }
>>
>> In the above code, all FLAG_SET_ERGO does is to check the range for 
>> you. It doesn't tell you why the range check fails. So the caller 
>> would have to redo the calculation (with no more information, other 
>> than that the last value was no good). What if the new value is also 
>> no good?
>
> To me that indicates we don't know what the heck we are calculating! 
> How can you calculate a setting for a flag if you don't even 
> understand the valid range of that flag. Which may mean the max, for 
> example, needs to be exposed programmatically.
>
>> With this style, you almost have to wrap every FLAG_SET_ERGO  in a 
>> loop, keep trying (in the dark) until you succeed.
>>
>> ==================
>> I think it's better to just assert inside FLAG_SET_ERGO if the value 
>> is out of range. The caller should make sure it has calculated a 
>> correct value.
>
> If the calculation uses some externally settable value then an assert 
> won't help if we're in a product build.
>

We have lots of set_xxx() APIs in various classes that assert when given 
an inappropriate value -- this indicates a programming error. So 
FLAG_SET_ERGO would be no different.

It's the responsibility of the caller of FLAG_SET_ERGO to pass a correct 
value to FLAG_SET_ERGO . How the caller comes up with that value 
(whether it depends on some external input) is irrelevant.

========

To properly catch the programming errors with ERGO setting, we should:

- Change FLAG_SET_ERGO to return "void". The current API gives the false 
impression that the caller needs to check the returned value and do 
something about it.
- Assert if the value is out of range, so we can catch the programming 
error during testing (with debug builds)

It's up to the caller of FLAG_SET_ERGO to determine a value that's 
within the allowed range.

If the caller wants to query the constraints of the flag that it wants 
to set, that can be done via the JVMFlagLimit API. Using JVMFlagLimit is 
much better than blindly setting a value and getting an error code that 
doesn't tell you why it failed.


> As I said it depends on exactly what we are using to do the 
> calculation. If it is "closed world" then it is a fatal error to get 
> it wrong and should be detected during testing. Otherwise the code has 
> to deal with the possibility of error.
>
>>
>> Maybe we can add a new FLAG_SET_ERGO_CAPPED() that would cap the 
>> value, but that should be an opt-in. FLAG_SET_ERGO() should not 
>> automatically cap the value.
>
> That might be suitable for some flags/calculations.
>
> I don't like trying to talk about this in generalities. Each specific 
> situation needs to be examined to understand what is being calculated 
> and how. But it is the callers of FLAG_SET_ERGO that need to know what 
> they are doing.
>
> Cheers,
> David
>
>>
>> Thanks
>> - Ioi
>>