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