8316632: Shenandoah: Raise OOME when gc threshold is exceeded

Reviewed-by: kdnilsen, ysr, shade

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

@@ -524,7 +524,7 @@ void ShenandoahControlThread::handle_requested_gc(GCCause::Cause cause) {
   }
 }
 
-void ShenandoahControlThread::handle_alloc_failure(ShenandoahAllocRequest& req) {
+void ShenandoahControlThread::handle_alloc_failure(ShenandoahAllocRequest& req, bool block) {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
 
   assert(current()->is_Java_thread(), "expect Java thread here");
@@ -539,9 +539,12 @@ void ShenandoahControlThread::handle_alloc_failure(ShenandoahAllocRequest& req)
     heap->cancel_gc(GCCause::_allocation_failure);
   }
 
-  MonitorLocker ml(&_alloc_failure_waiters_lock);
-  while (is_alloc_failure_gc()) {
-    ml.wait();
+
+  if (block) {
+    MonitorLocker ml(&_alloc_failure_waiters_lock);
+    while (is_alloc_failure_gc()) {
+      ml.wait();
+    }
   }
 }
 

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

@@ -121,12 +121,13 @@ class ShenandoahControlThread: public ConcurrentGCThread {
   ShenandoahControlThread();
   ~ShenandoahControlThread();
 
-  // Handle allocation failure from normal allocation.
-  // Blocks until memory is available.
-  void handle_alloc_failure(ShenandoahAllocRequest& req);
+  // Handle allocation failure from a mutator allocation.
+  // Optionally blocks while collector is handling the failure. If the GC
+  // threshold has been exceeded, the mutator allocation will not block so
+  // that the out of memory error can be raised promptly.
+  void handle_alloc_failure(ShenandoahAllocRequest& req, bool block = true);
 
   // Handle allocation failure from evacuation path.
-  // Optionally blocks while collector is handling the failure.
   void handle_alloc_failure_evac(size_t words);
 
   void request_gc(GCCause::Cause cause);

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

@@ -503,6 +503,7 @@ ShenandoahHeap::ShenandoahHeap(ShenandoahCollectorPolicy* policy) :
   _num_regions(0),
   _regions(nullptr),
   _update_refs_iterator(this),
+  _gc_no_progress_count(0),
   _control_thread(nullptr),
   _shenandoah_policy(policy),
   _gc_mode(nullptr),
@@ -873,7 +874,19 @@ HeapWord* ShenandoahHeap::allocate_memory(ShenandoahAllocRequest& req) {
       result = allocate_memory_under_lock(req, in_new_region);
     }
 
-    // Allocation failed, block until control thread reacted, then retry allocation.
+    // Check that gc overhead is not exceeded.
+    //
+    // Shenandoah will grind along for quite a while allocating one
+    // object at a time using shared (non-tlab) allocations. This check
+    // is testing that the GC overhead limit has not been exceeded.
+    // This will notify the collector to start a cycle, but will raise
+    // an OOME to the mutator if the last Full GCs have not made progress.
+    if (result == nullptr && !req.is_lab_alloc() && get_gc_no_progress_count() > ShenandoahNoProgressThreshold) {
+      control_thread()->handle_alloc_failure(req, false);
+      return nullptr;
+    }
+
+    // Block until control thread reacted, then retry allocation.
     //
     // It might happen that one of the threads requesting allocation would unblock
     // way later after GC happened, only to fail the second allocation, because
@@ -882,10 +895,16 @@ HeapWord* ShenandoahHeap::allocate_memory(ShenandoahAllocRequest& req) {
     // one full GC has completed).
     size_t original_count = shenandoah_policy()->full_gc_count();
     while (result == nullptr
-        && (_progress_last_gc.is_set() || original_count == shenandoah_policy()->full_gc_count())) {
+        && (get_gc_no_progress_count() == 0 || original_count == shenandoah_policy()->full_gc_count())) {
       control_thread()->handle_alloc_failure(req);
       result = allocate_memory_under_lock(req, in_new_region);
     }
+
+    if (log_is_enabled(Debug, gc, alloc)) {
+      ResourceMark rm;
+      log_debug(gc, alloc)("Thread: %s, Result: " PTR_FORMAT ", Request: %s, Size: " SIZE_FORMAT ", Original: " SIZE_FORMAT ", Latest: " SIZE_FORMAT,
+                           Thread::current()->name(), p2i(result), req.type_string(), req.size(), original_count, get_gc_no_progress_count());
+    }
   } else {
     assert(req.is_gc_alloc(), "Can only accept GC allocs here");
     result = allocate_memory_under_lock(req, in_new_region);

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

@@ -287,9 +287,10 @@ class ShenandoahHeap : public CollectedHeap, public ShenandoahSpaceInfo {
   ShenandoahSharedFlag   _degenerated_gc_in_progress;
   ShenandoahSharedFlag   _full_gc_in_progress;
   ShenandoahSharedFlag   _full_gc_move_in_progress;
-  ShenandoahSharedFlag   _progress_last_gc;
   ShenandoahSharedFlag   _concurrent_strong_root_in_progress;
 
+  size_t _gc_no_progress_count;
+
   void set_gc_state_all_threads(char state);
   void set_gc_state_mask(uint mask, bool value);
 
@@ -370,8 +371,9 @@ class ShenandoahHeap : public CollectedHeap, public ShenandoahSpaceInfo {
   void rendezvous_threads();
   void recycle_trash();
 public:
-  void notify_gc_progress()    { _progress_last_gc.set();   }
-  void notify_gc_no_progress() { _progress_last_gc.unset(); }
+  void notify_gc_progress();
+  void notify_gc_no_progress();
+  size_t get_gc_no_progress_count() const;
 
 //
 // Mark support

src/hotspot/share/gc/shenandoah/shenandoahHeap.inline.hpp

@@ -73,6 +73,18 @@ inline WorkerThreads* ShenandoahHeap::safepoint_workers() {
   return _safepoint_workers;
 }
 
+inline void ShenandoahHeap::notify_gc_progress() {
+  Atomic::store(&_gc_no_progress_count, (size_t) 0);
+
+}
+inline void ShenandoahHeap::notify_gc_no_progress() {
+  Atomic::inc(&_gc_no_progress_count);
+}
+
+inline size_t ShenandoahHeap::get_gc_no_progress_count() const {
+  return Atomic::load(&_gc_no_progress_count);
+}
+
 inline size_t ShenandoahHeap::heap_region_index_containing(const void* addr) const {
   uintptr_t region_start = ((uintptr_t) addr);
   uintptr_t index = (region_start - (uintptr_t) base()) >> ShenandoahHeapRegion::region_size_bytes_shift();

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

@@ -294,6 +294,11 @@
           "How many back-to-back Degenerated GCs should happen before "     \
           "going to a Full GC.")                                            \
                                                                             \
+  product(uintx, ShenandoahNoProgressThreshold, 5, EXPERIMENTAL,            \
+          "After this number of consecutive Full GCs fail to make "         \
+          "progress, Shenandoah will raise out of memory errors. Note "     \
+          "that progress is determined by ShenandoahCriticalFreeThreshold") \
+                                                                            \
   product(bool, ShenandoahImplicitGCInvokesConcurrent, false, EXPERIMENTAL, \
           "Should internally-caused GC requests invoke concurrent cycles, " \
           "should they do the stop-the-world (Degenerated / Full GC)? "     \

test/jdk/com/sun/jdi/EATests.java

@@ -272,6 +272,7 @@ public static class TargetVMOptions {
         public final boolean DeoptimizeObjectsALot;
         public final boolean DoEscapeAnalysis;
         public final boolean ZGCIsSelected;
+        public final boolean ShenandoahGCIsSelected;
         public final boolean StressReflectiveCode;
 
         public TargetVMOptions(EATests env, ClassType testCaseBaseTargetClass) {
@@ -287,6 +288,8 @@ public TargetVMOptions(EATests env, ClassType testCaseBaseTargetClass) {
             UseJVMCICompiler = ((PrimitiveValue) val).booleanValue();
             val = testCaseBaseTargetClass.getValue(testCaseBaseTargetClass.fieldByName("ZGCIsSelected"));
             ZGCIsSelected = ((PrimitiveValue) val).booleanValue();
+            val = testCaseBaseTargetClass.getValue(testCaseBaseTargetClass.fieldByName("ShenandoahGCIsSelected"));
+            ShenandoahGCIsSelected = ((PrimitiveValue) val).booleanValue();
             val = testCaseBaseTargetClass.getValue(testCaseBaseTargetClass.fieldByName("StressReflectiveCode"));
             StressReflectiveCode = ((PrimitiveValue) val).booleanValue();
         }
@@ -773,6 +776,7 @@ public static boolean unbox(Boolean value, boolean dflt) {
     public static final boolean EliminateAllocations = unbox(WB.getBooleanVMFlag("EliminateAllocations"), UseJVMCICompiler);
     public static final boolean DeoptimizeObjectsALot = WB.getBooleanVMFlag("DeoptimizeObjectsALot");
     public static final boolean ZGCIsSelected = GC.Z.isSelected();
+    public static final boolean ShenandoahGCIsSelected = GC.Shenandoah.isSelected();
     public static final boolean StressReflectiveCode = unbox(WB.getBooleanVMFlag("StressReflectiveCode"), false);
 
     public String testMethodName;
@@ -2450,8 +2454,9 @@ public boolean shouldSkip() {
         // And Graal currently doesn't provide all information about non-escaping objects in debug info
         return super.shouldSkip() ||
                 !env.targetVMOptions.EliminateAllocations ||
-                // With ZGC the OOME is not always thrown as expected
+                // With ZGC or Shenandoah the OOME is not always thrown as expected
                 env.targetVMOptions.ZGCIsSelected ||
+                env.targetVMOptions.ShenandoahGCIsSelected ||
                 env.targetVMOptions.DeoptimizeObjectsALot ||
                 env.targetVMOptions.UseJVMCICompiler;
     }
@@ -2495,8 +2500,9 @@ public boolean shouldSkip() {
         // And Graal currently doesn't provide all information about non-escaping objects in debug info
         return super.shouldSkip() ||
                 !EliminateAllocations ||
-                // With ZGC the OOME is not always thrown as expected
+                // With ZGC or Shenandoah the OOME is not always thrown as expected
                 ZGCIsSelected ||
+                ShenandoahGCIsSelected ||
                 DeoptimizeObjectsALot ||
                 UseJVMCICompiler;
     }
@@ -2548,8 +2554,9 @@ public boolean shouldSkip() {
         // And Graal currently doesn't provide all information about non-escaping objects in debug info
         return super.shouldSkip() ||
                 !env.targetVMOptions.EliminateAllocations ||
-                // With ZGC the OOME is not always thrown as expected
+                // With ZGC or Shenandoah the OOME is not always thrown as expected
                 env.targetVMOptions.ZGCIsSelected ||
+                env.targetVMOptions.ShenandoahGCIsSelected ||
                 env.targetVMOptions.DeoptimizeObjectsALot ||
                 env.targetVMOptions.UseJVMCICompiler;
     }
@@ -2609,8 +2616,9 @@ public boolean shouldSkip() {
         // And Graal currently doesn't provide all information about non-escaping objects in debug info
         return super.shouldSkip() ||
                 !EliminateAllocations ||
-                // With ZGC the OOME is not always thrown as expected
+                // With ZGC or Shenandoah the OOME is not always thrown as expected
                 ZGCIsSelected ||
+                ShenandoahGCIsSelected ||
                 DeoptimizeObjectsALot ||
                 UseJVMCICompiler;
     }
@@ -2815,8 +2823,9 @@ public boolean shouldSkip() {
         // And Graal currently doesn't support Force Early Return
         return super.shouldSkip() ||
                 !env.targetVMOptions.EliminateAllocations ||
-                // With ZGC the OOME is not always thrown as expected
+                // With ZGC or Shenandoah the OOME is not always thrown as expected
                 env.targetVMOptions.ZGCIsSelected ||
+                env.targetVMOptions.ShenandoahGCIsSelected ||
                 env.targetVMOptions.DeoptimizeObjectsALot ||
                 env.targetVMOptions.UseJVMCICompiler;
     }
@@ -2877,8 +2886,9 @@ public boolean shouldSkip() {
         // And Graal currently doesn't support Force Early Return
         return super.shouldSkip() ||
                 !EliminateAllocations ||
-                // With ZGC the OOME is not always thrown as expected
+                // With ZGC or Shenandoah the OOME is not always thrown as expected
                 ZGCIsSelected ||
+                ShenandoahGCIsSelected ||
                 DeoptimizeObjectsALot ||
                 UseJVMCICompiler;
     }