RFR: 8224980: FLAG_SET_ERGO silently ignores invalid values [v2]

[David Holmes]

On Tue, 14 Feb 2023 21:59:49 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> src/hotspot/share/compiler/compilerDefinitions.cpp line 134:
>> 
>>> 132: intx CompilerConfig::jvmflag_scaled_freq_log(intx freq_log) {
>>> 133:   return MAX2((intx)0, MIN2(scaled_freq_log(freq_log), (intx)30));
>>> 134: }
>> 
>> If the passed in arguments are out of range this appears to cap them within range but the initial error is never made known to the caller. ??
>
> This code is called only by `CompilerConfig::set_compilation_policy_flags()` to scale 15 flags according to `CompileThresholdScaling`. If the user sets `CompileThresholdScaling` to be too high, then some of the calculated values will be capped.
> 
> @veresov @TobiHartmann - do you think some warning messages are needed here?
> 
> My thinking is that `CompileThresholdScaling` is a somewhat magical knob. If the user really wants to know what's happening, they should add `-XX:+PrintFlagsFinal` and see the effect on all the affected flags.

Okay - whether this should produce warnings is a distinct issue for compiler folk to decide.

>> src/hotspot/share/runtime/flags/jvmFlagAccess.cpp line 71:
>> 
>>> 69:       if (err != JVMFlag::SUCCESS) {
>>> 70:         if (origin == JVMFlagOrigin::ERGONOMIC) {
>>> 71:           fatal("FLAG_SET_ERGO cannot be used to set an invalid value for %s", flag->name());
>> 
>> Just occurred to me, should the `fatal` rather be a `vm_exit_during_initialization`?
>
> `vm_exit_during_initialization` is usually for conditions not controllable by the VM (out of memory, bad command-line options, etc). Here were have a programming error in the VM's ergo code. So `fatal()` is more appropriate. It also generates an hs_err file that can help debugging.

Okay

-------------

PR: https://git.openjdk.org/jdk/pull/12549