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

[Thomas Schatzl]

On Thu, 2 May 2024 11:14:09 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 incrementally with one additional commit since the last revision:
> 
>   Thomas Review

Changes requested by tschatzl (Reviewer).

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

> 95: #endif
> 96: 
> 97: int G1CollectionCandidateList::compare(G1CollectionSetCandidateInfo* ci1, G1CollectionSetCandidateInfo* ci2) {

I would prefer if the old comparison function would be renamed as well similarly to the other, e.g. `compare_by_efficiencies`.

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

> 127:   if (reclaimable1 == reclaimable2) return 0;
> 128:   if (reclaimable1 > reclaimable2) return -1;
> 129:   return 1;

Please use the same style for this if-elseif-else cascade than for the other one.

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

> 200:     (*iter)->_gc_efficiency = hr->calc_gc_efficiency();
> 201:   }
> 202:   _marking_regions.sort_by_efficiency();

It is a bit surprising that this method also sorts by efficiency. I would almost say that the sorting is more important than the calculations.
I.e. something like `sort_by_gc_efficiency` seems better.

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

PR Review: https://git.openjdk.org/jdk/pull/18970#pullrequestreview-2035871363
PR Review Comment: https://git.openjdk.org/jdk/pull/18970#discussion_r1587711377
PR Review Comment: https://git.openjdk.org/jdk/pull/18970#discussion_r1587712415
PR Review Comment: https://git.openjdk.org/jdk/pull/18970#discussion_r1587719204