RFR: 8354228: Parallel: Set correct minimum of InitialSurvivorRatio [v2]

[Guoxiong Li]

On Tue, 22 Apr 2025 07:51:55 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> src/hotspot/share/gc/parallel/parallelArguments.cpp line 78:
>> 
>>> 76:     } else {
>>> 77:       FLAG_SET_DEFAULT(InitialSurvivorRatio, MinSurvivorRatio);
>>> 78:     }
>> 
>> If both `InitialSurvivorRatio` and `MinSurvivorRatio` are not set in command line and the condition  `InitialSurvivorRatio < MinSurvivorRatio` is true, it seems the corresponding default/ergonomic values, we set before, are wrong. Should we guard this situation (such as printing an error message) to catch the bug (unexpected default/ergonomic values) in the previous code?
>
>> If both InitialSurvivorRatio and MinSurvivorRatio are not set in command line and the condition InitialSurvivorRatio < MinSurvivorRatio is true
> 
> When will that happen? AFAIS, if neither is set on command line, the default values should be MinSurvivorRatio == 3 and InitialSurvivorRatio == 8 (as defined in gc_globals.hpp), so `MinSurvivorRatio <= InitialSurvivorRatio` should hold.
> 
>> Should we guard this situation (such as printing an error message) to catch the bug in the previous code?
> 
> I didn't really understand your suggestion. Could you clarify what you mean by "previous code"? Or maybe some pseudo code to outline your suggestion?

Please read the following code. It can help us catch the bug about the wrong default/ergonomic values.

```C++
  if (InitialSurvivorRatio < MinSurvivorRatio) {
    if (FLAG_IS_CMDLINE(InitialSurvivorRatio)) {
      if (FLAG_IS_CMDLINE(MinSurvivorRatio)) {
        jio_fprintf(defaultStream::error_stream(),
          "Inconsistent MinSurvivorRatio vs InitialSurvivorRatio: %d vs %d\n", MinSurvivorRatio, InitialSurvivorRatio);
      }
      FLAG_SET_DEFAULT(MinSurvivorRatio, InitialSurvivorRatio);
    } else if (FLAG_IS_CMDLINE(MinSurvivorRatio)) {
      FLAG_SET_DEFAULT(InitialSurvivorRatio, MinSurvivorRatio);
    } else {
      // Here <----------
      jio_fprintf(defaultStream::error_stream(),
          "Inconsistent default/ergonomic MinSurvivorRatio vs InitialSurvivorRatio: %d vs %d\n", MinSurvivorRatio, InitialSurvivorRatio);
    }
}


> When will that happen? AFAIS, if neither is set on command line, the default values should be MinSurvivorRatio == 3 and InitialSurvivorRatio == 8 (as defined in gc_globals.hpp), so MinSurvivorRatio <= InitialSurvivorRatio should hold.

Yes, the current default values are good, but we may change them in the future. So such guard operation seems good and helps us find the bug earlier.  May I be overthinking?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24556#discussion_r2055228577