JVM Flag ergonomics and constraints

[David Holmes]

On 1/02/2023 7:48 pm, Thomas Stüfe wrote:
> On Wed, Feb 1, 2023 at 5:24 AM David Holmes <david.holmes at oracle.com 
> <mailto:david.holmes at oracle.com>> wrote:
> 
>     On 1/02/2023 5:23 am, Ioi Lam wrote:
>      > CC-ing hotspot-dev for wider discussion.
>      >
>      > On 1/31/2023 12:43 AM, Thomas Stüfe wrote:
>      >> Hi Ioi,
>      >>
>      >> a short question. You did the JVM flag handling revamp, right?
>      >>
>      >> I recently ran into a weird problem where FLAG_SET_ERGO would not
>      >> work, turned out it ends up using the constraint function, which
>      >> silently refused to set the argument if the argument violates the
>      >> constraint.
>      >>
>      >> That makes perfect sense, I had an error in my code. But if we
>     set a
>      >> flag with ERGO origin, we set it in the JVM, and the JVM should
>     adhere
>      >> to constraints, not doing so is a programming error and I would
>     expect
>      >> an assert here. The same logic may be applied to other origins too,
>      >> e.g. DEFAULT.
>      >> Do you think this makes sense?
>      >>
>      >> BTW I added debug output (setting verbose to true for ERGO), and
>     I see
>      >> other flag's ergonomics also getting ignored, which would have
>     to be
>      >> fixed too.
>      >>
>      >> Cheers, Thomas
>      >>
>      >
>      >
>      > This problem is also reported in
>      > https://bugs.openjdk.org/browse/JDK-8224980
>     <https://bugs.openjdk.org/browse/JDK-8224980>
>      >
>      > There are proposals for making the debug message more obvious,
>     and/or
>      > assert().
>      >
>      > Currently we don't have documentation of what the caller of
>      > FLAG_SET_ERGO is supposed to do.
>      >
>      > There are 3 ways to set a flag. The current behavior of the
>     following
>      > two ways seems reasonable
>      >
>      >     FLAG_SET_CMDLINE - rejects user input and exit VM
>      >     FLAG_SET_MGMT    - return JVMFlag::Error
>      >
>      > With command-line, the user should inspect the limits for the
>     current
>      > environment and restart the app with an appropriate value
>      >
>      > With FLAG_SET_MGMT, I suppose it's similar -- whoever is using the
>      > management API should detect the error and adjust their parameter
>     values
>      > (by hand, or programmatically)
>      >
>      > However, FLAG_SET_ERGO also returns JVMFlag::Error, but I don't
>     see any
>      > of our code checking for the return value. So the unwritten
>     convention
>      > is that the call must not fail.
>      > However, if the specified value is out of range, should we abort
>     the VM,
>      > ignore the value, or cap the value? E.g., if the range is 0~100,
>     but you
>      > call SET_ERGO with 101, should we cap the value to 100?
> 
>     Generally if our code tries to sets a flags value to something invalid
>     that is a programming error that should be reported right away. If the
>     value is a "constant" then we should just assert that setting the value
>     worked. If the value is determined by external factors that might vary
>     at runtime (so no guarantee the end result will be valid) then we
>     should
>     have logic that can correct for the invalid value (or we should change
>     the way we do the calculation).
> 
> 
> Agreed, but how do you want to tell them apart?

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.

> 
>      > Perhaps capping would make the ergo code easier to write,
>     especially if
>      > the constraint function produces a dynamic range (depending on the
>      > machine's RAM size, for example). Without capping, you will need to
>      > duplicate (or share) the range detection logic between the ergo
>     setter
>      > and the constraint checker.
> 
> 
> I like capping as a pragmatic solution.
> 
> The only problem I see is for "special" values that are outside the 
> allowed range but still hold information.
> 
> For example, NonNMethodCodeHeapSize is set to 0 via FLAG_SET_ERGO 
> (SegmentedCodeCache off), which gets ignored since it violates 
> VMPageSizeConstraintFunc (valid range starts at 4096).
> 
> Though, while typing, I realized that this maybe is a simple error, and 
> the answer would be to either allow 0 as a value in the constraint 
> function or to not use this constraint function.

Yeah that seems like a flat-out bug in the code.

Cheers,
David

> 
> Thanks, Thomas