From e4aed95cac343f1339b9bc87721561bdc4c2f5ad Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Tue, 4 Nov 2025 08:48:48 +0000 Subject: [PATCH] 8370682: G1: Survivor regions not in young gen cset group Reviewed-by: iwalulya, ayang --- src/hotspot/share/gc/g1/g1CollectedHeap.cpp | 28 +++++++++++-------- src/hotspot/share/gc/g1/g1CollectionSet.cpp | 9 +++++- .../share/gc/g1/g1FullGCResetMetadataTask.cpp | 7 ++--- .../gc/g1/g1ParScanThreadState.inline.hpp | 8 ++++-- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp index 72a1ae0e2a6..90b38a96d67 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp @@ -2563,10 +2563,12 @@ void G1CollectedHeap::verify_region_attr_is_remset_tracked() { public: virtual bool do_heap_region(G1HeapRegion* r) { G1CollectedHeap* g1h = G1CollectedHeap::heap(); - const bool is_remset_tracked = g1h->region_attr(r->bottom()).is_remset_tracked(); - assert(r->rem_set()->is_tracked() == is_remset_tracked, - "Region %u remset tracking status (%s) different to region attribute (%s)", - r->hrm_index(), BOOL_TO_STR(r->rem_set()->is_tracked()), BOOL_TO_STR(is_remset_tracked)); + G1HeapRegionAttr attr = g1h->region_attr(r->bottom()); + bool const is_remset_tracked = attr.is_remset_tracked(); + assert((r->rem_set()->is_tracked() == is_remset_tracked) || + (attr.is_new_survivor() && is_remset_tracked), + "Region %u (%s) remset tracking status (%s) different to region attribute (%s)", + r->hrm_index(), r->get_type_str(), BOOL_TO_STR(r->rem_set()->is_tracked()), BOOL_TO_STR(is_remset_tracked)); return false; } } cl; @@ -3105,9 +3107,8 @@ G1HeapRegion* G1CollectedHeap::new_mutator_alloc_region(size_t word_size, new_alloc_region->set_eden(); _eden.add(new_alloc_region); _policy->set_region_eden(new_alloc_region); - _policy->remset_tracker()->update_at_allocate(new_alloc_region); - // Install the group cardset. - young_regions_cset_group()->add(new_alloc_region); + + collection_set()->add_eden_region(new_alloc_region); G1HeapRegionPrinter::alloc(new_alloc_region); return new_alloc_region; } @@ -3120,7 +3121,6 @@ void G1CollectedHeap::retire_mutator_alloc_region(G1HeapRegion* alloc_region, assert_heap_locked_or_at_safepoint(true /* should_be_vm_thread */); assert(alloc_region->is_eden(), "all mutator alloc regions should be eden"); - collection_set()->add_eden_region(alloc_region); increase_used(allocated_bytes); _eden.add_used_bytes(allocated_bytes); G1HeapRegionPrinter::retire(alloc_region); @@ -3164,14 +3164,20 @@ G1HeapRegion* G1CollectedHeap::new_gc_alloc_region(size_t word_size, G1HeapRegio if (type.is_survivor()) { new_alloc_region->set_survivor(); _survivor.add(new_alloc_region); + // The remembered set/group cardset for this region will be installed at the + // end of GC. Cannot do that right now because we still need the current young + // gen cardset group. + // However, register with the attribute table to collect remembered set entries + // immediately as it is the only source for determining the need for remembered + // set tracking during GC. register_new_survivor_region_with_region_attr(new_alloc_region); - // Install the group cardset. - young_regions_cset_group()->add(new_alloc_region); } else { new_alloc_region->set_old(); + // Update remembered set/cardset. + _policy->remset_tracker()->update_at_allocate(new_alloc_region); + // Synchronize with region attribute table. update_region_attr(new_alloc_region); } - _policy->remset_tracker()->update_at_allocate(new_alloc_region); G1HeapRegionPrinter::alloc(new_alloc_region); return new_alloc_region; } diff --git a/src/hotspot/share/gc/g1/g1CollectionSet.cpp b/src/hotspot/share/gc/g1/g1CollectionSet.cpp index eb50b7ae5be..abfddf860e6 100644 --- a/src/hotspot/share/gc/g1/g1CollectionSet.cpp +++ b/src/hotspot/share/gc/g1/g1CollectionSet.cpp @@ -163,6 +163,7 @@ void G1CollectionSet::clear() { assert_at_safepoint_on_vm_thread(); _regions_cur_length = 0; _groups.clear(); + assert(_optional_groups.length() == 0, "must be"); } void G1CollectionSet::iterate(G1HeapRegionClosure* cl) const { @@ -216,7 +217,11 @@ void G1CollectionSet::add_young_region_common(G1HeapRegion* hr) { assert(hr->is_young(), "invariant"); assert(_inc_build_state == CSetBuildType::Active, "Precondition"); - assert(!hr->in_collection_set(), "invariant"); + // Add to remembered set/cardset group. + _g1h->policy()->remset_tracker()->update_at_allocate(hr); + _g1h->young_regions_cset_group()->add(hr); + + // Synchronize with the region attribute table. _g1h->register_young_region_with_region_attr(hr); // We use UINT_MAX as "invalid" marker in verification. @@ -233,11 +238,13 @@ void G1CollectionSet::add_young_region_common(G1HeapRegion* hr) { } void G1CollectionSet::add_survivor_regions(G1HeapRegion* hr) { + assert_at_safepoint_on_vm_thread(); assert(hr->is_survivor(), "Must only add survivor regions, but is %s", hr->get_type_str()); add_young_region_common(hr); } void G1CollectionSet::add_eden_region(G1HeapRegion* hr) { + assert_heap_locked_or_at_safepoint(true /* should_be_vm_thread */); assert(hr->is_eden(), "Must only add eden regions, but is %s", hr->get_type_str()); add_young_region_common(hr); } diff --git a/src/hotspot/share/gc/g1/g1FullGCResetMetadataTask.cpp b/src/hotspot/share/gc/g1/g1FullGCResetMetadataTask.cpp index 02397392a6e..c8fba3459aa 100644 --- a/src/hotspot/share/gc/g1/g1FullGCResetMetadataTask.cpp +++ b/src/hotspot/share/gc/g1/g1FullGCResetMetadataTask.cpp @@ -31,16 +31,13 @@ G1FullGCResetMetadataTask::G1ResetMetadataClosure::G1ResetMetadataClosure(G1Full _collector(collector) { } void G1FullGCResetMetadataTask::G1ResetMetadataClosure::reset_region_metadata(G1HeapRegion* hr) { + assert(hr->is_humongous() || !hr->rem_set()->has_cset_group(), + "Non-humongous regions must not have cset group"); hr->rem_set()->clear(); hr->clear_both_card_tables(); } bool G1FullGCResetMetadataTask::G1ResetMetadataClosure::do_heap_region(G1HeapRegion* hr) { - if (!hr->is_humongous()) { - hr->uninstall_cset_group(); - } - - uint const region_idx = hr->hrm_index(); if (!_collector->is_compaction_target(region_idx)) { assert(!hr->is_free(), "all free regions should be compaction targets"); diff --git a/src/hotspot/share/gc/g1/g1ParScanThreadState.inline.hpp b/src/hotspot/share/gc/g1/g1ParScanThreadState.inline.hpp index 5e827a7fd60..854c341f720 100644 --- a/src/hotspot/share/gc/g1/g1ParScanThreadState.inline.hpp +++ b/src/hotspot/share/gc/g1/g1ParScanThreadState.inline.hpp @@ -151,11 +151,13 @@ template void G1ParScanThreadState::mark_card_if_tracked(G1HeapRegionA #ifdef ASSERT G1HeapRegion* const hr_obj = _g1h->heap_region_containing(o); - assert(region_attr.is_remset_tracked() == hr_obj->rem_set()->is_tracked(), - "State flag indicating remset tracking disagrees (%s) with actual remembered set (%s) for region %u", + assert((region_attr.is_remset_tracked() == hr_obj->rem_set()->is_tracked()) || + (region_attr.is_new_survivor() && region_attr.is_remset_tracked()), + "State flag indicating remset tracking disagrees (%s) with actual remembered set (%s) for region %u (%s)", BOOL_TO_STR(region_attr.is_remset_tracked()), BOOL_TO_STR(hr_obj->rem_set()->is_tracked()), - hr_obj->hrm_index()); + hr_obj->hrm_index(), + hr_obj->get_type_str()); #endif if (!region_attr.is_remset_tracked()) { return;