From 501fa2041a77139a9ac42fef69f28b1fd50fee65 Mon Sep 17 00:00:00 2001 From: William Kemper Date: Thu, 9 Oct 2025 18:25:47 +0000 Subject: [PATCH] 8368501: Shenandoah: GC progress evaluation does not use generation Reviewed-by: ysr --- .../gc/shenandoah/shenandoahDegeneratedGC.cpp | 7 +- .../share/gc/shenandoah/shenandoahFullGC.cpp | 11 +-- .../share/gc/shenandoah/shenandoahMetrics.cpp | 79 +++++++------------ .../share/gc/shenandoah/shenandoahMetrics.hpp | 18 ++--- 4 files changed, 42 insertions(+), 73 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp index bbecd12f095..aad5c1384be 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp @@ -115,8 +115,7 @@ void ShenandoahDegenGC::op_degenerated() { } #endif - ShenandoahMetricsSnapshot metrics; - metrics.snap_before(); + ShenandoahMetricsSnapshot metrics(heap->free_set()); switch (_degen_point) { // The cases below form the Duff's-like device: it describes the actual GC cycle, @@ -308,10 +307,8 @@ void ShenandoahDegenGC::op_degenerated() { Universe::verify(); } - metrics.snap_after(); - // Decide if this cycle made good progress, and, if not, should it upgrade to a full GC. - const bool progress = metrics.is_good_progress(_generation); + const bool progress = metrics.is_good_progress(); ShenandoahCollectorPolicy* policy = heap->shenandoah_policy(); policy->record_degenerated(_generation->is_young(), _abbreviated, progress); if (progress) { diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp index 27ff45e67de..557a3847bab 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp @@ -104,21 +104,18 @@ void ShenandoahFullGC::entry_full(GCCause::Cause cause) { } void ShenandoahFullGC::op_full(GCCause::Cause cause) { - ShenandoahMetricsSnapshot metrics; - metrics.snap_before(); + ShenandoahHeap* const heap = ShenandoahHeap::heap(); + + ShenandoahMetricsSnapshot metrics(heap->free_set()); // Perform full GC do_it(cause); - ShenandoahHeap* const heap = ShenandoahHeap::heap(); - if (heap->mode()->is_generational()) { ShenandoahGenerationalFullGC::handle_completion(heap); } - metrics.snap_after(); - - if (metrics.is_good_progress(heap->global_generation())) { + if (metrics.is_good_progress()) { heap->notify_gc_progress(); } else { // Nothing to do. Tell the allocation path that we have failed to make diff --git a/src/hotspot/share/gc/shenandoah/shenandoahMetrics.cpp b/src/hotspot/share/gc/shenandoah/shenandoahMetrics.cpp index dc666a34c59..d774a8dba42 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahMetrics.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahMetrics.cpp @@ -28,69 +28,45 @@ #include "gc/shenandoah/shenandoahHeapRegion.hpp" #include "gc/shenandoah/shenandoahMetrics.hpp" -ShenandoahMetricsSnapshot::ShenandoahMetricsSnapshot() { - _heap = ShenandoahHeap::heap(); +ShenandoahMetricsSnapshot::ShenandoahMetricsSnapshot(ShenandoahFreeSet* free_set) + : _free_set(free_set) + , _used_before(free_set->used()) + , _if_before(free_set->internal_fragmentation()) + , _ef_before(free_set->external_fragmentation()) { } -void ShenandoahMetricsSnapshot::snap_before() { - _used_before = _heap->used(); - _if_before = _heap->free_set()->internal_fragmentation(); - _ef_before = _heap->free_set()->external_fragmentation(); -} -void ShenandoahMetricsSnapshot::snap_after() { - _used_after = _heap->used(); - _if_after = _heap->free_set()->internal_fragmentation(); - _ef_after = _heap->free_set()->external_fragmentation(); -} - -// For degenerated GC, generation is Young in generational mode, Global in non-generational mode. -// For full GC, generation is always Global. -// -// Note that the size of the chosen collection set is proportional to the relevant generation's collection set. -// Note also that the generation size may change following selection of the collection set, as a side effect -// of evacuation. Evacuation may promote objects, causing old to grow and young to shrink. Or this may be a -// mixed evacuation. When old regions are evacuated, this typically allows young to expand. In all of these -// various scenarios, the purpose of asking is_good_progress() is to determine if there is enough memory available -// within young generation to justify making an attempt to perform a concurrent collection. For this reason, we'll -// use the current size of the generation (which may not be different than when the collection set was chosen) to -// assess how much free memory we require in order to consider the most recent GC to have had good progress. - -bool ShenandoahMetricsSnapshot::is_good_progress(ShenandoahGeneration* generation) { +bool ShenandoahMetricsSnapshot::is_good_progress() const { // Under the critical threshold? - ShenandoahFreeSet* free_set = _heap->free_set(); - size_t free_actual = free_set->available(); + const size_t free_actual = _free_set->available(); assert(free_actual != ShenandoahFreeSet::FreeSetUnderConstruction, "Avoid this race"); - // ShenandoahCriticalFreeThreshold is expressed as a percentage. We multiple this percentage by 1/100th - // of the generation capacity to determine whether the available memory within the generation exceeds the - // critical threshold. - size_t free_expected = (ShenandoahHeap::heap()->soft_max_capacity() / 100) * ShenandoahCriticalFreeThreshold; - - bool prog_free = free_actual >= free_expected; - log_info(gc, ergo)("%s progress for free space: %zu%s, need %zu%s", - prog_free ? "Good" : "Bad", - byte_size_in_proper_unit(free_actual), proper_unit_for_byte_size(free_actual), - byte_size_in_proper_unit(free_expected), proper_unit_for_byte_size(free_expected)); + // ShenandoahCriticalFreeThreshold is expressed as a percentage. We multiply this percentage by 1/100th + // of the soft max capacity to determine whether the available memory within the mutator partition of the + // freeset exceeds the critical threshold. + const size_t free_expected = (ShenandoahHeap::heap()->soft_max_capacity() / 100) * ShenandoahCriticalFreeThreshold; + const bool prog_free = free_actual >= free_expected; + log_info(gc, ergo)("%s progress for free space: " PROPERFMT ", need " PROPERFMT, + prog_free ? "Good" : "Bad", PROPERFMTARGS(free_actual), PROPERFMTARGS(free_expected)); if (!prog_free) { return false; } // Freed up enough? - size_t progress_actual = (_used_before > _used_after) ? _used_before - _used_after : 0; - size_t progress_expected = ShenandoahHeapRegion::region_size_bytes(); - bool prog_used = progress_actual >= progress_expected; - log_info(gc, ergo)("%s progress for used space: %zu%s, need %zu%s", - prog_used ? "Good" : "Bad", - byte_size_in_proper_unit(progress_actual), proper_unit_for_byte_size(progress_actual), - byte_size_in_proper_unit(progress_expected), proper_unit_for_byte_size(progress_expected)); + const size_t used_after = _free_set->used(); + const size_t progress_actual = (_used_before > used_after) ? _used_before - used_after : 0; + const size_t progress_expected = ShenandoahHeapRegion::region_size_bytes(); + const bool prog_used = progress_actual >= progress_expected; + log_info(gc, ergo)("%s progress for used space: " PROPERFMT ", need " PROPERFMT, + prog_used ? "Good" : "Bad", PROPERFMTARGS(progress_actual), PROPERFMTARGS(progress_expected)); if (prog_used) { return true; } // Internal fragmentation is down? - double if_actual = _if_before - _if_after; - double if_expected = 0.01; // 1% should be enough - bool prog_if = if_actual >= if_expected; + const double if_after = _free_set->internal_fragmentation(); + const double if_actual = _if_before - if_after; + const double if_expected = 0.01; // 1% should be enough + const bool prog_if = if_actual >= if_expected; log_info(gc, ergo)("%s progress for internal fragmentation: %.1f%%, need %.1f%%", prog_if ? "Good" : "Bad", if_actual * 100, if_expected * 100); @@ -99,9 +75,10 @@ bool ShenandoahMetricsSnapshot::is_good_progress(ShenandoahGeneration* generatio } // External fragmentation is down? - double ef_actual = _ef_before - _ef_after; - double ef_expected = 0.01; // 1% should be enough - bool prog_ef = ef_actual >= ef_expected; + const double ef_after = _free_set->external_fragmentation(); + const double ef_actual = _ef_before - ef_after; + const double ef_expected = 0.01; // 1% should be enough + const bool prog_ef = ef_actual >= ef_expected; log_info(gc, ergo)("%s progress for external fragmentation: %.1f%%, need %.1f%%", prog_ef ? "Good" : "Bad", ef_actual * 100, ef_expected * 100); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahMetrics.hpp b/src/hotspot/share/gc/shenandoah/shenandoahMetrics.hpp index 20d8ebfd595..c554a065386 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahMetrics.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahMetrics.hpp @@ -25,22 +25,20 @@ #ifndef SHARE_GC_SHENANDOAH_SHENANDOAHMETRICS_HPP #define SHARE_GC_SHENANDOAH_SHENANDOAHMETRICS_HPP -#include "gc/shenandoah/shenandoahHeap.hpp" +#include "gc/shenandoah/shenandoahFreeSet.hpp" class ShenandoahMetricsSnapshot : public StackObj { private: - ShenandoahHeap* _heap; - size_t _used_before, _used_after; - double _if_before, _if_after; - double _ef_before, _ef_after; + ShenandoahFreeSet* _free_set; + size_t _used_before; + double _if_before; + double _ef_before; public: - ShenandoahMetricsSnapshot(); + explicit ShenandoahMetricsSnapshot(ShenandoahFreeSet* free_set); - void snap_before(); - void snap_after(); - - bool is_good_progress(ShenandoahGeneration *generation); + // Decide if the GC made "good" progress (i.e., reduced fragmentation, freed up sufficient memory). + bool is_good_progress() const; }; #endif // SHARE_GC_SHENANDOAH_SHENANDOAHMETRICS_HPP