From 02fe095d29994bec28c85beb6bf2a69b0f49b206 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Thu, 21 Aug 2025 11:53:57 +0000 Subject: [PATCH] 8364934: G1: Rename members of G1CollectionSet Reviewed-by: ayang, kbarrett --- src/hotspot/share/gc/g1/g1CollectedHeap.cpp | 2 +- src/hotspot/share/gc/g1/g1CollectionSet.cpp | 101 +++++++++--------- src/hotspot/share/gc/g1/g1CollectionSet.hpp | 88 ++++++++------- .../share/gc/g1/g1CollectionSet.inline.hpp | 6 +- src/hotspot/share/gc/g1/g1RemSet.cpp | 2 +- src/hotspot/share/gc/g1/g1YoungCollector.cpp | 2 +- .../gc/g1/g1YoungGCPostEvacuateTasks.cpp | 2 +- 7 files changed, 107 insertions(+), 96 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp index ed08d43bbbe..e50821e96c1 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp @@ -3159,5 +3159,5 @@ void G1CollectedHeap::finish_codecache_marking_cycle() { void G1CollectedHeap::prepare_group_cardsets_for_scan() { young_regions_cardset()->reset_table_scanner_for_groups(); - collection_set()->prepare_groups_for_scan(); + collection_set()->prepare_for_scan(); } diff --git a/src/hotspot/share/gc/g1/g1CollectionSet.cpp b/src/hotspot/share/gc/g1/g1CollectionSet.cpp index 90189cfdf8a..804ded144bd 100644 --- a/src/hotspot/share/gc/g1/g1CollectionSet.cpp +++ b/src/hotspot/share/gc/g1/g1CollectionSet.cpp @@ -38,13 +38,13 @@ #include "utilities/globalDefinitions.hpp" #include "utilities/quickSort.hpp" -uint G1CollectionSet::selected_groups_cur_length() const { +uint G1CollectionSet::groups_cur_length() const { assert(_inc_build_state == CSetBuildType::Inactive, "must be"); - return _collection_set_groups.length(); + return _groups.length(); } -uint G1CollectionSet::collection_groups_increment_length() const { - return selected_groups_cur_length() - _selected_groups_inc_part_start; +uint G1CollectionSet::groups_increment_length() const { + return groups_cur_length() - _groups_inc_part_start; } G1CollectorState* G1CollectionSet::collector_state() const { @@ -59,21 +59,21 @@ G1CollectionSet::G1CollectionSet(G1CollectedHeap* g1h, G1Policy* policy) : _g1h(g1h), _policy(policy), _candidates(), - _collection_set_regions(nullptr), - _collection_set_cur_length(0), - _collection_set_max_length(0), - _collection_set_groups(), - _selected_groups_inc_part_start(0), + _regions(nullptr), + _regions_max_length(0), + _regions_cur_length(0), + _groups(), _eden_region_length(0), _survivor_region_length(0), _initial_old_region_length(0), _optional_groups(), - _inc_build_state(Inactive), - _inc_part_start(0) { + _inc_build_state(CSetBuildType::Inactive), + _regions_inc_part_start(0), + _groups_inc_part_start(0) { } G1CollectionSet::~G1CollectionSet() { - FREE_C_HEAP_ARRAY(uint, _collection_set_regions); + FREE_C_HEAP_ARRAY(uint, _regions); abandon_all_candidates(); } @@ -84,8 +84,8 @@ void G1CollectionSet::init_region_lengths(uint eden_cset_region_length, _eden_region_length = eden_cset_region_length; _survivor_region_length = survivor_cset_region_length; - assert((size_t)young_region_length() == _collection_set_cur_length, - "Young region length %u should match collection set length %u", young_region_length(), _collection_set_cur_length); + assert((size_t)young_region_length() == _regions_cur_length, + "Young region length %u should match collection set length %u", young_region_length(), _regions_cur_length); _initial_old_region_length = 0; assert(_optional_groups.length() == 0, "Should not have any optional groups yet"); @@ -93,9 +93,9 @@ void G1CollectionSet::init_region_lengths(uint eden_cset_region_length, } void G1CollectionSet::initialize(uint max_region_length) { - guarantee(_collection_set_regions == nullptr, "Must only initialize once."); - _collection_set_max_length = max_region_length; - _collection_set_regions = NEW_C_HEAP_ARRAY(uint, max_region_length, mtGC); + guarantee(_regions == nullptr, "Must only initialize once."); + _regions_max_length = max_region_length; + _regions = NEW_C_HEAP_ARRAY(uint, max_region_length, mtGC); _candidates.initialize(max_region_length); } @@ -105,14 +105,14 @@ void G1CollectionSet::abandon_all_candidates() { _initial_old_region_length = 0; } -void G1CollectionSet::prepare_groups_for_scan () { - collection_set_groups()->prepare_for_scan(); +void G1CollectionSet::prepare_for_scan () { + groups()->prepare_for_scan(); } void G1CollectionSet::add_old_region(G1HeapRegion* hr) { assert_at_safepoint_on_vm_thread(); - assert(_inc_build_state == Active, + assert(_inc_build_state == CSetBuildType::Active, "Precondition, actively building cset or adding optional later on"); assert(hr->is_old(), "the region should be old"); @@ -121,46 +121,46 @@ void G1CollectionSet::add_old_region(G1HeapRegion* hr) { assert(!hr->in_collection_set(), "should not already be in the collection set"); _g1h->register_old_region_with_region_attr(hr); - assert(_collection_set_cur_length < _collection_set_max_length, "Collection set now larger than maximum size."); - _collection_set_regions[_collection_set_cur_length++] = hr->hrm_index(); + assert(_regions_cur_length < _regions_max_length, "Collection set now larger than maximum size."); + _regions[_regions_cur_length++] = hr->hrm_index(); _initial_old_region_length++; _g1h->old_set_remove(hr); } void G1CollectionSet::start_incremental_building() { - assert(_collection_set_cur_length == 0, "Collection set must be empty before starting a new collection set."); - assert(selected_groups_cur_length() == 0, "Collection set groups must be empty before starting a new collection set."); + assert(_regions_cur_length == 0, "Collection set must be empty before starting a new collection set."); + assert(groups_cur_length() == 0, "Collection set groups must be empty before starting a new collection set."); assert(_optional_groups.length() == 0, "Collection set optional gorups must be empty before starting a new collection set."); continue_incremental_building(); } void G1CollectionSet::continue_incremental_building() { - assert(_inc_build_state == Inactive, "Precondition"); + assert(_inc_build_state == CSetBuildType::Inactive, "Precondition"); - _inc_part_start = _collection_set_cur_length; - _selected_groups_inc_part_start = selected_groups_cur_length(); + _regions_inc_part_start = _regions_cur_length; + _groups_inc_part_start = groups_cur_length(); _inc_build_state = CSetBuildType::Active; } void G1CollectionSet::stop_incremental_building() { - _inc_build_state = Inactive; + _inc_build_state = CSetBuildType::Inactive; } void G1CollectionSet::clear() { assert_at_safepoint_on_vm_thread(); - _collection_set_cur_length = 0; - _collection_set_groups.clear(); + _regions_cur_length = 0; + _groups.clear(); } void G1CollectionSet::iterate(G1HeapRegionClosure* cl) const { - size_t len = _collection_set_cur_length; + size_t len = _regions_cur_length; OrderAccess::loadload(); for (uint i = 0; i < len; i++) { - G1HeapRegion* r = _g1h->region_at(_collection_set_regions[i]); + G1HeapRegion* r = _g1h->region_at(_regions[i]); bool result = cl->do_heap_region(r); if (result) { cl->set_incomplete(); @@ -187,7 +187,7 @@ void G1CollectionSet::iterate_optional(G1HeapRegionClosure* cl) const { void G1CollectionSet::iterate_incremental_part_from(G1HeapRegionClosure* cl, G1HeapRegionClaimer* hr_claimer, uint worker_id) const { - iterate_part_from(cl, hr_claimer, _inc_part_start, increment_length(), worker_id); + iterate_part_from(cl, hr_claimer, _regions_inc_part_start, regions_cur_length(), worker_id); } void G1CollectionSet::iterate_part_from(G1HeapRegionClosure* cl, @@ -197,29 +197,29 @@ void G1CollectionSet::iterate_part_from(G1HeapRegionClosure* cl, uint worker_id) const { _g1h->par_iterate_regions_array(cl, hr_claimer, - &_collection_set_regions[offset], + &_regions[offset], length, worker_id); } void G1CollectionSet::add_young_region_common(G1HeapRegion* hr) { assert(hr->is_young(), "invariant"); - assert(_inc_build_state == Active, "Precondition"); + assert(_inc_build_state == CSetBuildType::Active, "Precondition"); assert(!hr->in_collection_set(), "invariant"); _g1h->register_young_region_with_region_attr(hr); // We use UINT_MAX as "invalid" marker in verification. - assert(_collection_set_cur_length < (UINT_MAX - 1), - "Collection set is too large with %u entries", _collection_set_cur_length); - hr->set_young_index_in_cset(_collection_set_cur_length + 1); + assert(_regions_cur_length < (UINT_MAX - 1), + "Collection set is too large with %u entries", _regions_cur_length); + hr->set_young_index_in_cset(_regions_cur_length + 1); - assert(_collection_set_cur_length < _collection_set_max_length, "Collection set larger than maximum allowed."); - _collection_set_regions[_collection_set_cur_length] = hr->hrm_index(); + assert(_regions_cur_length < _regions_max_length, "Collection set larger than maximum allowed."); + _regions[_regions_cur_length] = hr->hrm_index(); // Concurrent readers must observe the store of the value in the array before an // update to the length field. OrderAccess::storestore(); - _collection_set_cur_length++; + _regions_cur_length++; } void G1CollectionSet::add_survivor_regions(G1HeapRegion* hr) { @@ -301,7 +301,7 @@ void G1CollectionSet::print(outputStream* st) { // pinned by JNI) to allow faster future evacuation. We already "paid" for this work // when sizing the young generation. double G1CollectionSet::finalize_young_part(double target_pause_time_ms, G1SurvivorRegions* survivors) { - assert(_inc_build_state == Active, "Precondition"); + assert(_inc_build_state == CSetBuildType::Active, "Precondition"); assert(SafepointSynchronize::is_at_safepoint(), "should be at a safepoint"); Ticks start_time = Ticks::now(); @@ -626,7 +626,8 @@ double G1CollectionSet::select_candidates_from_optional_groups(double time_remai selected.append(group); } - log_debug(gc, ergo, cset) ("Completed with groups, selected %u", num_regions_selected); + log_debug(gc, ergo, cset)("Completed with groups, selected %u region in %u groups", + num_regions_selected, num_groups_selected); // Remove selected groups from candidate list. if (num_groups_selected > 0) { _optional_groups.remove(&selected); @@ -635,7 +636,7 @@ double G1CollectionSet::select_candidates_from_optional_groups(double time_remai return total_prediction_ms; } -uint G1CollectionSet::select_optional_collection_set_regions(double time_remaining_ms) { +uint G1CollectionSet::select_optional_groups(double time_remaining_ms) { uint optional_regions_count = num_optional_regions(); assert(optional_regions_count > 0, "Should only be called when there are optional regions"); @@ -670,7 +671,7 @@ void G1CollectionSet::add_group_to_collection_set(G1CSetCandidateGroup* gr) { assert(r->rem_set()->is_complete(), "must be"); add_region_to_collection_set(r); } - _collection_set_groups.append(gr); + _groups.append(gr); } void G1CollectionSet::add_region_to_collection_set(G1HeapRegion* r) { @@ -680,20 +681,20 @@ void G1CollectionSet::add_region_to_collection_set(G1HeapRegion* r) { } void G1CollectionSet::finalize_initial_collection_set(double target_pause_time_ms, G1SurvivorRegions* survivor) { - assert(_inc_part_start == 0, "must be"); - assert(_selected_groups_inc_part_start == 0, "must be"); + assert(_regions_inc_part_start == 0, "must be"); + assert(_groups_inc_part_start == 0, "must be"); double time_remaining_ms = finalize_young_part(target_pause_time_ms, survivor); finalize_old_part(time_remaining_ms); stop_incremental_building(); - QuickSort::sort(_collection_set_regions, _collection_set_cur_length, compare_region_idx); + QuickSort::sort(_regions, _regions_cur_length, compare_region_idx); } bool G1CollectionSet::finalize_optional_for_evacuation(double remaining_pause_time) { continue_incremental_building(); - uint num_regions_selected = select_optional_collection_set_regions(remaining_pause_time); + uint num_regions_selected = select_optional_groups(remaining_pause_time); stop_incremental_building(); @@ -756,7 +757,7 @@ public: void G1CollectionSet::verify_young_cset_indices() const { assert_at_safepoint_on_vm_thread(); - G1VerifyYoungCSetIndicesClosure cl(_collection_set_cur_length); + G1VerifyYoungCSetIndicesClosure cl(_regions_cur_length); iterate(&cl); } #endif diff --git a/src/hotspot/share/gc/g1/g1CollectionSet.hpp b/src/hotspot/share/gc/g1/g1CollectionSet.hpp index e12f3a82d12..2bde960fbc4 100644 --- a/src/hotspot/share/gc/g1/g1CollectionSet.hpp +++ b/src/hotspot/share/gc/g1/g1CollectionSet.hpp @@ -41,19 +41,19 @@ class G1HeapRegionClosure; // The collection set. // -// The set of regions that are evacuated during an evacuation pause. +// The set of regions and candidate groups that were evacuated during an +// evacuation pause. // -// At the end of a collection, before freeing the collection set, this set -// contains all regions that were evacuated during this collection: +// At the end of a collection, before freeing it, this set contains all regions +// and collection set groups that were evacuated during this collection: // // - survivor regions from the last collection (if any) // - eden regions allocated by the mutator // - old gen regions evacuated during mixed gc // -// This set is built incrementally at mutator time as regions are retired, and -// if this had been a mixed gc, some additional (during gc) incrementally added -// old regions from the collection set candidates built during the concurrent -// cycle. +// This set is initially built at mutator time as regions are retired. If the +// collection is a mixed gc, it contains some additional (during the pause) +// incrementally added old regions from the collection set candidates. // // A more detailed overview of how the collection set changes over time follows: // @@ -129,6 +129,7 @@ class G1HeapRegionClosure; // || ... after step b6) // |SSS| ... after step 7), with three survivor regions // +// Candidate groups are kept in sync with the contents of the collection set regions. class G1CollectionSet { G1CollectedHeap* _g1h; G1Policy* _policy; @@ -137,46 +138,52 @@ class G1CollectionSet { G1CollectionSetCandidates _candidates; // The actual collection set as a set of region indices. - // All entries in _collection_set_regions below _collection_set_cur_length are - // assumed to be part of the collection set. + // + // All regions in _regions below _regions_cur_length are assumed to be part of the + // collection set. // We assume that at any time there is at most only one writer and (one or more) - // concurrent readers. This means we are good with using storestore and loadload - // barriers on the writer and reader respectively only. - uint* _collection_set_regions; - volatile uint _collection_set_cur_length; - uint _collection_set_max_length; + // concurrent readers. This means synchronization using storestore and loadload + // barriers on the writer and reader respectively only are sufficient. + // + // This corresponds to the regions referenced by the candidate groups further below. + uint* _regions; + uint _regions_max_length; + + volatile uint _regions_cur_length; // Old gen groups selected for evacuation. - G1CSetCandidateGroupList _collection_set_groups; + G1CSetCandidateGroupList _groups; - uint selected_groups_cur_length() const; - uint _selected_groups_inc_part_start; + uint groups_cur_length() const; uint _eden_region_length; uint _survivor_region_length; uint _initial_old_region_length; // When doing mixed collections we can add old regions to the collection set, which - // will be collected only if there is enough time. We call these optional (old) regions. + // will be collected only if there is enough time. We call these optional (old) + // groups. Regions are reachable via this list as well. G1CSetCandidateGroupList _optional_groups; - enum CSetBuildType { + enum class CSetBuildType { Active, // We are actively building the collection set Inactive // We are not actively building the collection set }; CSetBuildType _inc_build_state; - size_t _inc_part_start; + // Index into the _regions indicating the start of the current collection set increment. + size_t _regions_inc_part_start; + // Index into the _groups indicating the start of the current collection set increment. + uint _groups_inc_part_start; G1CollectorState* collector_state() const; G1GCPhaseTimes* phase_times(); void verify_young_cset_indices() const NOT_DEBUG_RETURN; - // Update the incremental collection set information when adding a region. void add_young_region_common(G1HeapRegion* hr); - // Add the given old region to the head of the current collection set. + // Add the given old region to the current collection set. void add_old_region(G1HeapRegion* hr); void prepare_optional_group(G1CSetCandidateGroup* gr, uint cur_index); @@ -189,14 +196,14 @@ class G1CollectionSet { void select_candidates_from_retained(double time_remaining_ms); - // Select regions for evacuation from the optional candidates given the remaining time - // and return the number of actually selected regions. - uint select_optional_collection_set_regions(double time_remaining_ms); - double select_candidates_from_optional_groups(double time_remaining_ms, uint& num_regions_selected); + // Select groups for evacuation from the optional candidates given the remaining time + // and return the number of actually selected regions. + uint select_optional_groups(double time_remaining_ms); + double select_candidates_from_optional_groups(double time_remaining_ms, uint& num_groups_selected); // Finalize the young part of the initial collection set. Relabel survivor regions // as Eden and calculate a prediction on how long the evacuation of all young regions - // will take. + // will take. Returns the time remaining from the given target pause time. double finalize_young_part(double target_pause_time_ms, G1SurvivorRegions* survivors); // Select the regions comprising the initial and optional collection set from marking @@ -218,27 +225,29 @@ public: // Initializes the collection set giving the maximum possible length of the collection set. void initialize(uint max_region_length); + // Drop all collection set candidates (only the candidates). void abandon_all_candidates(); G1CollectionSetCandidates* candidates() { return &_candidates; } const G1CollectionSetCandidates* candidates() const { return &_candidates; } - G1CSetCandidateGroupList* collection_set_groups() { return &_collection_set_groups; } - const G1CSetCandidateGroupList* collection_set_groups() const { return &_collection_set_groups; } + G1CSetCandidateGroupList* groups() { return &_groups; } + const G1CSetCandidateGroupList* groups() const { return &_groups; } - void prepare_groups_for_scan(); + void prepare_for_scan(); void init_region_lengths(uint eden_cset_region_length, uint survivor_cset_region_length); - uint region_length() const { return young_region_length() + - initial_old_region_length(); } + // Total length of the initial collection set in regions. + uint initial_region_length() const { return young_region_length() + + initial_old_region_length(); } uint young_region_length() const { return eden_region_length() + survivor_region_length(); } - uint eden_region_length() const { return _eden_region_length; } + uint eden_region_length() const { return _eden_region_length; } uint survivor_region_length() const { return _survivor_region_length; } - uint initial_old_region_length() const { return _initial_old_region_length; } + uint initial_old_region_length() const { return _initial_old_region_length; } uint num_optional_regions() const { return _optional_groups.num_regions(); } bool only_contains_young_regions() const { return (initial_old_region_length() + num_optional_regions()) == 0; } @@ -263,14 +272,14 @@ public: void iterate_incremental_part_from(G1HeapRegionClosure* cl, G1HeapRegionClaimer* hr_claimer, uint worker_id) const; // Returns the length of the current increment in number of regions. - size_t increment_length() const { return _collection_set_cur_length - _inc_part_start; } + size_t regions_cur_length() const { return _regions_cur_length - _regions_inc_part_start; } // Returns the length of the whole current collection set in number of regions - size_t cur_length() const { return _collection_set_cur_length; } + size_t cur_length() const { return _regions_cur_length; } - uint collection_groups_increment_length() const; + uint groups_increment_length() const; // Iterate over the entire collection set (all increments calculated so far), applying - // the given G1HeapRegionClosure on all of them. + // the given G1HeapRegionClosure on all of the regions. void iterate(G1HeapRegionClosure* cl) const; void par_iterate(G1HeapRegionClosure* cl, G1HeapRegionClaimer* hr_claimer, @@ -278,10 +287,11 @@ public: void iterate_optional(G1HeapRegionClosure* cl) const; - // Finalize the initial collection set consisting of all young regions potentially a + // Finalize the initial collection set consisting of all young regions and potentially a // few old gen regions. void finalize_initial_collection_set(double target_pause_time_ms, G1SurvivorRegions* survivor); // Finalize the next collection set from the set of available optional old gen regions. + // Returns whether there still were some optional regions. bool finalize_optional_for_evacuation(double remaining_pause_time); // Abandon (clean up) optional collection set regions that were not evacuated in this // pause. diff --git a/src/hotspot/share/gc/g1/g1CollectionSet.inline.hpp b/src/hotspot/share/gc/g1/g1CollectionSet.inline.hpp index 717f6860eb6..0f166cdf9ff 100644 --- a/src/hotspot/share/gc/g1/g1CollectionSet.inline.hpp +++ b/src/hotspot/share/gc/g1/g1CollectionSet.inline.hpp @@ -31,8 +31,8 @@ template inline void G1CollectionSet::merge_cardsets_for_collection_groups(CardOrRangeVisitor& cl, uint worker_id, uint num_workers) { - uint length = collection_groups_increment_length(); - uint offset = _selected_groups_inc_part_start; + uint length = groups_increment_length(); + uint offset = _groups_inc_part_start; if (length == 0) { return; } @@ -41,7 +41,7 @@ inline void G1CollectionSet::merge_cardsets_for_collection_groups(CardOrRangeVis uint cur_pos = start_pos; uint count = 0; do { - G1HeapRegionRemSet::iterate_for_merge(collection_set_groups()->at(offset + cur_pos)->card_set(), cl); + G1HeapRegionRemSet::iterate_for_merge(groups()->at(offset + cur_pos)->card_set(), cl); cur_pos++; count++; if (cur_pos == length) { diff --git a/src/hotspot/share/gc/g1/g1RemSet.cpp b/src/hotspot/share/gc/g1/g1RemSet.cpp index 44b3234d26b..6b7532a99aa 100644 --- a/src/hotspot/share/gc/g1/g1RemSet.cpp +++ b/src/hotspot/share/gc/g1/g1RemSet.cpp @@ -1426,7 +1426,7 @@ void G1RemSet::merge_heap_roots(bool initial_evacuation) { } WorkerThreads* workers = g1h->workers(); - size_t const increment_length = g1h->collection_set()->increment_length(); + size_t const increment_length = g1h->collection_set()->regions_cur_length(); uint const num_workers = initial_evacuation ? workers->active_workers() : MIN2(workers->active_workers(), (uint)increment_length); diff --git a/src/hotspot/share/gc/g1/g1YoungCollector.cpp b/src/hotspot/share/gc/g1/g1YoungCollector.cpp index c61c766bbdc..29eab44d4a8 100644 --- a/src/hotspot/share/gc/g1/g1YoungCollector.cpp +++ b/src/hotspot/share/gc/g1/g1YoungCollector.cpp @@ -271,7 +271,7 @@ void G1YoungCollector::calculate_collection_set(G1EvacInfo* evacuation_info, dou allocator()->release_mutator_alloc_regions(); collection_set()->finalize_initial_collection_set(target_pause_time_ms, survivor_regions()); - evacuation_info->set_collection_set_regions(collection_set()->region_length() + + evacuation_info->set_collection_set_regions(collection_set()->initial_region_length() + collection_set()->num_optional_regions()); concurrent_mark()->verify_no_collection_set_oops(); diff --git a/src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp b/src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp index bb567e2af5c..b13e7b8a62f 100644 --- a/src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp +++ b/src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp @@ -887,7 +887,7 @@ public: p->record_serial_free_cset_time_ms((Ticks::now() - serial_time).seconds() * 1000.0); } - double worker_cost() const override { return G1CollectedHeap::heap()->collection_set()->region_length(); } + double worker_cost() const override { return G1CollectedHeap::heap()->collection_set()->initial_region_length(); } void set_max_workers(uint max_workers) override { _active_workers = max_workers;