RFR: 8331048: G1: Prune rebuild candidates based on G1HeapWastePercent early [v4]

[Albert Mingkun Yang]

On Fri, 3 May 2024 08:25:26 GMT, Ivan Walulya <iwalulya at openjdk.org> wrote:

>> Hi, 
>> 
>> Please review this change that moves the `G1CollectionSetChooser::build` out of the cleanup pause into the remark pause. This prunes the rebuild candidates based on G1HeapWastePercent, thereby reducing the rebuild time in cases where pruning happens. 
>> 
>> Testing: Tier 1 - 5
>> 
>> ![concurrent_rebuild](https://github.com/openjdk/jdk/assets/69453999/b14dd9d7-24e7-43ed-bf0f-5d3030d9180f)
>
> Ivan Walulya 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 11 additional commits since the last revision:
> 
>  - Merge remote-tracking branch 'upstream/master' into PruneBeforeRebuild
>  - Refactor cleanup
>  - Thomas Review
>  - remove methods
>  - Merge branch 'master' into PruneBeforeRebuild
>  - remove Fixme
>  - cleanup
>  - Merge branch 'master' into GroupOldRegions_Remsets
>  - save
>  - clean up
>  - ... and 1 more: https://git.openjdk.org/jdk/compare/d9723855...a42cfbd1

Now that pruning occurs in `remark`, I wonder if those pruned regions should/can switch to remset-untracked instead.

src/hotspot/share/gc/g1/g1CollectionSetCandidates.cpp line 228:

> 226: }
> 227: 
> 228: void G1CollectionSetCandidates::sort_retained_by_efficiency() {

Why is this renaming? Checking its callers, I feel the postcondition is that both kinds of candidates are sorted -- marking-candidates can be skipped seems an impl detail.

src/hotspot/share/gc/g1/g1CollectionSetCandidates.hpp line 132:

> 130:   // will cause regions with a lot of live objects and large remembered sets to end
> 131:   // up at the end of the list.
> 132:   static int compare_gc_efficiency(G1CollectionSetCandidateInfo* ci1, G1CollectionSetCandidateInfo* ci2);

Preexisting: I wonder if these can be static functions in cpp file, i.e. not in hpp at all.

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

PR Review: https://git.openjdk.org/jdk/pull/18970#pullrequestreview-2037680243
PR Review Comment: https://git.openjdk.org/jdk/pull/18970#discussion_r1588918560
PR Review Comment: https://git.openjdk.org/jdk/pull/18970#discussion_r1588919397