8368015: Shenandoah: fix error in computation of average allocation rate

Reviewed-by: wkemper

Author: kdnilsen

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;
   }

src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.hpp

@@ -37,10 +37,12 @@ class ShenandoahAllocationRate : public CHeapObj<mtGC> {
   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 @@ class ShenandoahAdaptiveHeuristics : public ShenandoahHeuristics {
 
   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 @@ class ShenandoahAdaptiveHeuristics : public ShenandoahHeuristics {
     _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

src/hotspot/share/gc/shenandoah/heuristics/shenandoahHeuristics.hpp

@@ -241,6 +241,11 @@ class ShenandoahHeuristics : public CHeapObj<mtGC> {
 
   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);
 };

src/hotspot/share/gc/shenandoah/heuristics/shenandoahSpaceInfo.hpp

@@ -41,6 +41,10 @@ class ShenandoahSpaceInfo {
   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;
 };
 

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) {

src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp

@@ -142,7 +142,7 @@ class ShenandoahGeneration : public CHeapObj<mtGC>, public ShenandoahSpaceInfo {
   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

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() {
+  // 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.
+
+  // 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()) {
-    young_generation()->reset_bytes_allocated_since_gc_start();
-    old_generation()->reset_bytes_allocated_since_gc_start();
+    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();
+  global_generation()->reset_bytes_allocated_since_gc_start(unaccounted_bytes);
 }
 
 void ShenandoahHeap::set_degenerated_gc_in_progress(bool in_progress) {