From 6545c2d75fc6dbfcdd970e60741f6e060798cd78 Mon Sep 17 00:00:00 2001 From: duke Date: Tue, 9 Jun 2026 17:22:07 +0000 Subject: [PATCH] Backport 5719b671a226ac5cfd116e96d0c9b1c25607b41d --- src/hotspot/share/gc/g1/g1ConcurrentMark.cpp | 15 +- .../share/gc/g1/g1UncommitRegionTask.cpp | 6 +- .../share/gc/g1/g1UncommitRegionTask.hpp | 4 +- src/hotspot/share/gc/g1/g1_globals.hpp | 6 +- ...stUncommitDuringConcurrentBitmapClear.java | 133 ++++++++++++++++++ 5 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 test/hotspot/jtreg/resourcehogs/gc/TestUncommitDuringConcurrentBitmapClear.java diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp index 83dda2a043b..4afc7fa8ff1 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp @@ -714,6 +714,11 @@ private: } HeapWord* region_clear_limit(G1HeapRegion* r) { + // A garbage collection might have made the region unavailable after a yield during + // clearing. Just return bottom as the limit, causing the clearing for this region to end. + if (G1CollectedHeap::heap()->region_at_or_null(r->hrm_index()) == nullptr) { + return r->bottom(); + } // During a Concurrent Undo Mark cycle, the per region top_at_mark_start and // live_words data are current wrt to the _mark_bitmap. We use this information // to only clear ranges of the bitmap that require clearing. @@ -743,7 +748,7 @@ private: } HeapWord* cur = r->bottom(); - HeapWord* const end = region_clear_limit(r); + HeapWord* end = region_clear_limit(r); size_t const chunk_size_in_words = G1ClearBitMapTask::chunk_size() / HeapWordSize; @@ -761,8 +766,12 @@ private: assert(!suspendible() || _cm->is_in_reset_for_next_cycle(), "invariant"); // Abort iteration if necessary. - if (has_aborted()) { - return true; + if (suspendible() && _cm->do_yield_check()) { + if (_cm->has_aborted()) { + return true; + } + // Re-read end. The region might have been uncommitted. + end = region_clear_limit(r); } } assert(cur >= end, "Must have completed iteration over the bitmap for region %u.", r->hrm_index()); diff --git a/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp b/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp index e1203229557..f88736c5642 100644 --- a/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp +++ b/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2026, 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 @@ -58,9 +58,9 @@ void G1UncommitRegionTask::enqueue() { G1UncommitRegionTask* uncommit_task = instance(); if (!uncommit_task->is_active()) { - // Change state to active and schedule using UncommitInitialDelayMs. + // Change state to active and schedule. uncommit_task->set_active(true); - G1CollectedHeap::heap()->service_thread()->schedule_task(uncommit_task, UncommitInitialDelayMs); + G1CollectedHeap::heap()->service_thread()->schedule_task(uncommit_task, G1UncommitInitialDelay); } } diff --git a/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp b/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp index 7c9a25f6857..835217d2a59 100644 --- a/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp +++ b/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2026, 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 @@ -34,8 +34,6 @@ class G1UncommitRegionTask : public G1ServiceTask { // This limit is small enough to ensure that the duration of each invocation // is short, while still making reasonable progress. static const uint UncommitSizeLimit = 128 * M; - // Initial delay in milliseconds after GC before the regions are uncommitted. - static const uint UncommitInitialDelayMs = 100; // The delay between two uncommit task executions. static const uint UncommitTaskDelayMs = 10; diff --git a/src/hotspot/share/gc/g1/g1_globals.hpp b/src/hotspot/share/gc/g1/g1_globals.hpp index 14daac4800b..8a346c5c78f 100644 --- a/src/hotspot/share/gc/g1/g1_globals.hpp +++ b/src/hotspot/share/gc/g1/g1_globals.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 2026, 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 @@ -184,6 +184,10 @@ "shrink attempt.") \ range(0, 100) \ \ + develop(uint, G1UncommitInitialDelay, 100, \ + "Delay in milliseconds until regions just made eligible for " \ + "uncommit are actually uncommitted.") \ + \ product(uint, G1CPUUsageDeviationPercent, 25, DIAGNOSTIC, \ "The acceptable deviation (in percent) from the target GC CPU " \ "usage (based on GCTimeRatio). Creates a tolerance range " \ diff --git a/test/hotspot/jtreg/resourcehogs/gc/TestUncommitDuringConcurrentBitmapClear.java b/test/hotspot/jtreg/resourcehogs/gc/TestUncommitDuringConcurrentBitmapClear.java new file mode 100644 index 00000000000..f79f4992817 --- /dev/null +++ b/test/hotspot/jtreg/resourcehogs/gc/TestUncommitDuringConcurrentBitmapClear.java @@ -0,0 +1,133 @@ +/* + * Copyright (c) 2026, 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. + */ + +package gc.g1; + +/* + * @test TestUncommitDuringConcurrentBitmapClear + * @bug 8385369 + * @requires vm.gc.G1 + * @requires vm.debug + * @requires vm.flagless + * @requires vm.bits == 64 + * @requires os.maxMemory > 8g + * @summary Verify that G1 does not crash while uncommitting a region whose + * bitmap is currently being cleared. + * Options are geared towards uncommitting aggressively. Also use a large + * region size so that corresponding bitmaps get uncommitted always too. + * @library /test/lib + * @build jdk.test.whitebox.WhiteBox + * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox + * @run main/othervm + * -Xbootclasspath/a:. + * -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI + * -Xmx8g + * -Xms32m + * -XX:G1HeapRegionSize=16m + * -XX:+UseG1GC + * -XX:ConcGCThreads=1 + * -XX:GCTimeRatio=1 + * -XX:G1CPUUsageShrinkThreshold=1 + * -XX:G1ShrinkByPercentOfAvailable=100 + * -XX:G1UncommitInitialDelay=0 + * -Xlog:gc+marking,gc,gc+ergo+heap=debug + * gc.g1.TestUncommitDuringConcurrentBitmapClear + */ + +import java.util.concurrent.FutureTask; +import java.util.concurrent.TimeUnit; + +import jdk.test.whitebox.WhiteBox; + +/* + * Repeatedly make the concurrent cycle stop after the cleanup pause, issuing + * young GCs during the Concurrent Cleanup for Next Mark phase. Humongous + * regions allocated and dropped before that should get eager-reclaimed and + * their memory uncommitted while bitmap clearing runs. + */ +public class TestUncommitDuringConcurrentBitmapClear { + + private static final WhiteBox WB = WhiteBox.getWhiteBox(); + + private static final int NumObjs = 400; // Number of humongous objects to allocate + // per attempt. Sized to fill a fair amount of + // the available memory. + private static final int LargeObjSize = 9 * 1024 * 1024; // Large enough to be a humongous object. + + private static Object[] objects; + + private static void test() throws Exception { + + // This task drops the humongous objects, making them eligible for + // uncommit, and starts the concurrent bitmap clearing. While it is + // running, the caller triggers GCs that may or may not trigger the issue. + FutureTask concurrentClearTask = new FutureTask<>(() -> { + objects = null; + WB.concurrentGCRunTo(WB.G1_BEFORE_CLEANUP_COMPLETED); + return null; + }); + + try { + System.out.println("taking control"); + WB.concurrentGCAcquireControl(); + + // Allocate a new set of humongous objects. Acquire control first to avoid + // unnecessary concurrent cycles due to that allocation. We do not need them. + objects = new Object[NumObjs]; + for (int i = 0; i < objects.length; i++) { + objects[i] = new byte[LargeObjSize]; + } + + WB.concurrentGCRunTo(WB.G1_AFTER_CLEANUP_STARTED); + + new Thread(concurrentClearTask).start(); + + int numYoungGCs = 0; + // Execute at least one young GC, even if the concurrent + // clear bitmap finishes very quickly. + do { + WB.youngGC(); + numYoungGCs++; + // Wait a bit. This should give the concurrent clear task a chance + // to finish execution. + Thread.sleep(1); + } while (!concurrentClearTask.isDone() && numYoungGCs < 200); + + concurrentClearTask.get(30, TimeUnit.SECONDS); // Propagates exceptions, if any. + } finally { + WB.concurrentGCRunToIdle(); + + System.out.println("Releasing control"); + WB.concurrentGCReleaseControl(); + } + } + + public static void main(String[] args) throws Exception { + if (!WB.supportsConcurrentGCBreakpoints()) { + throw new RuntimeException("G1 should support GC breakpoints"); + } + for (int i = 0; i < 20; i++) { + test(); + } + } +}