diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp index 28ae5eb5833..01caa934eaf 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp @@ -420,8 +420,7 @@ template ShenandoahHeapRegion* ShenandoahFreeSet::find_region_for_alloc(size_t, bool&); ShenandoahHeapRegion* ShenandoahFreeSet::steal_from_mutator(ShenandoahFreeSetPartitionId target_partition, - ShenandoahAllocRequest& req, - bool& in_new_region) { + ShenandoahAllocRequest& req) { shenandoah_assert_heaplocked(); assert(target_partition != ShenandoahFreeSetPartitionId::Mutator, "Cannot steal from self"); @@ -442,14 +441,13 @@ ShenandoahHeapRegion* ShenandoahFreeSet::steal_from_mutator(ShenandoahFreeSetPar } log_debug(gc, free)("Flipped region %zu to gc for request: " PTR_FORMAT, idx, p2i(&req)); - in_new_region = r->is_empty(); - if (in_new_region) { - ShenandoahAffiliation aff = (target_partition == ShenandoahFreeSetPartitionId::OldCollector) - ? OLD_GENERATION : YOUNG_GENERATION; - r->set_affiliation(aff); - if (r->is_old()) { - r->end_preemptible_coalesce_and_fill(); - } + r->try_recycle_under_lock(); + assert(r->is_empty(), "Must be empty"); + ShenandoahAffiliation aff = (target_partition == ShenandoahFreeSetPartitionId::OldCollector) + ? OLD_GENERATION : YOUNG_GENERATION; + r->set_affiliation(aff); + if (r->is_old()) { + r->end_preemptible_coalesce_and_fill(); } return r; } diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp index 1dda8e276a2..5311fe84dd7 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp @@ -669,11 +669,10 @@ public: // Steal an empty region from the Mutator partition for the given collector partition. // Flips the region, sets up affiliation, and returns it ready for allocation. - // Returns nullptr if no region can be stolen. Sets in_new_region to true on success. - // Caller must hold the heap lock. + // The returned region is always empty (newly available for allocation). + // Returns nullptr if no region can be stolen. Caller must hold the heap lock. ShenandoahHeapRegion* steal_from_mutator(ShenandoahFreeSetPartitionId target_partition, - ShenandoahAllocRequest& req, - bool& in_new_region); + ShenandoahAllocRequest& req); // Allocate contiguous regions for humongous objects. Caller must hold heap lock. HeapWord* allocate_contiguous(ShenandoahAllocRequest& req, bool is_humongous); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahPartitionAllocator.cpp b/src/hotspot/share/gc/shenandoah/shenandoahPartitionAllocator.cpp index 16eabac6136..0674fb2cfb3 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahPartitionAllocator.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahPartitionAllocator.cpp @@ -49,44 +49,66 @@ HeapWord* ShenandoahPartitionAllocator::allocate(ShenandoahAllocReque } } + bool boundary_changed = false; + size_t min_req_words = req.is_lab_alloc() ? req.min_size() : req.size(); // Fast path: try the retained region first. if (_retained_region != nullptr) { - size_t min_size = req.is_lab_alloc() ? req.min_size() : req.size(); - if (_free_set->alloc_capacity(_retained_region) >= min_size * HeapWordSize) { - HeapWord* result = try_allocate_in(_retained_region, req, in_new_region); - if (result != nullptr) { - return result; - } + HeapWord* result = nullptr; + size_t ac_words = _retained_region->free() >> LogHeapWordSize; + if (ac_words >= min_req_words) { + result = allocate_in(_retained_region, req, boundary_changed); + } else if (ac_words < PLAB::min_size()) { + // Retained region is too small for any future PLAB; retire it. + _free_set->retire_region(PARTITION, _retained_region->index(), _retained_region->used()); + boundary_changed = true; + _retained_region = nullptr; + } + // else: retained region can't satisfy this request but still has usable capacity for + // a smaller future LAB. Keep it retained. + if (result != nullptr) { + in_new_region = false; + _free_set->notify_allocation(PARTITION, false, boundary_changed); + return result; } - _retained_region = nullptr; } // Ask FreeSet to find a suitable region. - size_t min_size_words = req.is_lab_alloc() ? req.min_size() : req.size(); - ShenandoahHeapRegion* r = _free_set->find_region_for_alloc(min_size_words, in_new_region); - + ShenandoahHeapRegion* r = _free_set->find_region_for_alloc(min_req_words, in_new_region); if (r != nullptr) { - HeapWord* result = try_allocate_in(r, req, in_new_region); - if (result != nullptr) { - return result; + HeapWord* result = allocate_in(r, req, boundary_changed); + if (in_new_region) { + _free_set->mark_region_used(PARTITION); + boundary_changed = true; } + _free_set->notify_allocation(PARTITION, in_new_region, boundary_changed); + return result; } // Collector partitions can overflow into Mutator partition. if constexpr (PARTITION != ShenandoahFreeSetPartitionId::Mutator) { if (ShenandoahEvacReserveOverflow) { - ShenandoahHeapRegion* stolen = _free_set->steal_from_mutator(PARTITION, req, in_new_region); + ShenandoahHeapRegion* stolen = _free_set->steal_from_mutator(PARTITION, req); if (stolen != nullptr) { - return try_allocate_in(stolen, req, in_new_region); + assert(stolen->is_empty(), "Stolen region must be empty"); + HeapWord* result = allocate_in(stolen, req, boundary_changed); + _free_set->mark_region_used(PARTITION); + // Stealing always produces a new region, which implies a boundary change. + _free_set->notify_allocation(PARTITION, /* in_new_region */ true, /* boundary_changed */ true); + return result; } } } + // No allocation happened, but the retained region may have been retired above. + // Flush the resulting boundary change so partition bookkeeping stays consistent. + if (boundary_changed) { + _free_set->notify_allocation(PARTITION, /* in_new_region */ false, /* boundary_changed */ true); + } return nullptr; } template -HeapWord* ShenandoahPartitionAllocator::try_allocate_in(ShenandoahHeapRegion* r, ShenandoahAllocRequest& req, bool& in_new_region) { +HeapWord* ShenandoahPartitionAllocator::allocate_in(ShenandoahHeapRegion* r, ShenandoahAllocRequest& req, bool& boundary_changed) { assert(_free_set->alloc_capacity(r) > 0, "Performance: should avoid full regions on this path: %zu", r->index()); HeapWord* result = nullptr; @@ -98,43 +120,30 @@ HeapWord* ShenandoahPartitionAllocator::try_allocate_in(ShenandoahHea if (adjusted_size > free) { adjusted_size = free; } - if (adjusted_size >= req.min_size()) { - result = r->allocate(adjusted_size, req); - assert(result != nullptr, "Allocation must succeed: free %zu, actual %zu", free, adjusted_size); - req.set_actual_size(adjusted_size); - } else { - log_trace(gc, free)("Failed to shrink LAB request (%zu) in region %zu to %zu" - " because min_size() is %zu", req.size(), r->index(), adjusted_size, req.min_size()); - } + assert(adjusted_size >= req.min_size(), "Must be"); + result = r->allocate(adjusted_size, req); + req.set_actual_size(adjusted_size); } else { size_t size = req.size(); result = r->allocate(size, req); - if (result != nullptr) { - req.set_actual_size(size); - } + req.set_actual_size(size); } + assert(result != nullptr, "Allocation must succeed, region free: %zu, request minimal size: %zu", + r->free(), req.is_lab_alloc() ? req.min_size() : req.size()); - // Update partition used bytes on success. - if (result != nullptr) { - if constexpr (PARTITION == ShenandoahFreeSetPartitionId::Mutator) { - assert(req.is_young(), "Mutator allocations always come from young generation."); - _free_set->increase_partition_used(PARTITION, req.actual_size() * HeapWordSize); - } else { - assert(req.is_gc_alloc(), "Should be gc_alloc since req wasn't mutator alloc"); - // GC allocations set update_watermark so relocated objects aren't re-updated during update-refs. - r->set_update_watermark(r->top()); - _free_set->increase_partition_used(PARTITION, (req.actual_size() + req.waste()) * HeapWordSize); - } - } - - bool boundary_changed = false; - if ((result != nullptr) && in_new_region) { - _free_set->mark_region_used(PARTITION); - boundary_changed = true; + // Update partition used bytes after allocation + if constexpr (PARTITION == ShenandoahFreeSetPartitionId::Mutator) { + assert(req.is_young(), "Mutator allocations always come from young generation."); + _free_set->increase_partition_used(PARTITION, req.actual_size() * HeapWordSize); + } else { + assert(req.is_gc_alloc(), "Should be gc_alloc since req wasn't mutator alloc"); + // GC allocations set update_watermark so relocated objects aren't re-updated during update-refs. + r->set_update_watermark(r->top()); + _free_set->increase_partition_used(PARTITION, (req.actual_size() + req.waste()) * HeapWordSize); } // Retire the region if remaining capacity is too small for any future PLAB. - if (_free_set->alloc_capacity(r) < PLAB::min_size() * HeapWordSize) { + if ((r->free() >> LogHeapWordSize) < PLAB::min_size()) { size_t idx = r->index(); size_t waste_bytes = _free_set->retire_region(PARTITION, idx, r->used()); boundary_changed = true; @@ -146,13 +155,11 @@ HeapWord* ShenandoahPartitionAllocator::try_allocate_in(ShenandoahHea if (_retained_region == r) { _retained_region = nullptr; } - } else if (result != nullptr) { + } else if (_retained_region == nullptr) { // Region still has usable capacity — retain for next allocation. _retained_region = r; } - // Recompute generation used/affiliated totals and validate bounds if changed. - _free_set->notify_allocation(PARTITION, in_new_region, boundary_changed); return result; } diff --git a/src/hotspot/share/gc/shenandoah/shenandoahPartitionAllocator.hpp b/src/hotspot/share/gc/shenandoah/shenandoahPartitionAllocator.hpp index 8c455a031ff..4439017e3e0 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahPartitionAllocator.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahPartitionAllocator.hpp @@ -45,9 +45,12 @@ private: // to avoid asking FreeSet to scan for a region. Cleared on free-set rebuild. ShenandoahHeapRegion* _retained_region; - // Attempt allocation within a single region. Handles LAB sizing, updates partition - // accounting via ShenandoahFreeSet, and retires the region if capacity drops below PLAB::min_size(). - HeapWord* try_allocate_in(ShenandoahHeapRegion* r, ShenandoahAllocRequest& req, bool& in_new_region); + // Allocate within a single region; the caller must guarantee the region has enough free + // capacity for the request. Handles LAB sizing, updates partition accounting via + // ShenandoahFreeSet, and retires the region if remaining capacity drops below PLAB::min_size(). + // boundary_changed is set to true if the region is retired or otherwise mutates the partition + // boundary; it is never reset to false. + HeapWord* allocate_in(ShenandoahHeapRegion* r, ShenandoahAllocRequest& req, bool& boundary_changed); public: ShenandoahPartitionAllocator(ShenandoahFreeSet* free_set);