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