RFR: 8365055: G1: Merge Heap Roots phase incorrectly clears young gen remembered set every time [v2]

[Ivan Walulya]

On Fri, 8 Aug 2025 14:12:31 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   please review this fix to G1 heap root merging that cleared the young gen remembered set multiple times (for every merging) and overwriting the most recent statistics for the number of young gen cards.
>> 
>> The latter is probably the more problematic, as it directly impacts young gen sizing statistics, potentially making young gen too large.
>> 
>> The fix is to move the two statements to the proper place - get current number of young gen cards at startup, clear the young gen remset when starting a new collection set - we do not need to do that earlier after all. Additionally we can add some (superfluous?) verification that during GC we really do not add to the young gen remset.
>> 
>> Testing: tier1-5
>> 
>> Thanks,
>>   Thomas
>
> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
> 
>   * fix comment in verification code
>   * move that G1Policy member to G1CollectedHeap as it's used to verify stuff in G1CollectedHeap (and not used elsewhere anyway)

Changes requested by iwalulya (Reviewer).

src/hotspot/share/gc/g1/g1Policy.cpp line 933:

> 931: 
> 932:     _analytics->report_pending_cards((double)pending_cards_at_gc_start(), is_young_only_pause);
> 933:     _analytics->report_card_rs_length((double)_g1h->young_regions_cardset()->occupied(), is_young_only_pause);

At this point in the execution, the `_g1h->young_regions_cardset()->clear()` has already been called, so this will be `0`. 

`post_evacuate_collection_set()` calls `prepare_for_mutator_after_young_collection()`  which calls  `start_new_collection_set()`, but `post_evacuate_collection_set()` is called before `policy()->record_young_collection_end()`

Probably `_num_young_rem_set_cards_at_start` was added to `G1Policy`  to assert on this, but it is not used.

src/hotspot/share/gc/g1/g1RemSet.cpp line 1443:

> 1441:   }
> 1442: 
> 1443:   {

guarding this block with `if (initial_evacuation)`  might be an easier alternative.

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

PR Review: https://git.openjdk.org/jdk/pull/26695#pullrequestreview-3127345332
PR Review Comment: https://git.openjdk.org/jdk/pull/26695#discussion_r2281680822
PR Review Comment: https://git.openjdk.org/jdk/pull/26695#discussion_r2281682289