RFR: 8343782: G1: Use one G1CardSet instance for multiple old gen regions [v3]

[Ivan Walulya]

On Tue, 3 Dec 2024 12:29:01 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Ivan Walulya has updated the pull request incrementally with four additional commits since the last revision:
>> 
>>  - Update src/hotspot/share/gc/g1/g1CollectionSet.cpp
>>    
>>    Co-authored-by: Thomas Schatzl <59967451+tschatzl at users.noreply.github.com>
>>  - Update src/hotspot/share/gc/g1/g1_globals.hpp
>>    
>>    Co-authored-by: Thomas Schatzl <59967451+tschatzl at users.noreply.github.com>
>>  - Update src/hotspot/share/gc/g1/g1CollectionSetCandidates.hpp
>>    
>>    Co-authored-by: Thomas Schatzl <59967451+tschatzl at users.noreply.github.com>
>>  - Update src/hotspot/share/gc/g1/g1CollectionSet.cpp
>>    
>>    Co-authored-by: Thomas Schatzl <59967451+tschatzl at users.noreply.github.com>
>
> src/hotspot/share/gc/g1/g1CardSet.cpp line 788:
> 
>> 786:   G1HeapRegion* r = G1CollectedHeap::heap()->region_at(region_idx);
>> 787:   assert(r->rem_set()->card_set() != this, "must be");
>> 788: #endif
> 
> Since this introduces local vars, can they be grouped in a `{}` scope?

It's possible, but I have not seen this done in the hotspot code.

> src/hotspot/share/gc/g1/g1CollectionSetCandidates.cpp line 320:
> 
>> 318:   }
>> 319: 
>> 320:   _from_marking_groups.append(current);
> 
> I wonder if this part can be written somehow to eliminate some "duplicate" code, so that the following occur only once.
> 
> 
> _from_marking_groups.append(current);
> current = new G1CSetCandidateGroup(G1CollectedHeap::heap()->card_set_config());
> num_added_to_group = 0;

Suggestions are welcome, I failed to find a way to handle the corner case.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22015#discussion_r1868289268
PR Review Comment: https://git.openjdk.org/jdk/pull/22015#discussion_r1868289641