RFR: 8351405: G1: Collection set early pruning causes suboptimal region selection [v2]

[Albert Mingkun Yang]

On Wed, 19 Mar 2025 08:32:49 GMT, Ivan Walulya <iwalulya at openjdk.org> wrote:

>> Hi all,
>> 
>> Please review this change which aims to reduce spikes in mixed GCs, especially the last mixed-gc in a mixed cycle. Currently, G1 sorts regions identified for collection by reclaimable bytes, then prunes the list removing regions that with the lowest amount of reclaimable bytes. The pruned list is then split into collection groups which are later sorted on gc-efficiency. 
>> 
>> In the cachestress benchmark, we run into a case where some regions contain onlya  few live objects but having many incoming references from other regions.  These regions very expensive collect (low gc-efficiency).
>> 
>> This patch improves the pruning process by tracking incoming references to regions during marking. Instead of pruning based on reclaimable bytes alone, we estimate GC efficiency beforehand and prune regions with the worst GC efficiency.
>> 
>> This reduces the spikes in gc pause time as shown for cachestress benchmark in the image below. 
>> 
>> ![mixed-gc](https://github.com/user-attachments/assets/740fb51d-eb20-4946-bf90-4eef23afe2e4)
>> 
>> 
>> Testing: Tier 1-3.
>
> Ivan Walulya has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Thomas Review

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

> 148: 
> 149:   size_t reclaimable1 = ci1->_r->gc_efficiency();
> 150:   size_t reclaimable2 = ci2->_r->gc_efficiency();

Do the var names need to be updated?

src/hotspot/share/gc/g1/g1HeapRegion.cpp line 357:

> 355: double G1HeapRegion::gc_efficiency() {
> 356:   return reclaimable_bytes() / total_based_on_incoming_refs_ms();
> 357: }

I wonder if `total_based_on_incoming_refs_ms` can be inlined to its single caller. Also, these logic doesn't belong to a heap-region -- maybe `G1Policy`, since most of code use `p->`?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24076#discussion_r2009987868
PR Review Comment: https://git.openjdk.org/jdk/pull/24076#discussion_r2009986535