diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp index f9ebc74805b..4b20bb43317 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp @@ -869,7 +869,7 @@ public: NoteStartOfMarkHRClosure() : HeapRegionClosure(), _cm(G1CollectedHeap::heap()->concurrent_mark()) { } bool do_heap_region(HeapRegion* r) override { - if (r->is_old_or_humongous() && !r->is_collection_set_candidate()) { + if (r->is_old_or_humongous() && !r->is_collection_set_candidate() && !r->in_collection_set()) { _cm->update_top_at_mark_start(r); } return false; diff --git a/src/hotspot/share/gc/g1/g1YoungCollector.cpp b/src/hotspot/share/gc/g1/g1YoungCollector.cpp index 4fa3b928763..906d9854d8d 100644 --- a/src/hotspot/share/gc/g1/g1YoungCollector.cpp +++ b/src/hotspot/share/gc/g1/g1YoungCollector.cpp @@ -484,13 +484,8 @@ void G1YoungCollector::set_young_collection_default_active_worker_threads(){ } void G1YoungCollector::pre_evacuate_collection_set(G1EvacInfo* evacuation_info) { - - // Must be before collection set calculation, requires collection set to not - // be calculated yet. - if (collector_state()->in_concurrent_start_gc()) { - concurrent_mark()->pre_concurrent_start(_gc_cause); - } - + // Flush various data in thread-local buffers to be able to determine the collection + // set { Ticks start = Ticks::now(); G1PreEvacuateCollectionSetBatchTask cl; @@ -501,6 +496,10 @@ void G1YoungCollector::pre_evacuate_collection_set(G1EvacInfo* evacuation_info) // Needs log buffers flushed. calculate_collection_set(evacuation_info, policy()->max_pause_time_ms()); + if (collector_state()->in_concurrent_start_gc()) { + concurrent_mark()->pre_concurrent_start(_gc_cause); + } + // Please see comment in g1CollectedHeap.hpp and // G1CollectedHeap::ref_processing_init() to see how // reference processing currently works in G1. diff --git a/src/hotspot/share/gc/g1/g1YoungGCPreEvacuateTasks.hpp b/src/hotspot/share/gc/g1/g1YoungGCPreEvacuateTasks.hpp index effefa7a1eb..912941fa2a2 100644 --- a/src/hotspot/share/gc/g1/g1YoungGCPreEvacuateTasks.hpp +++ b/src/hotspot/share/gc/g1/g1YoungGCPreEvacuateTasks.hpp @@ -29,6 +29,7 @@ // Set of pre evacuate collection set tasks containing ("s" means serial): // - Retire TLAB and Flush Logs (Java threads) +// - Flush pin count cache (Java threads) // - Flush Logs (s) (Non-Java threads) class G1PreEvacuateCollectionSetBatchTask : public G1BatchedTask { class JavaThreadRetireTLABAndFlushLogs; diff --git a/test/hotspot/jtreg/gc/g1/pinnedobjs/TestDroppedRetainedTAMS.java b/test/hotspot/jtreg/gc/g1/pinnedobjs/TestDroppedRetainedTAMS.java new file mode 100644 index 00000000000..f650e53a25f --- /dev/null +++ b/test/hotspot/jtreg/gc/g1/pinnedobjs/TestDroppedRetainedTAMS.java @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* @test + * @summary Check that TAMSes are correctly updated for regions dropped from + * the retained collection set candidates during a Concurrent Start pause. + * @requires vm.gc.G1 + * @requires vm.flagless + * @library /test/lib + * @build jdk.test.whitebox.WhiteBox + * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox + * @run main/othervm -XX:+UseG1GC -XX:+UnlockDiagnosticVMOptions -XX:+UnlockExperimentalVMOptions + -XX:+WhiteBoxAPI -Xbootclasspath/a:. -Xmx32m -XX:G1NumCollectionsKeepPinned=1 + -XX:+VerifyBeforeGC -XX:+VerifyAfterGC -XX:G1MixedGCLiveThresholdPercent=100 + -XX:G1HeapWastePercent=0 -Xlog:gc,gc+ergo+cset=trace gc.g1.pinnedobjs.TestDroppedRetainedTAMS + */ + +package gc.g1.pinnedobjs; + +import jdk.test.whitebox.WhiteBox; + +public class TestDroppedRetainedTAMS { + + private static final WhiteBox wb = WhiteBox.getWhiteBox(); + + private static final char[] dummy = new char[100]; + + public static void main(String[] args) { + wb.fullGC(); // Move the target dummy object to old gen. + + wb.pinObject(dummy); + + // After this concurrent cycle the pinned region will be in the the (marking) + // collection set candidates. + wb.g1RunConcurrentGC(); + + // Pass the Prepare mixed gc which will not do anything about the marking + // candidates. + wb.youngGC(); + // Mixed GC. Will complete. That pinned region is now retained. The mixed gcs + // will end here. + wb.youngGC(); + + // The pinned region will be dropped from the retained candidates during the + // Concurrent Start GC, leaving that region's TAMS broken. + wb.g1RunConcurrentGC(); + + // Verification will find a lot of broken objects. + wb.youngGC(); + System.out.println(dummy); + } +}