JVM Flag ergonomics and constraints

[Thomas Stüfe]

On Fri, Feb 3, 2023 at 8:12 AM Ioi Lam <ioi.lam at oracle.com> 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?
>
> 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.
>
> 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 sounds like a pragmatic solution. The CAPPED version may not even be
needed. I expects the asserts mostly to fire for "special" values, as in
the cited case of NonNMethodCodeHeapSize, and point to problems in the
constraint function itself, which should accept special values.

Cheers, Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-dev/attachments/20230203/f8e36055/attachment-0001.htm>