From d75147e37d1b3ebd9d1513c9510f74ad639a3164 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Tue, 21 Oct 2025 13:41:30 +0200 Subject: [PATCH 1/3] 8370325 Hi all, please review this change to the allocation path to only allow one path doing GCs during that time. Currently, when allocating an object, G1 (or actually any collector) first allocates from TLABs, then allocates from outside TLABs. When G1 can't find space for a TLAB in the current region, it passes on control to `G1CollectedHeap::attempt_allocation[_slow]` that first tries to allocate a new region, or do a GC. Allocation from outside TLABs does the same. G1's behavior to do a GC during failed TLAB allocation is problematic for the `UseGCOverheadLimit` functionality I'm also working on: if the GC overhead limit triggers in a particular operation, returning null for that TLAB allocation. This will will cause another GC right away trying to allocate outside TLAB. Since the garbage collections for TLAB allocations did free some memory (but because of exceeded gc overhead we returned null for the TLAB allocation), that allocation/collection for this outside-tlab allocation will succeed, effectively swallowing the attempt to tell the mutator that GC overhead has been exceeded. Testing: tier1-5, performance neutral after some tests Hth, Thomas --- src/hotspot/share/gc/g1/g1CollectedHeap.cpp | 16 +++++++++------- src/hotspot/share/gc/g1/g1CollectedHeap.hpp | 21 +++++++++------------ 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp index 485caa9f6c0e2..a096c77062213 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp @@ -403,21 +403,20 @@ HeapWord* G1CollectedHeap::allocate_new_tlab(size_t min_size, assert_heap_not_locked_and_not_at_safepoint(); assert(!is_humongous(requested_size), "we do not allow humongous TLABs"); - return attempt_allocation(min_size, requested_size, actual_size); + return attempt_allocation(min_size, requested_size, actual_size, false /* allow_gc */); } -HeapWord* -G1CollectedHeap::mem_allocate(size_t word_size) { +HeapWord* G1CollectedHeap::mem_allocate(size_t word_size) { assert_heap_not_locked_and_not_at_safepoint(); if (is_humongous(word_size)) { return attempt_allocation_humongous(word_size); } size_t dummy = 0; - return attempt_allocation(word_size, word_size, &dummy); + return attempt_allocation(word_size, word_size, &dummy, true /* allow_gc */); } -HeapWord* G1CollectedHeap::attempt_allocation_slow(uint node_index, size_t word_size) { +HeapWord* G1CollectedHeap::attempt_allocation_slow(uint node_index, size_t word_size, bool allow_gc) { ResourceMark rm; // For retrieving the thread names in log messages. // Make sure you read the note in attempt_allocation_humongous(). @@ -444,6 +443,8 @@ HeapWord* G1CollectedHeap::attempt_allocation_slow(uint node_index, size_t word_ result = _allocator->attempt_allocation_locked(node_index, word_size); if (result != nullptr) { return result; + } else if (!allow_gc) { + return nullptr; } // Read the GC count while still holding the Heap_lock. @@ -612,7 +613,8 @@ void G1CollectedHeap::dealloc_archive_regions(MemRegion range) { inline HeapWord* G1CollectedHeap::attempt_allocation(size_t min_word_size, size_t desired_word_size, - size_t* actual_word_size) { + size_t* actual_word_size, + bool is_tlab) { assert_heap_not_locked_and_not_at_safepoint(); assert(!is_humongous(desired_word_size), "attempt_allocation() should not " "be called for humongous allocation requests"); @@ -624,7 +626,7 @@ inline HeapWord* G1CollectedHeap::attempt_allocation(size_t min_word_size, if (result == nullptr) { *actual_word_size = desired_word_size; - result = attempt_allocation_slow(node_index, desired_word_size); + result = attempt_allocation_slow(node_index, desired_word_size, is_tlab); } assert_heap_not_locked(); diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp index 7e3f8a3028568..a797c5db5dfbd 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp @@ -439,18 +439,14 @@ class G1CollectedHeap : public CollectedHeap { // // * If either call cannot satisfy the allocation request using the // current allocating region, they will try to get a new one. If - // this fails, they will attempt to do an evacuation pause and - // retry the allocation. - // - // * If all allocation attempts fail, even after trying to schedule - // an evacuation pause, allocate_new_tlab() will return null, - // whereas mem_allocate() will attempt a heap expansion and/or - // schedule a Full GC. + // this fails, (only) mem_allocate() will attempt to do an evacuation + // pause and retry the allocation. Allocate_new_tlab() will return null, + // deferring to the following mem_allocate(). // // * We do not allow humongous-sized TLABs. So, allocate_new_tlab // should never be called with word_size being humongous. All // humongous allocation requests should go to mem_allocate() which - // will satisfy them with a special path. + // will satisfy them in a special path. HeapWord* allocate_new_tlab(size_t min_size, size_t requested_size, @@ -463,12 +459,13 @@ class G1CollectedHeap : public CollectedHeap { // should only be used for non-humongous allocations. inline HeapWord* attempt_allocation(size_t min_word_size, size_t desired_word_size, - size_t* actual_word_size); - + size_t* actual_word_size, + bool is_tlab); // Second-level mutator allocation attempt: take the Heap_lock and // retry the allocation attempt, potentially scheduling a GC - // pause. This should only be used for non-humongous allocations. - HeapWord* attempt_allocation_slow(uint node_index, size_t word_size); + // pause if allow_gc is set. This should only be used for non-humongous + // allocations. + HeapWord* attempt_allocation_slow(uint node_index, size_t word_size, bool allow_gc); // Takes the Heap_lock and attempts a humongous allocation. It can // potentially schedule a GC pause. From 83de6f2592c16bed5ae102933250ff0401df955f Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Wed, 22 Oct 2025 12:09:05 +0200 Subject: [PATCH 2/3] * fix comments, walulyai review --- src/hotspot/share/gc/g1/g1CollectedHeap.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp index a096c77062213..49b95a0da411b 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp @@ -403,7 +403,7 @@ HeapWord* G1CollectedHeap::allocate_new_tlab(size_t min_size, assert_heap_not_locked_and_not_at_safepoint(); assert(!is_humongous(requested_size), "we do not allow humongous TLABs"); - return attempt_allocation(min_size, requested_size, actual_size, false /* allow_gc */); + return attempt_allocation(min_size, requested_size, actual_size, false /* is_tlab */); } HeapWord* G1CollectedHeap::mem_allocate(size_t word_size) { @@ -413,7 +413,7 @@ HeapWord* G1CollectedHeap::mem_allocate(size_t word_size) { return attempt_allocation_humongous(word_size); } size_t dummy = 0; - return attempt_allocation(word_size, word_size, &dummy, true /* allow_gc */); + return attempt_allocation(word_size, word_size, &dummy, true /* is_tlab */); } HeapWord* G1CollectedHeap::attempt_allocation_slow(uint node_index, size_t word_size, bool allow_gc) { From 23057abe2abcca43d563a7d4c18d2f4fb66bff74 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Wed, 22 Oct 2025 12:36:06 +0200 Subject: [PATCH 3/3] * more parameter name changes, walulyai review --- src/hotspot/share/gc/g1/g1CollectedHeap.cpp | 13 +++++++++---- src/hotspot/share/gc/g1/g1CollectedHeap.hpp | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp index 49b95a0da411b..d3e02df3e09b6 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp @@ -403,7 +403,12 @@ HeapWord* G1CollectedHeap::allocate_new_tlab(size_t min_size, assert_heap_not_locked_and_not_at_safepoint(); assert(!is_humongous(requested_size), "we do not allow humongous TLABs"); - return attempt_allocation(min_size, requested_size, actual_size, false /* is_tlab */); + // Do not allow a GC because we are allocating a new TLAB to avoid an issue + // with UseGCOverheadLimit: although this GC would return null if the overhead + // limit would be exceeded, but it would likely free at least some space. + // So the subsequent outside-TLAB allocation could be successful anyway and + // the indication that the overhead limit had been exceeded swallowed. + return attempt_allocation(min_size, requested_size, actual_size, false /* allow_gc */); } HeapWord* G1CollectedHeap::mem_allocate(size_t word_size) { @@ -413,7 +418,7 @@ HeapWord* G1CollectedHeap::mem_allocate(size_t word_size) { return attempt_allocation_humongous(word_size); } size_t dummy = 0; - return attempt_allocation(word_size, word_size, &dummy, true /* is_tlab */); + return attempt_allocation(word_size, word_size, &dummy, true /* allow_gc */); } HeapWord* G1CollectedHeap::attempt_allocation_slow(uint node_index, size_t word_size, bool allow_gc) { @@ -614,7 +619,7 @@ void G1CollectedHeap::dealloc_archive_regions(MemRegion range) { inline HeapWord* G1CollectedHeap::attempt_allocation(size_t min_word_size, size_t desired_word_size, size_t* actual_word_size, - bool is_tlab) { + bool allow_gc) { assert_heap_not_locked_and_not_at_safepoint(); assert(!is_humongous(desired_word_size), "attempt_allocation() should not " "be called for humongous allocation requests"); @@ -626,7 +631,7 @@ inline HeapWord* G1CollectedHeap::attempt_allocation(size_t min_word_size, if (result == nullptr) { *actual_word_size = desired_word_size; - result = attempt_allocation_slow(node_index, desired_word_size, is_tlab); + result = attempt_allocation_slow(node_index, desired_word_size, allow_gc); } assert_heap_not_locked(); diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp index a797c5db5dfbd..0d354525d89da 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp @@ -460,7 +460,7 @@ class G1CollectedHeap : public CollectedHeap { inline HeapWord* attempt_allocation(size_t min_word_size, size_t desired_word_size, size_t* actual_word_size, - bool is_tlab); + bool allow_gc); // Second-level mutator allocation attempt: take the Heap_lock and // retry the allocation attempt, potentially scheduling a GC // pause if allow_gc is set. This should only be used for non-humongous