RFR: 8352969: G1: Improve testability of optional collections [v3]

[Thomas Schatzl]

On Tue, 26 Aug 2025 06:32:56 GMT, Guanqiang Han <ghan at openjdk.org> wrote:

>> This PR introduces a new diagnostic flag EvacuateAllOptionalRegions to force G1 GC to evacuate all optional regions regardless of the predicted pause time. The motivation is to allow testing and validation of optional region evacuation behavior without being constrained by the remaining pause time.
>
> Guanqiang Han has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update g1YoungCollector.cpp
>   
>   format fix

Changes requested by tschatzl (Reviewer).

src/hotspot/share/gc/g1/g1CollectionSet.cpp line 652:

> 650:                             num_regions_selected, optional_regions_count, total_prediction_ms);
> 651:   return num_regions_selected;
> 652: }

Please separate out this cleanup, it has nothing to do with this change.

src/hotspot/share/gc/g1/g1YoungCollector.cpp line 828:

> 826:       double time_used_ms = os::elapsedTime() * 1000.0 - pause_start_time_ms;
> 827:       time_left_ms = MaxGCPauseMillis - time_used_ms;
> 828:     }

Even if it is not 100% technically correct, I would prefer to put the total available time into a local, and set it to either `MaxGCPauseMillis` or `DBL_MAX` outside the loop, and keep the loop as is.

This reduces the complexity of the loop body for the negligible issue in case that option is set.

src/hotspot/share/gc/g1/g1_globals.hpp line 375:

> 373:   product(bool, EvacuateAllOptionalRegions, false, DIAGNOSTIC,              \
> 374:           "Force to evacuate all optional regions.")                        \
> 375:                                                                             \

This new option looks like a `develop` option, not a product option to me since it is explicitly about increasing testing of some functionality.

This change should also have a test that exercises the option (and optional regions if possible). Note that if there were a good test for optional regions, this flag would be unnecessary in the first place.

If you plan to add tests incorporating this flag later it would be great if you could give us a preview for them.

We want to avoid adding flags and logic that is not used somewhere/somehow.

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

PR Review: https://git.openjdk.org/jdk/pull/26880#pullrequestreview-3154308825
PR Review Comment: https://git.openjdk.org/jdk/pull/26880#discussion_r2300054069
PR Review Comment: https://git.openjdk.org/jdk/pull/26880#discussion_r2300109914
PR Review Comment: https://git.openjdk.org/jdk/pull/26880#discussion_r2300172748