RFR: 8366881: Parallel: Obsolete HeapMaximumCompactionInterval [v2]

[Albert Mingkun Yang]

On Mon, 8 Sep 2025 14:40:28 GMT, Francesco Andreuzzi <duke at openjdk.org> wrote:

>> Albert Mingkun Yang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - review
>>  - Merge branch 'master' into pgc-max-compaction-obsolete
>>  - pgc-max-compaction-obsolete
>
> src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 342:
> 
>> 340:   // No need for max-compaction in this context.
>> 341:   const bool should_do_max_compaction = false;
>> 342:   PSParallelCompact::invoke(clear_all_soft_refs, should_do_max_compaction);
> 
> Perhaps this might do too, without the need for an additional variable?
> Suggestion:
> 
>   PSParallelCompact::invoke(clear_all_soft_refs, false /* should_do_max_compaction */);

True; I chose this way mostly for consistency -- `satisfy_failed_allocation` uses local var instead of comment.

> src/hotspot/share/gc/parallel/psParallelCompact.cpp line 829:
> 
>> 827: }
>> 828: 
>> 829: bool PSParallelCompact::check_maximum_compaction(bool should_do_max_compaction,
> 
> I'd expect this method not to have any side effect, does it?
> 
> If that's the case, you could consider not having `should_do_max_compaction` as a parameter, and replace calls to  `check_maximum_compaction` with `should_do_max_compaction || check_maximum_compaction(...)`.

This method is side-effect free. I chose this style so that all criteria on deciding max-compaction are grouped in a single location.

> src/hotspot/share/runtime/arguments.cpp line 571:
> 
>> 569: 
>> 570:   { "PretenureSizeThreshold",       JDK_Version::undefined(), JDK_Version::jdk(26), JDK_Version::jdk(27) },
>> 571:   { "HeapMaximumCompactionInterval",JDK_Version::undefined(), JDK_Version::jdk(26), JDK_Version::jdk(27) },
> 
> Maybe add a space?
> Suggestion:
> 
>   { "HeapMaximumCompactionInterval", JDK_Version::undefined(), JDK_Version::jdk(26), JDK_Version::jdk(27) },

That did cross my mind. However, I went for the current version as I feel aligning with previous line (identical obsolete/expire version) is more important.

WDYT?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27091#discussion_r2330489484
PR Review Comment: https://git.openjdk.org/jdk/pull/27091#discussion_r2330497252
PR Review Comment: https://git.openjdk.org/jdk/pull/27091#discussion_r2330503549