8372543: Shenandoah: undercalculated the available size when soft max takes effect

Reviewed-by: wkemper, kdnilsen

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

@@ -31,7 +31,6 @@
 #include "gc/shenandoah/heuristics/shenandoahSpaceInfo.hpp"
 #include "gc/shenandoah/shenandoahCollectionSet.hpp"
 #include "gc/shenandoah/shenandoahCollectorPolicy.hpp"
-#include "gc/shenandoah/shenandoahFreeSet.hpp"
 #include "gc/shenandoah/shenandoahHeap.inline.hpp"
 #include "gc/shenandoah/shenandoahHeapRegion.inline.hpp"
 #include "logging/log.hpp"
@@ -234,11 +233,12 @@ static double saturate(double value, double min, double max) {
 //    allocation rate computation independent.
 bool ShenandoahAdaptiveHeuristics::should_start_gc() {
   size_t capacity = ShenandoahHeap::heap()->soft_max_capacity();
-  size_t available = _space_info->soft_available();
+  size_t available = _space_info->soft_mutator_available();
   size_t allocated = _space_info->bytes_allocated_since_gc_start();
 
-  log_debug(gc)("should_start_gc? available: %zu, soft_max_capacity: %zu"
+  log_debug(gc, ergo)("should_start_gc calculation: available: " PROPERFMT ", soft_max_capacity: "  PROPERFMT ", "
-                ", allocated: %zu", available, capacity, allocated);
+                "allocated_since_gc_start: "  PROPERFMT,
+                PROPERFMTARGS(available), PROPERFMTARGS(capacity), PROPERFMTARGS(allocated));
 
   // Track allocation rate even if we decide to start a cycle for other reasons.
   double rate = _allocation_rate.sample(allocated);
@@ -252,9 +252,8 @@ bool ShenandoahAdaptiveHeuristics::should_start_gc() {
 
   size_t min_threshold = min_free_threshold();
   if (available < min_threshold) {
-    log_trigger("Free (%zu%s) is below minimum threshold (%zu%s)",
+    log_trigger("Free (Soft) (" PROPERFMT ") is below minimum threshold (" PROPERFMT ")",
-                 byte_size_in_proper_unit(available), proper_unit_for_byte_size(available),
+                 PROPERFMTARGS(available), PROPERFMTARGS(min_threshold));
-                 byte_size_in_proper_unit(min_threshold), proper_unit_for_byte_size(min_threshold));
     accept_trigger_with_type(OTHER);
     return true;
   }

src/hotspot/share/gc/shenandoah/heuristics/shenandoahCompactHeuristics.cpp

@@ -26,7 +26,6 @@
 
 #include "gc/shenandoah/heuristics/shenandoahCompactHeuristics.hpp"
 #include "gc/shenandoah/shenandoahCollectionSet.hpp"
-#include "gc/shenandoah/shenandoahFreeSet.hpp"
 #include "gc/shenandoah/shenandoahHeap.inline.hpp"
 #include "gc/shenandoah/shenandoahHeapRegion.inline.hpp"
 #include "logging/log.hpp"
@@ -47,26 +46,25 @@ ShenandoahCompactHeuristics::ShenandoahCompactHeuristics(ShenandoahSpaceInfo* sp
 }
 
 bool ShenandoahCompactHeuristics::should_start_gc() {
-  size_t max_capacity = _space_info->max_capacity();
   size_t capacity = ShenandoahHeap::heap()->soft_max_capacity();
-  size_t available = _space_info->available();
+  size_t available = _space_info->soft_mutator_available();
+  size_t bytes_allocated = _space_info->bytes_allocated_since_gc_start();
 
-  // Make sure the code below treats available without the soft tail.
+  log_debug(gc, ergo)("should_start_gc calculation: available: " PROPERFMT ", soft_max_capacity: "  PROPERFMT ", "
-  size_t soft_tail = max_capacity - capacity;
+                "allocated_since_gc_start: "  PROPERFMT,
-  available = (available > soft_tail) ? (available - soft_tail) : 0;
+                PROPERFMTARGS(available), PROPERFMTARGS(capacity), PROPERFMTARGS(bytes_allocated));
 
   size_t threshold_bytes_allocated = capacity / 100 * ShenandoahAllocationThreshold;
   size_t min_threshold = capacity / 100 * ShenandoahMinFreeThreshold;
 
   if (available < min_threshold) {
-    log_trigger("Free (%zu%s) is below minimum threshold (%zu%s)",
+    log_trigger("Free (Soft) (%zu%s) is below minimum threshold (%zu%s)",
                 byte_size_in_proper_unit(available),     proper_unit_for_byte_size(available),
                 byte_size_in_proper_unit(min_threshold), proper_unit_for_byte_size(min_threshold));
     accept_trigger();
     return true;
   }
 
-  size_t bytes_allocated = _space_info->bytes_allocated_since_gc_start();
   if (bytes_allocated > threshold_bytes_allocated) {
     log_trigger("Allocated since last cycle (%zu%s) is larger than allocation threshold (%zu%s)",
                 byte_size_in_proper_unit(bytes_allocated),           proper_unit_for_byte_size(bytes_allocated),

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

@@ -38,7 +38,7 @@ class ShenandoahSpaceInfo {
 public:
   virtual const char* name() const = 0;
   virtual size_t max_capacity() const = 0;
-  virtual size_t soft_available() const = 0;
+  virtual size_t soft_mutator_available() const = 0;
   virtual size_t available() const = 0;
   virtual size_t used() const = 0;
 

src/hotspot/share/gc/shenandoah/heuristics/shenandoahStaticHeuristics.cpp

@@ -26,7 +26,6 @@
 
 #include "gc/shenandoah/heuristics/shenandoahStaticHeuristics.hpp"
 #include "gc/shenandoah/shenandoahCollectionSet.hpp"
-#include "gc/shenandoah/shenandoahFreeSet.hpp"
 #include "gc/shenandoah/shenandoahHeap.inline.hpp"
 #include "gc/shenandoah/shenandoahHeapRegion.inline.hpp"
 #include "logging/log.hpp"
@@ -41,20 +40,19 @@ ShenandoahStaticHeuristics::ShenandoahStaticHeuristics(ShenandoahSpaceInfo* spac
 ShenandoahStaticHeuristics::~ShenandoahStaticHeuristics() {}
 
 bool ShenandoahStaticHeuristics::should_start_gc() {
-  size_t max_capacity = _space_info->max_capacity();
   size_t capacity = ShenandoahHeap::heap()->soft_max_capacity();
-  size_t available = _space_info->available();
+  size_t available = _space_info->soft_mutator_available();
+  size_t allocated = _space_info->bytes_allocated_since_gc_start();
 
-  // Make sure the code below treats available without the soft tail.
+  log_debug(gc, ergo)("should_start_gc calculation: available: " PROPERFMT ", soft_max_capacity: "  PROPERFMT ", "
-  size_t soft_tail = max_capacity - capacity;
+                "allocated_since_gc_start: "  PROPERFMT,
-  available = (available > soft_tail) ? (available - soft_tail) : 0;
+                PROPERFMTARGS(available), PROPERFMTARGS(capacity), PROPERFMTARGS(allocated));
 
   size_t threshold_available = capacity / 100 * ShenandoahMinFreeThreshold;
 
   if (available < threshold_available) {
-    log_trigger("Free (%zu%s) is below minimum threshold (%zu%s)",
+    log_trigger("Free (Soft) (" PROPERFMT ") is below minimum threshold (" PROPERFMT ")",
-                 byte_size_in_proper_unit(available),           proper_unit_for_byte_size(available),
+                 PROPERFMTARGS(available), PROPERFMTARGS(threshold_available));
-                 byte_size_in_proper_unit(threshold_available), proper_unit_for_byte_size(threshold_available));
     accept_trigger();
     return true;
   }

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp

@@ -2921,6 +2921,29 @@ void ShenandoahFreeSet::log_status_under_lock() {
   }
 }
 
+void ShenandoahFreeSet::log_freeset_stats(ShenandoahFreeSetPartitionId partition_id, LogStream& ls) {
+  size_t max = 0;
+  size_t total_free = 0;
+  size_t total_used = 0;
+
+  for (idx_t idx = _partitions.leftmost(partition_id);
+        idx <= _partitions.rightmost(partition_id); idx++) {
+    if (_partitions.in_free_set(partition_id, idx)) {
+      ShenandoahHeapRegion *r = _heap->get_region(idx);
+      size_t free = alloc_capacity(r);
+      max = MAX2(max, free);
+      total_free += free;
+      total_used += r->used();
+    }
+  }
+
+  ls.print(" %s freeset stats: Partition count: %zu, Reserved: " PROPERFMT ", Max free available in a single region: " PROPERFMT ";",
+            partition_name(partition_id),
+            _partitions.count(partition_id),
+            PROPERFMTARGS(total_free), PROPERFMTARGS(max)
+          );
+}
+
 void ShenandoahFreeSet::log_status() {
   shenandoah_assert_heaplocked();
 
@@ -3040,20 +3063,18 @@ void ShenandoahFreeSet::log_status() {
       // retired, the sum of used and capacities within regions that are still in the Mutator free partition may not match
       // my internally tracked values of used() and free().
       assert(free == total_free, "Free memory (%zu) should match calculated memory (%zu)", free, total_free);
-      ls.print("Free: %zu%s, Max: %zu%s regular, %zu%s humongous, ",
+      ls.print("Whole heap stats: Total free: " PROPERFMT ", Total used: " PROPERFMT ", Max free in a single region: " PROPERFMT
-               byte_size_in_proper_unit(total_free),    proper_unit_for_byte_size(total_free),
+               ", Max humongous: " PROPERFMT "; ",
-               byte_size_in_proper_unit(max),           proper_unit_for_byte_size(max),
+               PROPERFMTARGS(total_free), PROPERFMTARGS(total_used), PROPERFMTARGS(max), PROPERFMTARGS(max_humongous));
-               byte_size_in_proper_unit(max_humongous), proper_unit_for_byte_size(max_humongous)
-      );
 
-      ls.print("Frag: ");
+      ls.print("Frag stats: ");
       size_t frag_ext;
       if (total_free_ext > 0) {
         frag_ext = 100 - (100 * max_humongous / total_free_ext);
       } else {
         frag_ext = 0;
       }
-      ls.print("%zu%% external, ", frag_ext);
+      ls.print("External: %zu%%, ", frag_ext);
 
       size_t frag_int;
       if (_partitions.count(ShenandoahFreeSetPartitionId::Mutator) > 0) {
@@ -3062,52 +3083,13 @@ void ShenandoahFreeSet::log_status() {
       } else {
         frag_int = 0;
       }
-      ls.print("%zu%% internal; ", frag_int);
+      ls.print("Internal: %zu%%; ", frag_int);
-      ls.print("Used: %zu%s, Mutator Free: %zu",
-               byte_size_in_proper_unit(total_used), proper_unit_for_byte_size(total_used),
-               _partitions.count(ShenandoahFreeSetPartitionId::Mutator));
-    }
-
-    {
-      size_t max = 0;
-      size_t total_free = 0;
-      size_t total_used = 0;
-
-      for (idx_t idx = _partitions.leftmost(ShenandoahFreeSetPartitionId::Collector);
-           idx <= _partitions.rightmost(ShenandoahFreeSetPartitionId::Collector); idx++) {
-        if (_partitions.in_free_set(ShenandoahFreeSetPartitionId::Collector, idx)) {
-          ShenandoahHeapRegion *r = _heap->get_region(idx);
-          size_t free = alloc_capacity(r);
-          max = MAX2(max, free);
-          total_free += free;
-          total_used += r->used();
-        }
-      }
-      ls.print(" Collector Reserve: %zu%s, Max: %zu%s; Used: %zu%s",
-               byte_size_in_proper_unit(total_free), proper_unit_for_byte_size(total_free),
-               byte_size_in_proper_unit(max),        proper_unit_for_byte_size(max),
-               byte_size_in_proper_unit(total_used), proper_unit_for_byte_size(total_used));
     }
 
+    log_freeset_stats(ShenandoahFreeSetPartitionId::Mutator, ls);
+    log_freeset_stats(ShenandoahFreeSetPartitionId::Collector, ls);
     if (_heap->mode()->is_generational()) {
-      size_t max = 0;
+      log_freeset_stats(ShenandoahFreeSetPartitionId::OldCollector, ls);
-      size_t total_free = 0;
-      size_t total_used = 0;
-
-      for (idx_t idx = _partitions.leftmost(ShenandoahFreeSetPartitionId::OldCollector);
-           idx <= _partitions.rightmost(ShenandoahFreeSetPartitionId::OldCollector); idx++) {
-        if (_partitions.in_free_set(ShenandoahFreeSetPartitionId::OldCollector, idx)) {
-          ShenandoahHeapRegion *r = _heap->get_region(idx);
-          size_t free = alloc_capacity(r);
-          max = MAX2(max, free);
-          total_free += free;
-          total_used += r->used();
-        }
-      }
-      ls.print_cr(" Old Collector Reserve: %zu%s, Max: %zu%s; Used: %zu%s",
-                  byte_size_in_proper_unit(total_free), proper_unit_for_byte_size(total_free),
-                  byte_size_in_proper_unit(max),        proper_unit_for_byte_size(max),
-                  byte_size_in_proper_unit(total_used), proper_unit_for_byte_size(total_used));
     }
   }
 }

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

@@ -629,6 +629,7 @@ private:
   void establish_old_collector_alloc_bias();
   size_t get_usable_free_words(size_t free_bytes) const;
 
+  void log_freeset_stats(ShenandoahFreeSetPartitionId partition_id, LogStream& ls);
   // log status, assuming lock has already been acquired by the caller.
   void log_status();
 

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

@@ -938,8 +938,8 @@ size_t ShenandoahGeneration::available_with_reserve() const {
   return result;
 }
 
-size_t ShenandoahGeneration::soft_available() const {
+size_t ShenandoahGeneration::soft_mutator_available() const {
-  size_t result = available(ShenandoahHeap::heap()->soft_max_capacity());
+  size_t result = available(ShenandoahHeap::heap()->soft_max_capacity() * (100.0 - ShenandoahEvacReserve) / 100);
   return result;
 }
 

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

@@ -126,7 +126,7 @@ private:
   // The soft max heap size may be adjusted lower than the max heap size to cause the trigger
   // to believe it has less memory available than is _really_ available. Lowering the soft
   // max heap size will cause the adaptive heuristic to run more frequent cycles.
-  size_t soft_available() const override;
+  size_t soft_mutator_available() const override;
 
   void log_status(const char* msg) const;
 

src/hotspot/share/gc/shenandoah/shenandoahGlobalGeneration.cpp

@@ -78,15 +78,6 @@ size_t ShenandoahGlobalGeneration::available() const {
   return MIN2(available, ShenandoahHeap::heap()->free_set()->available());
 }
 
-size_t ShenandoahGlobalGeneration::soft_available() const {
-  size_t available = this->available();
-
-  // Make sure the code below treats available without the soft tail.
-  assert(max_capacity() >= ShenandoahHeap::heap()->soft_max_capacity(), "Max capacity must be greater than soft max capacity.");
-  size_t soft_tail = max_capacity() - ShenandoahHeap::heap()->soft_max_capacity();
-  return (available > soft_tail) ? (available - soft_tail) : 0;
-}
-
 void ShenandoahGlobalGeneration::set_concurrent_mark_in_progress(bool in_progress) {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   if (in_progress && heap->mode()->is_generational()) {

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

@@ -56,7 +56,6 @@ public:
   size_t max_capacity() const override;
 
   size_t available() const override;
-  size_t soft_available() const override;
 
   void set_concurrent_mark_in_progress(bool in_progress) override;
 

src/hotspot/share/gc/shenandoah/shenandoahYoungGeneration.cpp

@@ -138,8 +138,8 @@ size_t ShenandoahYoungGeneration::available() const {
   return MIN2(available, ShenandoahHeap::heap()->free_set()->available());
 }
 
-size_t ShenandoahYoungGeneration::soft_available() const {
+size_t ShenandoahYoungGeneration::soft_mutator_available() const {
-  size_t available = this->ShenandoahGeneration::soft_available();
+  size_t available = this->ShenandoahGeneration::soft_mutator_available();
   return MIN2(available, ShenandoahHeap::heap()->free_set()->available());
 }
 

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

@@ -83,7 +83,7 @@ public:
   size_t max_capacity() const override;
 
   size_t available() const override;
-  size_t soft_available() const override;
+  size_t soft_mutator_available() const override;
 
   void prepare_gc() override;
 };

test/hotspot/jtreg/gc/shenandoah/TestSoftMaxHeapSizeAvailableCalc.java

@@ -0,0 +1,123 @@
+/*
+ * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/**
+ * @test id=satb-adaptive
+ * @requires vm.gc.Shenandoah
+ * @library /test/lib
+ * @bug 8372543
+ * @summary When soft max heap size < Xmx, we had a bug reported in JBS-8372543 where available size was undercalculated.
+ *          This caused excessive GC runs.
+ *
+ * @run main/othervm -XX:SoftMaxHeapSize=512m -Xmx2g -XX:+UnlockDiagnosticVMOptions -XX:+UnlockExperimentalVMOptions
+ *      -XX:+UseShenandoahGC -Xlog:gc=info
+ *      -XX:ShenandoahGCMode=satb
+ *      -XX:+ShenandoahDegeneratedGC
+ *      -XX:ShenandoahGCHeuristics=adaptive
+ *      TestSoftMaxHeapSizeAvailableCalc
+ */
+
+/**
+ * @test id=satb-static
+ * @requires vm.gc.Shenandoah
+ * @library /test/lib
+ *
+ * @run main/othervm -XX:SoftMaxHeapSize=512m -Xmx2g -XX:+UnlockDiagnosticVMOptions -XX:+UnlockExperimentalVMOptions
+ *      -XX:+UseShenandoahGC -Xlog:gc=info
+ *      -XX:ShenandoahGCMode=satb
+ *      -XX:+ShenandoahDegeneratedGC
+ *      -XX:ShenandoahGCHeuristics=static
+ *      TestSoftMaxHeapSizeAvailableCalc
+ */
+
+/**
+ * @test id=generational
+ * @requires vm.gc.Shenandoah
+ * @library /test/lib
+ *
+ * @run main/othervm -XX:SoftMaxHeapSize=512m -Xmx2g -XX:+UnlockDiagnosticVMOptions -XX:+UnlockExperimentalVMOptions
+ *      -XX:+UseShenandoahGC -Xlog:gc=info
+ *      -XX:ShenandoahGCMode=generational
+ *      -XX:ShenandoahGCHeuristics=adaptive
+ *      TestSoftMaxHeapSizeAvailableCalc
+ *
+ */
+import java.lang.management.ManagementFactory;
+
+import jdk.test.lib.Asserts;
+import jdk.test.lib.Utils;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.dcmd.PidJcmdExecutor;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Random;
+
+import com.sun.management.GarbageCollectorMXBean;
+
+public class TestSoftMaxHeapSizeAvailableCalc {
+    public static void main(String[] args) throws Exception {
+        Allocate.test();
+    }
+
+    // This test runs an app that has a stable heap of ~300M and allocates temporary garbage at ~100M/s
+    // Soft max: 512M, ShenandoahMinFreeThreshold: 10 (default), ShenandoahEvacReserve: 5 (default)
+    // Soft max for mutator: 512M * (100.0 - 5) / 100 = 486.4M
+    // Threshold to trigger gc: 486.4M - 512 * 10 / 100.0 = 435.2M, just above (300 + 100)M.
+    // Expect gc count to be less than 1 / sec.
+    public static class Allocate {
+        static final List<byte[]> longLived = new ArrayList<>();
+
+        public static void test() throws Exception {
+            final int expectedMaxGcCount = Integer.getInteger("expectedMaxGcCount", 30);
+            List<java.lang.management.GarbageCollectorMXBean> collectors = ManagementFactory.getGarbageCollectorMXBeans();
+            java.lang.management.GarbageCollectorMXBean cycleCollector = null;
+            for (java.lang.management.GarbageCollectorMXBean bean : collectors) {
+                if (bean.getName().contains("Cycles")) {
+                    cycleCollector = bean;
+                }
+            }
+
+            // Allocate ~300MB of long-lived objects
+            for (int i = 0; i < 300; i++) {
+                longLived.add(new byte[1_000_000]);
+            }
+
+            // allocate short-lived garbage to the heap
+            long end = System.currentTimeMillis() + 30_000; // 30 seconds
+
+            while (System.currentTimeMillis() < end) {
+                byte[] garbage = new byte[1_000_000];
+                garbage[0] = 1; // prevent optimization
+
+                Thread.sleep(10); // Pace to generate garbage at speed of ~100M/s
+            }
+
+            long gcCount = cycleCollector.getCollectionCount();
+            Asserts.assertLessThan(gcCount, (long) expectedMaxGcCount, "GC was triggered too many times. Expected to be less than: " + expectedMaxGcCount + ", triggered: " + gcCount);
+        }
+    }
+}