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

[David Holmes]

On Tue, 14 Feb 2023 05:15:15 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Clarify the `FLAG_SET_ERGO` API - the caller must ensure that a valid value is passed. Added an assert to check this.
>> 
>> Fixed two issues found by the assert:
>> 
>> - `NonNMethodCodeHeapSize`
>> - `XXXThreshold` and `XXXNotifyFreqLog` flags in `CompilerConfig::set_compilation_policy_flags()`
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   @dholmes-ora comment - make sure errors are printed for FLAG_SET_ERGO

src/hotspot/share/code/codeCache.cpp line 1197:

> 1195:   } else {
> 1196:     // Use a single code heap
> 1197:     FLAG_SET_ERGO(NonNMethodCodeHeapSize, (uintx)os::vm_page_size());

Sanity check: this code does execute after `os::init()` doesn't it?

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. ??

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`?

src/hotspot/share/runtime/globals_extension.hpp line 92:

> 90: 
> 91: // FLAG_SET_ERGO must be always be called with a valid value. You cannot pass in a unvalidated
> 92: // value and expect a return code for success/failure.

Suggestion:

// FLAG_SET_ERGO must be always be called with a valid value. If an invalid value
// is detected then the VM will exit.

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

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