RFR: 8364925: G1: Improve program flow around incremental collection set building

[Thomas Schatzl]

On Thu, 7 Aug 2025 13:12:35 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Hi all,
>> 
>>   please review this change that improves the G1CollectionSet incremental state handling slightly:
>> 
>> * remove `_selected_groups_cur_length` with getter as we do not need a copy of that value around
>> * move implementations of some methods from header files into cpp file
>> * moved some incremental state updates from `finalize_old_part` to the respective `start/continue/stop_incremental_building` method
>> 
>> Testing: gha
>> 
>> Thanks,
>>   Thomas
>
> src/hotspot/share/gc/g1/g1CollectionSet.cpp line 133:
> 
>> 131: void G1CollectionSet::start_incremental_building() {
>> 132:   assert(_collection_set_cur_length == 0, "Collection set must be empty before starting a new collection set.");
>> 133: 
> 
> Can we assert `selected_groups_cur_length() == 0` also? If so, then maybe it's best to inline `continue_incremental_building` with explicit zero-setting.

Also adding `_optional_groups().length() == 0` here. I think calling `continue_increment...` is just fine; first start the collection set building as a whole, then start a new (first) increment within that. Maybe the name `continue_...` should be improved?

> src/hotspot/share/gc/g1/g1CollectionSet.hpp line 238:
> 
>> 236: 
>> 237:   uint region_length() const       { return young_region_length() +
>> 238:                                               initial_old_region_length(); }
> 
> Seems not needed.

When removing the other change I accidentally merged, I forgot that hunk :(

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26659#discussion_r2260615477
PR Review Comment: https://git.openjdk.org/jdk/pull/26659#discussion_r2260613241