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

[Thomas Schatzl]

On Fri, 6 Dec 2024 21:46:08 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Ivan Walulya has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Albert Review
>
> src/hotspot/share/gc/g1/g1CollectionSet.cpp line 655:
> 
>> 653:     G1HeapRegion* r = ci._r;
>> 654:     r->uninstall_group_cardset();
>> 655:     r->rem_set()->set_state_complete();
> 
> Why changing the remset state here? I'd expect it's already complete; otherwise, how can it be added to cset?

Maybe change to assert?

> src/hotspot/share/gc/g1/g1CollectionSetCandidates.cpp line 38:
> 
>> 36: { }
>> 37: 
>> 38: void G1CSetCandidateGroup::add(G1HeapRegion* hr) {
> 
> I believe this method is only for retained regions; if so, one can make that explicit by naming it sth like `add_region_region`.

(Probably `add_retained_region` was meant here?)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22015#discussion_r1883608612
PR Review Comment: https://git.openjdk.org/jdk/pull/22015#discussion_r1883620757