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

Albert Mingkun Yang ayang at openjdk.org
Thu Aug 7 13:15:16 UTC 2025


On Wed, 6 Aug 2025 15:52:47 GMT, Thomas Schatzl <tschatzl 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.

src/hotspot/share/gc/g1/g1CollectionSet.hpp line 152:

> 150:   G1CSetCandidateGroupList _collection_set_groups;
> 151: 
> 152:   uint selected_groups_cur_length() const;

Maybe move this down to methods section so that all fields are next to each other.

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.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26659#discussion_r2260286978
PR Review Comment: https://git.openjdk.org/jdk/pull/26659#discussion_r2260279534
PR Review Comment: https://git.openjdk.org/jdk/pull/26659#discussion_r2260277796


More information about the hotspot-gc-dev mailing list