From 936d117b7c1397e4fb2d587e762ff726eea3808b Mon Sep 17 00:00:00 2001 From: William Kemper Date: Thu, 23 Jan 2025 13:30:29 -0800 Subject: [PATCH 1/3] Recycle young trash region before flipping to GC --- .../share/gc/shenandoah/shenandoahFreeSet.cpp | 9 +++++++- .../gc/shenandoah/shenandoahGeneration.cpp | 22 +++++++++++-------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp index 032e478d4c247..6e87d7f6f3751 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp @@ -125,7 +125,7 @@ void ShenandoahRegionPartitions::dump_bitmap() const { _rightmosts[int(ShenandoahFreeSetPartitionId::OldCollector)]); log_debug(gc)("Empty Mutator range [%zd, %zd" "], Empty Collector range [%zd, %zd" - "], Empty Old Collecto range [%zd, %zd]", + "], Empty Old Collector range [%zd, %zd]", _leftmosts_empty[int(ShenandoahFreeSetPartitionId::Mutator)], _rightmosts_empty[int(ShenandoahFreeSetPartitionId::Mutator)], _leftmosts_empty[int(ShenandoahFreeSetPartitionId::Collector)], @@ -1297,6 +1297,13 @@ void ShenandoahFreeSet::flip_to_old_gc(ShenandoahHeapRegion* r) { bool transferred = gen_heap->generation_sizer()->transfer_to_old(1); if (!transferred) { log_warning(gc, free)("Forcing transfer of %zu to old reserve.", idx); + if (r->is_trash() && r->is_young()) { + // If this is a trash region and is still affiliated with the young generation + // we need to recycle it before decreasing the capacity of the young generation. + // Otherwise, we may violate the constraint that the size of regions affiliated + // with the young generation is always less than or equal to its maximum capacity + r->try_recycle_under_lock(); + } gen_heap->generation_sizer()->force_transfer_to_old(1); } // We do not ensure that the region is no longer trash, relying on try_allocate_in(), which always comes next, diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp index eb8027a0f7c84..8a92a24abe420 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp @@ -1025,15 +1025,19 @@ size_t ShenandoahGeneration::decrease_capacity(size_t decrement) { _max_capacity -= decrement; - // This detects arithmetic wraparound on _used - assert(ShenandoahHeap::heap()->is_full_gc_in_progress() || - (used_regions_size() >= used()), - "Affiliated regions must hold more than what is currently used"); - assert(ShenandoahHeap::heap()->is_full_gc_in_progress() || - (_used <= _max_capacity), "Cannot use more than capacity"); - assert(ShenandoahHeap::heap()->is_full_gc_in_progress() || - (used_regions_size() <= _max_capacity), - "Cannot use more than capacity"); +#ifdef ASSERT + if (!ShenandoahHeap::heap()->is_full_gc_in_progress()) { + assert(used_regions_size() >= used(), + "Affiliated regions (" PROPERFMT ") must at least hold what is currently used (" PROPERFMT ")", + PROPERFMTARGS(used_regions_size()), PROPERFMTARGS(used())); + assert(_used <= _max_capacity, + "Cannot use (" PROPERFMT ") more than capacity (" PROPERFMT ")", + PROPERFMTARGS(_used), PROPERFMTARGS(_max_capacity)); + assert(used_regions_size() <= _max_capacity, + "Affiliated regions (" PROPERFMT ") cannot use more than capacity (" PROPERFMT ")", + PROPERFMTARGS(used_regions_size()),PROPERFMTARGS(_max_capacity)); + } +#endif return _max_capacity; } From fa0e1527dac19801d3ad59b5bf7122518933be29 Mon Sep 17 00:00:00 2001 From: William Kemper Date: Fri, 24 Jan 2025 16:32:46 -0800 Subject: [PATCH 2/3] Fix instrumentation --- .../share/gc/shenandoah/shenandoahGeneration.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp index 8a92a24abe420..591dc8c4c88a9 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp @@ -1028,14 +1028,14 @@ size_t ShenandoahGeneration::decrease_capacity(size_t decrement) { #ifdef ASSERT if (!ShenandoahHeap::heap()->is_full_gc_in_progress()) { assert(used_regions_size() >= used(), - "Affiliated regions (" PROPERFMT ") must at least hold what is currently used (" PROPERFMT ")", - PROPERFMTARGS(used_regions_size()), PROPERFMTARGS(used())); + "Affiliated regions (" EXACTFMT ") must at least hold what is currently used (" EXACTFMT ")", + EXACTFMTARGS(used_regions_size()), EXACTFMTARGS(used())); assert(_used <= _max_capacity, - "Cannot use (" PROPERFMT ") more than capacity (" PROPERFMT ")", - PROPERFMTARGS(_used), PROPERFMTARGS(_max_capacity)); + "Cannot use (" EXACTFMT ") more than capacity (" EXACTFMT ")", + EXACTFMTARGS(_used), EXACTFMTARGS(_max_capacity)); assert(used_regions_size() <= _max_capacity, - "Affiliated regions (" PROPERFMT ") cannot use more than capacity (" PROPERFMT ")", - PROPERFMTARGS(used_regions_size()),PROPERFMTARGS(_max_capacity)); + "Affiliated regions (" EXACTFMT ") cannot use more than capacity (" EXACTFMT ")", + EXACTFMTARGS(used_regions_size()),EXACTFMTARGS(_max_capacity)); } #endif return _max_capacity; From 87d9f4e248fcec3cb506e996c3302a3280d9a691 Mon Sep 17 00:00:00 2001 From: William Kemper Date: Mon, 3 Feb 2025 14:13:26 -0800 Subject: [PATCH 3/3] Only transfer affiliated region when adjusting reserves --- .../share/gc/shenandoah/shenandoahFreeSet.cpp | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp index ff056856ffcae..045be61d802d0 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp @@ -1293,20 +1293,22 @@ void ShenandoahFreeSet::flip_to_old_gc(ShenandoahHeapRegion* r) { ShenandoahFreeSetPartitionId::OldCollector, region_capacity); _partitions.assert_bounds(); _heap->old_generation()->augment_evacuation_reserve(region_capacity); - bool transferred = gen_heap->generation_sizer()->transfer_to_old(1); - if (!transferred) { - log_warning(gc, free)("Forcing transfer of %zu to old reserve.", idx); - if (r->is_trash() && r->is_young()) { - // If this is a trash region and is still affiliated with the young generation - // we need to recycle it before decreasing the capacity of the young generation. - // Otherwise, we may violate the constraint that the size of regions affiliated - // with the young generation is always less than or equal to its maximum capacity + + if (r->is_young()) { + // If this is a trash region and is still affiliated with the young generation + // we need to recycle it before decreasing the capacity of the young generation. + // Otherwise, we may violate the constraint that the size of regions affiliated + // with the young generation is always less than or equal to its maximum capacity + if (r->is_trash()) { r->try_recycle_under_lock(); } - gen_heap->generation_sizer()->force_transfer_to_old(1); + + const bool transferred = gen_heap->generation_sizer()->transfer_to_old(1); + if (!transferred) { + log_warning(gc, free)("Forcing transfer of %zu to old reserve.", idx); + gen_heap->generation_sizer()->force_transfer_to_old(1); + } } - // We do not ensure that the region is no longer trash, relying on try_allocate_in(), which always comes next, - // to recycle trash before attempting to allocate anything in the region. } void ShenandoahFreeSet::flip_to_gc(ShenandoahHeapRegion* r) {