8205022: ZGC: SoftReferences not always cleared before throwing OOME

Reviewed-by: stefank, eosterlund

src/hotspot/share/gc/z/zDriver.cpp

@@ -126,6 +126,26 @@ public:
   }
 };
 
+static bool should_clear_soft_references() {
+  // Clear if one or more allocations have stalled
+  const bool stalled = ZHeap::heap()->is_alloc_stalled();
+  if (stalled) {
+    // Clear
+    return true;
+  }
+
+  // Clear if implied by the GC cause
+  const GCCause::Cause cause = ZCollectedHeap::heap()->gc_cause();
+  if (cause == GCCause::_wb_full_gc ||
+      cause == GCCause::_metadata_GC_clear_soft_refs) {
+    // Clear
+    return true;
+  }
+
+  // Don't clear
+  return false;
+}
+
 class ZMarkStartClosure : public ZOperationClosure {
 public:
   virtual const char* name() const {
@@ -140,6 +160,10 @@ public:
     ZStatTimer timer(ZPhasePauseMarkStart);
     ZServiceabilityMarkStartTracer tracer;
 
+    // Setup soft reference policy
+    const bool clear = should_clear_soft_references();
+    ZHeap::heap()->set_soft_reference_policy(clear);
+
     ZCollectedHeap::heap()->increment_total_collections(true /* full */);
 
     ZHeap::heap()->mark_start();
@@ -247,40 +271,11 @@ GCCause::Cause ZDriver::start_gc_cycle() {
   return _gc_cycle_port.receive();
 }
 
-class ZSoftReferencePolicyScope : public StackObj {
-private:
-  bool should_clear_soft_reference(GCCause::Cause cause) const {
-    const bool clear = ZCollectedHeap::heap()->soft_ref_policy()->should_clear_all_soft_refs();
-
-    // Clear all soft reference if the policy says so, or if
-    // the GC cause indicates that we're running low on memory.
-    return clear ||
-           cause == GCCause::_z_allocation_stall ||
-           cause == GCCause::_metadata_GC_clear_soft_refs;
-  }
-
-  void clear_should_clear_soft_reference() const {
-    ZCollectedHeap::heap()->soft_ref_policy()->set_should_clear_all_soft_refs(false);
-  }
-
-public:
-  ZSoftReferencePolicyScope(GCCause::Cause cause) {
-    const bool clear = should_clear_soft_reference(cause);
-    ZHeap::heap()->set_soft_reference_policy(clear);
-    clear_should_clear_soft_reference();
-  }
-
-  ~ZSoftReferencePolicyScope() {
-    Universe::update_heap_info_at_gc();
-  }
-};
-
 class ZDriverCycleScope : public StackObj {
 private:
-  GCIdMark                  _gc_id;
-  GCCauseSetter             _gc_cause_setter;
-  ZSoftReferencePolicyScope _soft_ref_policy;
-  ZStatTimer                _timer;
+  GCIdMark      _gc_id;
+  GCCauseSetter _gc_cause_setter;
+  ZStatTimer    _timer;
 
   bool should_boost_worker_threads(GCCause::Cause cause) const {
     return cause == GCCause::_java_lang_system_gc ||
@@ -291,7 +286,6 @@ public:
   ZDriverCycleScope(GCCause::Cause cause) :
       _gc_id(),
       _gc_cause_setter(ZCollectedHeap::heap(), cause),
-      _soft_ref_policy(cause),
       _timer(ZPhaseCycle) {
     // Update statistics
     ZStatCycle::at_start();
@@ -308,6 +302,9 @@ public:
 
     // Update statistics
     ZStatCycle::at_end(boost_factor);
+
+    // Update data used by soft reference policy
+    Universe::update_heap_info_at_gc();
   }
 };
 

src/hotspot/share/gc/z/zHeap.hpp

@@ -124,6 +124,7 @@ public:
   uintptr_t alloc_object(size_t size);
   uintptr_t alloc_object_for_relocation(size_t size);
   void undo_alloc_object_for_relocation(uintptr_t addr, size_t size);
+  bool is_alloc_stalled() const;
   void check_out_of_memory();
 
   // Marking

src/hotspot/share/gc/z/zHeap.inline.hpp

@@ -89,6 +89,10 @@ inline void ZHeap::undo_alloc_object_for_relocation(uintptr_t addr, size_t size)
   _object_allocator.undo_alloc_object_for_relocation(page, addr, size);
 }
 
+inline bool ZHeap::is_alloc_stalled() const {
+  return _page_allocator.is_alloc_stalled();
+}
+
 inline void ZHeap::check_out_of_memory() {
   _page_allocator.check_out_of_memory();
 }

src/hotspot/share/gc/z/zPageAllocator.cpp

@@ -459,24 +459,25 @@ void ZPageAllocator::free_page(ZPage* page, bool reclaimed) {
   satisfy_alloc_queue();
 }
 
+bool ZPageAllocator::is_alloc_stalled() const {
+  assert(SafepointSynchronize::is_at_safepoint(), "Should be at safepoint");
+  return !_queue.is_empty();
+}
+
 void ZPageAllocator::check_out_of_memory() {
   ZLocker locker(&_lock);
 
-  ZPageAllocRequest* const first = _queue.first();
-  if (first == NULL) {
-    // Allocation queue is empty
-    return;
-  }
-
-  // Fail the allocation request if it was enqueued before the
+  // Fail allocation requests that were enqueued before the
   // last GC cycle started, otherwise start a new GC cycle.
-  if (first->total_collections() < ZCollectedHeap::heap()->total_collections()) {
-    // Out of memory, fail all enqueued requests
-    for (ZPageAllocRequest* request = _queue.remove_first(); request != NULL; request = _queue.remove_first()) {
-      request->satisfy(NULL);
+  for (ZPageAllocRequest* request = _queue.first(); request != NULL; request = _queue.first()) {
+    if (request->total_collections() == ZCollectedHeap::heap()->total_collections()) {
+      // Start a new GC cycle, keep allocation requests enqueued
+      request->satisfy(gc_marker);
+      return;
     }
-  } else {
-    // Start another GC cycle, keep all enqueued requests
-    first->satisfy(gc_marker);
+
+    // Out of memory, fail allocation request
+    _queue.remove_first();
+    request->satisfy(NULL);
   }
 }

src/hotspot/share/gc/z/zPageAllocator.hpp

@@ -102,6 +102,7 @@ public:
 
   void flip_pre_mapped();
 
+  bool is_alloc_stalled() const;
   void check_out_of_memory();
 };