From a2870d6b4985a68beb3fe3bf6622e6245e9a82ec Mon Sep 17 00:00:00 2001 From: Kelvin Nilsen Date: Wed, 24 Sep 2025 22:49:01 +0000 Subject: [PATCH] 8368015: Shenandoah: fix error in computation of average allocation rate Reviewed-by: wkemper --- .../shenandoahAdaptiveHeuristics.cpp | 33 ++++++++++++++----- .../shenandoahAdaptiveHeuristics.hpp | 27 ++++++++++----- .../heuristics/shenandoahHeuristics.hpp | 5 +++ .../heuristics/shenandoahSpaceInfo.hpp | 4 +++ .../gc/shenandoah/shenandoahGeneration.cpp | 4 +-- .../gc/shenandoah/shenandoahGeneration.hpp | 2 +- .../share/gc/shenandoah/shenandoahHeap.cpp | 25 +++++++++++--- 7 files changed, 75 insertions(+), 25 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp b/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp index 144773908ca..6498f0acdb6 100644 --- a/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp +++ b/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp @@ -240,13 +240,14 @@ bool ShenandoahAdaptiveHeuristics::should_start_gc() { log_debug(gc)("should_start_gc? available: %zu, soft_max_capacity: %zu" ", allocated: %zu", available, capacity, allocated); + // Track allocation rate even if we decide to start a cycle for other reasons. + double rate = _allocation_rate.sample(allocated); + if (_start_gc_is_pending) { log_trigger("GC start is already pending"); return true; } - // Track allocation rate even if we decide to start a cycle for other reasons. - double rate = _allocation_rate.sample(allocated); _last_trigger = OTHER; size_t min_threshold = min_free_threshold(); @@ -360,16 +361,32 @@ ShenandoahAllocationRate::ShenandoahAllocationRate() : _rate_avg(int(ShenandoahAdaptiveSampleSizeSeconds * ShenandoahAdaptiveSampleFrequencyHz), ShenandoahAdaptiveDecayFactor) { } +double ShenandoahAllocationRate::force_sample(size_t allocated, size_t &unaccounted_bytes_allocated) { + const double MinSampleTime = 0.002; // Do not sample if time since last update is less than 2 ms + double now = os::elapsedTime(); + double time_since_last_update = now -_last_sample_time; + if (time_since_last_update < MinSampleTime) { + unaccounted_bytes_allocated = allocated - _last_sample_value; + _last_sample_value = 0; + return 0.0; + } else { + double rate = instantaneous_rate(now, allocated); + _rate.add(rate); + _rate_avg.add(_rate.avg()); + _last_sample_time = now; + _last_sample_value = allocated; + unaccounted_bytes_allocated = 0; + return rate; + } +} + double ShenandoahAllocationRate::sample(size_t allocated) { double now = os::elapsedTime(); double rate = 0.0; if (now - _last_sample_time > _interval_sec) { - if (allocated >= _last_sample_value) { - rate = instantaneous_rate(now, allocated); - _rate.add(rate); - _rate_avg.add(_rate.avg()); - } - + rate = instantaneous_rate(now, allocated); + _rate.add(rate); + _rate_avg.add(_rate.avg()); _last_sample_time = now; _last_sample_value = allocated; } diff --git a/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.hpp b/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.hpp index 68e540960c7..014a4d99131 100644 --- a/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.hpp +++ b/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.hpp @@ -37,10 +37,12 @@ class ShenandoahAllocationRate : public CHeapObj { explicit ShenandoahAllocationRate(); void allocation_counter_reset(); + double force_sample(size_t allocated, size_t &unaccounted_bytes_allocated); double sample(size_t allocated); double upper_bound(double sds) const; bool is_spiking(double rate, double threshold) const; + private: double instantaneous_rate(double time, size_t allocated) const; @@ -71,18 +73,18 @@ public: virtual void choose_collection_set_from_regiondata(ShenandoahCollectionSet* cset, RegionData* data, size_t size, - size_t actual_free); + size_t actual_free) override; - void record_cycle_start(); - void record_success_concurrent(); - void record_success_degenerated(); - void record_success_full(); + virtual void record_cycle_start() override; + virtual void record_success_concurrent() override; + virtual void record_success_degenerated() override; + virtual void record_success_full() override; - virtual bool should_start_gc(); + virtual bool should_start_gc() override; - virtual const char* name() { return "Adaptive"; } - virtual bool is_diagnostic() { return false; } - virtual bool is_experimental() { return false; } + virtual const char* name() override { return "Adaptive"; } + virtual bool is_diagnostic() override { return false; } + virtual bool is_experimental() override { return false; } private: // These are used to adjust the margin of error and the spike threshold @@ -150,6 +152,13 @@ protected: _last_trigger = trigger_type; ShenandoahHeuristics::accept_trigger(); } + +public: + virtual size_t force_alloc_rate_sample(size_t bytes_allocated) override { + size_t unaccounted_bytes; + _allocation_rate.force_sample(bytes_allocated, unaccounted_bytes); + return unaccounted_bytes; + } }; #endif // SHARE_GC_SHENANDOAH_HEURISTICS_SHENANDOAHADAPTIVEHEURISTICS_HPP diff --git a/src/hotspot/share/gc/shenandoah/heuristics/shenandoahHeuristics.hpp b/src/hotspot/share/gc/shenandoah/heuristics/shenandoahHeuristics.hpp index d036a62a32c..6e3062d158f 100644 --- a/src/hotspot/share/gc/shenandoah/heuristics/shenandoahHeuristics.hpp +++ b/src/hotspot/share/gc/shenandoah/heuristics/shenandoahHeuristics.hpp @@ -241,6 +241,11 @@ public: double elapsed_cycle_time() const; + virtual size_t force_alloc_rate_sample(size_t bytes_allocated) { + // do nothing + return 0; + } + // Format prefix and emit log message indicating a GC cycle hs been triggered void log_trigger(const char* fmt, ...) ATTRIBUTE_PRINTF(2, 3); }; diff --git a/src/hotspot/share/gc/shenandoah/heuristics/shenandoahSpaceInfo.hpp b/src/hotspot/share/gc/shenandoah/heuristics/shenandoahSpaceInfo.hpp index 2131f95b413..7fb44c7b71b 100644 --- a/src/hotspot/share/gc/shenandoah/heuristics/shenandoahSpaceInfo.hpp +++ b/src/hotspot/share/gc/shenandoah/heuristics/shenandoahSpaceInfo.hpp @@ -41,6 +41,10 @@ public: virtual size_t soft_available() const = 0; virtual size_t available() const = 0; virtual size_t used() const = 0; + + // Return an approximation of the bytes allocated since GC start. The value returned is monotonically non-decreasing + // in time within each GC cycle. For certain GC cycles, the value returned may include some bytes allocated before + // the start of the current GC cycle. virtual size_t bytes_allocated_since_gc_start() const = 0; }; diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp index fafa3fde437..0c55613efcc 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp @@ -151,8 +151,8 @@ size_t ShenandoahGeneration::bytes_allocated_since_gc_start() const { return AtomicAccess::load(&_bytes_allocated_since_gc_start); } -void ShenandoahGeneration::reset_bytes_allocated_since_gc_start() { - AtomicAccess::store(&_bytes_allocated_since_gc_start, (size_t)0); +void ShenandoahGeneration::reset_bytes_allocated_since_gc_start(size_t initial_bytes_allocated) { + AtomicAccess::store(&_bytes_allocated_since_gc_start, initial_bytes_allocated); } void ShenandoahGeneration::increase_allocated(size_t bytes) { diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp index 9d70d4d76f3..e6597b3c1e4 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp @@ -142,7 +142,7 @@ private: size_t soft_available() const override; size_t bytes_allocated_since_gc_start() const override; - void reset_bytes_allocated_since_gc_start(); + void reset_bytes_allocated_since_gc_start(size_t initial_bytes_allocated); void increase_allocated(size_t bytes); // These methods change the capacity of the generation by adding or subtracting the given number of bytes from the current diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp index 714c9cb9f5b..10d7e8edcad 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp @@ -2319,12 +2319,27 @@ address ShenandoahHeap::in_cset_fast_test_addr() { } void ShenandoahHeap::reset_bytes_allocated_since_gc_start() { - if (mode()->is_generational()) { - young_generation()->reset_bytes_allocated_since_gc_start(); - old_generation()->reset_bytes_allocated_since_gc_start(); - } + // It is important to force_alloc_rate_sample() before the associated generation's bytes_allocated has been reset. + // Note that there is no lock to prevent additional alloations between sampling bytes_allocated_since_gc_start() and + // reset_bytes_allocated_since_gc_start(). If additional allocations happen, they will be ignored in the average + // allocation rate computations. This effect is considered to be be negligible. - global_generation()->reset_bytes_allocated_since_gc_start(); + // unaccounted_bytes is the bytes not accounted for by our forced sample. If the sample interval is too short, + // the "forced sample" will not happen, and any recently allocated bytes are "unaccounted for". We pretend these + // bytes are allocated after the start of subsequent gc. + size_t unaccounted_bytes; + if (mode()->is_generational()) { + size_t bytes_allocated = young_generation()->bytes_allocated_since_gc_start(); + unaccounted_bytes = young_generation()->heuristics()->force_alloc_rate_sample(bytes_allocated); + young_generation()->reset_bytes_allocated_since_gc_start(unaccounted_bytes); + unaccounted_bytes = 0; + old_generation()->reset_bytes_allocated_since_gc_start(unaccounted_bytes); + } else { + size_t bytes_allocated = global_generation()->bytes_allocated_since_gc_start(); + // Single-gen Shenandoah uses global heuristics. + unaccounted_bytes = heuristics()->force_alloc_rate_sample(bytes_allocated); + } + global_generation()->reset_bytes_allocated_since_gc_start(unaccounted_bytes); } void ShenandoahHeap::set_degenerated_gc_in_progress(bool in_progress) {