8349094: GenShen: Race between control and regulator threads may violate assertions

8352428: GenShen: Old-gen cycles are still looping
8352091: GenShen: assert(!(request.generation->is_old() && _heap->old_generation()->is_doing_mixed_evacuations())) failed: Old heuristic should not request cycles while it waits for mixed evacuation
8351464: Shenandoah: Hang on ShenandoahController::handle_alloc_failure when run test TestAllocHumongousFragment#generational
8352299: GenShen: Young cycles that interrupt old cycles cannot be cancelled

Reviewed-by: kdnilsen
Backport-of: 3a8a432

Author: William Kemper

src/hotspot/share/gc/shared/gcCause.hpp

@@ -77,6 +77,7 @@ class GCCause : public AllStatic {
 
     _shenandoah_stop_vm,
     _shenandoah_allocation_failure_evac,
+    _shenandoah_humongous_allocation_failure,
     _shenandoah_concurrent_gc,
     _shenandoah_upgrade_to_full_gc,
 

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

@@ -188,6 +188,7 @@ void ShenandoahHeuristics::record_cycle_end() {
 
 bool ShenandoahHeuristics::should_start_gc() {
   if (_start_gc_is_pending) {
+    log_trigger("GC start is already pending");
     return true;
   }
   // Perform GC to cleanup metaspace

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

@@ -62,7 +62,6 @@ int ShenandoahOldHeuristics::compare_by_index(RegionData a, RegionData b) {
 ShenandoahOldHeuristics::ShenandoahOldHeuristics(ShenandoahOldGeneration* generation, ShenandoahGenerationalHeap* gen_heap) :
   ShenandoahHeuristics(generation),
   _heap(gen_heap),
-  _old_gen(generation),
   _first_pinned_candidate(NOT_FOUND),
   _last_old_collection_candidate(0),
   _next_old_collection_candidate(0),
@@ -569,9 +568,9 @@ void ShenandoahOldHeuristics::set_trigger_if_old_is_fragmented(size_t first_old_
     // allocation request will require a STW full GC.
     size_t allowed_old_gen_span = num_regions - (ShenandoahGenerationalHumongousReserve * num_regions) / 100;
 
-    size_t old_available = _old_gen->available() / HeapWordSize;
+    size_t old_available = _old_generation->available() / HeapWordSize;
     size_t region_size_words = ShenandoahHeapRegion::region_size_words();
-    size_t old_unaffiliated_available = _old_gen->free_unaffiliated_regions() * region_size_words;
+    size_t old_unaffiliated_available = _old_generation->free_unaffiliated_regions() * region_size_words;
     assert(old_available >= old_unaffiliated_available, "sanity");
     size_t old_fragmented_available = old_available - old_unaffiliated_available;
 
@@ -605,12 +604,12 @@ void ShenandoahOldHeuristics::set_trigger_if_old_is_fragmented(size_t first_old_
 }
 
 void ShenandoahOldHeuristics::set_trigger_if_old_is_overgrown() {
-  size_t old_used = _old_gen->used() + _old_gen->get_humongous_waste();
-  size_t trigger_threshold = _old_gen->usage_trigger_threshold();
+  size_t old_used = _old_generation->used() + _old_generation->get_humongous_waste();
+  size_t trigger_threshold = _old_generation->usage_trigger_threshold();
   // Detects unsigned arithmetic underflow
   assert(old_used <= _heap->capacity(),
          "Old used (" SIZE_FORMAT ", " SIZE_FORMAT") must not be more than heap capacity (" SIZE_FORMAT ")",
-         _old_gen->used(), _old_gen->get_humongous_waste(), _heap->capacity());
+         _old_generation->used(), _old_generation->get_humongous_waste(), _heap->capacity());
   if (old_used > trigger_threshold) {
     _growth_trigger = true;
   }
@@ -622,13 +621,32 @@ void ShenandoahOldHeuristics::evaluate_triggers(size_t first_old_region, size_t
   set_trigger_if_old_is_overgrown();
 }
 
+bool ShenandoahOldHeuristics::should_resume_old_cycle() {
+  // If we are preparing to mark old, or if we are already marking old, then try to continue that work.
+  if (_old_generation->is_concurrent_mark_in_progress()) {
+    assert(_old_generation->state() == ShenandoahOldGeneration::MARKING, "Unexpected old gen state: %s", _old_generation->state_name());
+    log_trigger("Resume marking old");
+    return true;
+  }
+
+  if (_old_generation->is_preparing_for_mark()) {
+    assert(_old_generation->state() == ShenandoahOldGeneration::FILLING, "Unexpected old gen state: %s", _old_generation->state_name());
+    log_trigger("Resume preparing to mark old");
+    return true;
+  }
+
+  return false;
+}
+
 bool ShenandoahOldHeuristics::should_start_gc() {
-  // Cannot start a new old-gen GC until previous one has finished.
-  //
-  // Future refinement: under certain circumstances, we might be more sophisticated about this choice.
-  // For example, we could choose to abandon the previous old collection before it has completed evacuations.
-  ShenandoahHeap* heap = ShenandoahHeap::heap();
-  if (!_old_generation->can_start_gc() || heap->collection_set()->has_old_regions()) {
+
+  const ShenandoahHeap* heap = ShenandoahHeap::heap();
+  if (_old_generation->is_doing_mixed_evacuations()) {
+    // Do not try to start an old cycle if we are waiting for old regions to be evacuated (we need
+    // a young cycle for this). Note that the young heuristic has a feature to expedite old evacuations.
+    // Future refinement: under certain circumstances, we might be more sophisticated about this choice.
+    // For example, we could choose to abandon the previous old collection before it has completed evacuations.
+    log_debug(gc)("Not starting an old cycle because we are waiting for mixed evacuations");
     return false;
   }
 

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

@@ -53,7 +53,6 @@ class ShenandoahOldHeuristics : public ShenandoahHeuristics {
   static uint NOT_FOUND;
 
   ShenandoahGenerationalHeap* _heap;
-  ShenandoahOldGeneration* _old_gen;
 
   // After final marking of the old generation, this heuristic will select
   // a set of candidate regions to be included in subsequent mixed collections.
@@ -186,6 +185,9 @@ class ShenandoahOldHeuristics : public ShenandoahHeuristics {
 
   bool should_start_gc() override;
 
+  // Returns true if the old generation needs to prepare for marking, or continue marking.
+  bool should_resume_old_cycle();
+
   void record_success_concurrent() override;
 
   void record_success_degenerated() override;

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

@@ -123,25 +123,28 @@ void ShenandoahCollectorPolicy::record_shutdown() {
   _in_shutdown.set();
 }
 
-bool ShenandoahCollectorPolicy::is_at_shutdown() {
+bool ShenandoahCollectorPolicy::is_at_shutdown() const {
   return _in_shutdown.is_set();
 }
 
-bool is_explicit_gc(GCCause::Cause cause) {
+bool ShenandoahCollectorPolicy::is_explicit_gc(GCCause::Cause cause) {
   return GCCause::is_user_requested_gc(cause)
-      || GCCause::is_serviceability_requested_gc(cause);
+      || GCCause::is_serviceability_requested_gc(cause)
+      || cause == GCCause::_wb_full_gc
+      || cause == GCCause::_wb_young_gc;
 }
 
 bool is_implicit_gc(GCCause::Cause cause) {
   return cause != GCCause::_no_gc
       && cause != GCCause::_shenandoah_concurrent_gc
       && cause != GCCause::_allocation_failure
-      && !is_explicit_gc(cause);
+      && !ShenandoahCollectorPolicy::is_explicit_gc(cause);
 }
 
 #ifdef ASSERT
 bool is_valid_request(GCCause::Cause cause) {
-  return is_explicit_gc(cause)
+  return ShenandoahCollectorPolicy::is_explicit_gc(cause)
+      || ShenandoahCollectorPolicy::is_shenandoah_gc(cause)
       || cause == GCCause::_metadata_GC_clear_soft_refs
       || cause == GCCause::_codecache_GC_aggressive
       || cause == GCCause::_codecache_GC_threshold
@@ -153,6 +156,22 @@ bool is_valid_request(GCCause::Cause cause) {
 }
 #endif
 
+bool ShenandoahCollectorPolicy::is_shenandoah_gc(GCCause::Cause cause) {
+  return cause == GCCause::_allocation_failure
+      || cause == GCCause::_shenandoah_stop_vm
+      || cause == GCCause::_shenandoah_allocation_failure_evac
+      || cause == GCCause::_shenandoah_humongous_allocation_failure
+      || cause == GCCause::_shenandoah_concurrent_gc
+      || cause == GCCause::_shenandoah_upgrade_to_full_gc;
+}
+
+
+bool ShenandoahCollectorPolicy::is_allocation_failure(GCCause::Cause cause) {
+  return cause == GCCause::_allocation_failure
+      || cause == GCCause::_shenandoah_allocation_failure_evac
+      || cause == GCCause::_shenandoah_humongous_allocation_failure;
+}
+
 bool ShenandoahCollectorPolicy::is_requested_gc(GCCause::Cause cause) {
   return is_explicit_gc(cause) || is_implicit_gc(cause);
 }

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

@@ -77,9 +77,9 @@ class ShenandoahCollectorPolicy : public CHeapObj<mtGC> {
   void record_collection_cause(GCCause::Cause cause);
 
   void record_shutdown();
-  bool is_at_shutdown();
+  bool is_at_shutdown() const;
 
-  ShenandoahTracer* tracer() {return _tracer;}
+  ShenandoahTracer* tracer() const {return _tracer;}
 
   void print_gc_stats(outputStream* out) const;
 
@@ -90,15 +90,18 @@ class ShenandoahCollectorPolicy : public CHeapObj<mtGC> {
   // If the heuristics find that the number of consecutive degenerated cycles is above
   // ShenandoahFullGCThreshold, then they will initiate a Full GC upon an allocation
   // failure.
-  inline size_t consecutive_degenerated_gc_count() const {
+  size_t consecutive_degenerated_gc_count() const {
     return _consecutive_degenerated_gcs;
   }
 
+  static bool is_allocation_failure(GCCause::Cause cause);
+  static bool is_shenandoah_gc(GCCause::Cause cause);
   static bool is_requested_gc(GCCause::Cause cause);
+  static bool is_explicit_gc(GCCause::Cause cause);
   static bool should_run_full_gc(GCCause::Cause cause);
   static bool should_handle_requested_gc(GCCause::Cause cause);
 
-  inline size_t consecutive_young_gc_count() const {
+  size_t consecutive_young_gc_count() const {
     return _consecutive_young_gcs;
   }
 

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

@@ -51,7 +51,6 @@ ShenandoahControlThread::ShenandoahControlThread() :
 
 void ShenandoahControlThread::run_service() {
   ShenandoahHeap* const heap = ShenandoahHeap::heap();
-
   const GCMode default_mode = concurrent_normal;
   const GCCause::Cause default_cause = GCCause::_shenandoah_concurrent_gc;
   int sleep = ShenandoahControlIntervalMin;
@@ -60,9 +59,14 @@ void ShenandoahControlThread::run_service() {
 
   ShenandoahCollectorPolicy* const policy = heap->shenandoah_policy();
   ShenandoahHeuristics* const heuristics = heap->heuristics();
-  while (!in_graceful_shutdown() && !should_terminate()) {
+  while (!should_terminate()) {
+    const GCCause::Cause cancelled_cause = heap->cancelled_cause();
+    if (cancelled_cause == GCCause::_shenandoah_stop_vm) {
+      break;
+    }
+
     // Figure out if we have pending requests.
-    const bool alloc_failure_pending = _alloc_failure_gc.is_set();
+    const bool alloc_failure_pending = ShenandoahCollectorPolicy::is_allocation_failure(cancelled_cause);
     const bool is_gc_requested = _gc_requested.is_set();
     const GCCause::Cause requested_gc_cause = _requested_gc_cause;
 
@@ -164,8 +168,8 @@ void ShenandoahControlThread::run_service() {
         notify_gc_waiters();
       }
 
-      // If this was the allocation failure GC cycle, notify waiters about it
-      if (alloc_failure_pending) {
+      // If this cycle completed without being cancelled, notify waiters about it
+      if (!heap->cancelled_gc()) {
         notify_alloc_failure_waiters();
       }
 
@@ -255,11 +259,6 @@ void ShenandoahControlThread::run_service() {
     }
     os::naked_short_sleep(sleep);
   }
-
-  // Wait for the actual stop(), can't leave run_service() earlier.
-  while (!should_terminate()) {
-    os::naked_short_sleep(ShenandoahControlIntervalMin);
-  }
 }
 
 void ShenandoahControlThread::service_concurrent_normal_cycle(GCCause::Cause cause) {
@@ -323,19 +322,24 @@ void ShenandoahControlThread::service_concurrent_normal_cycle(GCCause::Cause cau
 bool ShenandoahControlThread::check_cancellation_or_degen(ShenandoahGC::ShenandoahDegenPoint point) {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   if (heap->cancelled_gc()) {
-    assert (is_alloc_failure_gc() || in_graceful_shutdown(), "Cancel GC either for alloc failure GC, or gracefully exiting");
-    if (!in_graceful_shutdown()) {
+    if (heap->cancelled_cause() == GCCause::_shenandoah_stop_vm) {
+      return true;
+    }
+
+    if (ShenandoahCollectorPolicy::is_allocation_failure(heap->cancelled_cause())) {
       assert (_degen_point == ShenandoahGC::_degenerated_outside_cycle,
               "Should not be set yet: %s", ShenandoahGC::degen_point_to_string(_degen_point));
       _degen_point = point;
+      return true;
     }
-    return true;
+
+    fatal("Unexpected reason for cancellation: %s", GCCause::to_string(heap->cancelled_cause()));
   }
   return false;
 }
 
 void ShenandoahControlThread::stop_service() {
-  // Nothing to do here.
+  ShenandoahHeap::heap()->cancel_gc(GCCause::_shenandoah_stop_vm);
 }
 
 void ShenandoahControlThread::service_stw_full_cycle(GCCause::Cause cause) {
@@ -364,6 +368,11 @@ void ShenandoahControlThread::request_gc(GCCause::Cause cause) {
 }
 
 void ShenandoahControlThread::handle_requested_gc(GCCause::Cause cause) {
+  if (should_terminate()) {
+    log_info(gc)("Control thread is terminating, no more GCs");
+    return;
+  }
+
   // For normal requested GCs (System.gc) we want to block the caller. However,
   // for whitebox requested GC, we want to initiate the GC and return immediately.
   // The whitebox caller thread will arrange for itself to wait until the GC notifies
@@ -386,7 +395,7 @@ void ShenandoahControlThread::handle_requested_gc(GCCause::Cause cause) {
   MonitorLocker ml(&_gc_waiters_lock);
   size_t current_gc_id = get_gc_id();
   size_t required_gc_id = current_gc_id + 1;
-  while (current_gc_id < required_gc_id) {
+  while (current_gc_id < required_gc_id && !should_terminate()) {
     // Although setting gc request is under _gc_waiters_lock, but read side (run_service())
     // does not take the lock. We need to enforce following order, so that read side sees
     // latest requested gc cause when the flag is set.

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

@@ -24,6 +24,7 @@
 #include "precompiled.hpp"
 
 #include "gc/shared/gc_globals.hpp"
+#include "gc/shenandoah/shenandoahCollectorPolicy.hpp"
 #include "gc/shenandoah/shenandoahController.hpp"
 #include "gc/shenandoah/shenandoahHeap.hpp"
 #include "gc/shenandoah/shenandoahHeapRegion.inline.hpp"
@@ -37,14 +38,6 @@ size_t ShenandoahController::reset_allocs_seen() {
   return Atomic::xchg(&_allocs_seen, (size_t)0, memory_order_relaxed);
 }
 
-void ShenandoahController::prepare_for_graceful_shutdown() {
-  _graceful_shutdown.set();
-}
-
-bool ShenandoahController::in_graceful_shutdown() {
-  return _graceful_shutdown.is_set();
-}
-
 void ShenandoahController::update_gc_id() {
   Atomic::inc(&_gc_id);
 }
@@ -53,60 +46,39 @@ size_t ShenandoahController::get_gc_id() {
   return Atomic::load(&_gc_id);
 }
 
-void ShenandoahController::handle_alloc_failure(ShenandoahAllocRequest& req, bool block) {
-  ShenandoahHeap* heap = ShenandoahHeap::heap();
-
+void ShenandoahController::handle_alloc_failure(const ShenandoahAllocRequest& req, bool block) {
   assert(current()->is_Java_thread(), "expect Java thread here");
-  bool is_humongous = ShenandoahHeapRegion::requires_humongous(req.size());
 
-  if (try_set_alloc_failure_gc(is_humongous)) {
-    // Only report the first allocation failure
-    log_info(gc)("Failed to allocate %s, " SIZE_FORMAT "%s",
-                 req.type_string(),
-                 byte_size_in_proper_unit(req.size() * HeapWordSize), proper_unit_for_byte_size(req.size() * HeapWordSize));
+  const bool is_humongous = ShenandoahHeapRegion::requires_humongous(req.size());
+  const GCCause::Cause cause = is_humongous ? GCCause::_shenandoah_humongous_allocation_failure : GCCause::_allocation_failure;
 
-    // Now that alloc failure GC is scheduled, we can abort everything else
-    heap->cancel_gc(GCCause::_allocation_failure);
+  ShenandoahHeap* const heap = ShenandoahHeap::heap();
+  if (heap->cancel_gc(cause)) {
+    log_info(gc)("Failed to allocate %s, " PROPERFMT, req.type_string(), PROPERFMTARGS(req.size() * HeapWordSize));
+    request_gc(cause);
   }
 
-
   if (block) {
     MonitorLocker ml(&_alloc_failure_waiters_lock);
-    while (is_alloc_failure_gc()) {
+    while (!should_terminate() && ShenandoahCollectorPolicy::is_allocation_failure(heap->cancelled_cause())) {
       ml.wait();
     }
   }
 }
 
 void ShenandoahController::handle_alloc_failure_evac(size_t words) {
-  ShenandoahHeap* heap = ShenandoahHeap::heap();
-  bool is_humongous = ShenandoahHeapRegion::requires_humongous(words);
 
-  if (try_set_alloc_failure_gc(is_humongous)) {
-    // Only report the first allocation failure
-    log_info(gc)("Failed to allocate " SIZE_FORMAT "%s for evacuation",
-                 byte_size_in_proper_unit(words * HeapWordSize), proper_unit_for_byte_size(words * HeapWordSize));
-  }
+  ShenandoahHeap* const heap = ShenandoahHeap::heap();
+  const bool is_humongous = ShenandoahHeapRegion::requires_humongous(words);
+  const GCCause::Cause cause = is_humongous ? GCCause::_shenandoah_humongous_allocation_failure : GCCause::_shenandoah_allocation_failure_evac;
 
-  // Forcefully report allocation failure
-  heap->cancel_gc(GCCause::_shenandoah_allocation_failure_evac);
+  if (heap->cancel_gc(cause)) {
+    log_info(gc)("Failed to allocate " PROPERFMT " for evacuation", PROPERFMTARGS(words * HeapWordSize));
+  }
 }
 
 void ShenandoahController::notify_alloc_failure_waiters() {
-  _alloc_failure_gc.unset();
-  _humongous_alloc_failure_gc.unset();
   MonitorLocker ml(&_alloc_failure_waiters_lock);
   ml.notify_all();
 }
 
-bool ShenandoahController::try_set_alloc_failure_gc(bool is_humongous) {
-  if (is_humongous) {
-    _humongous_alloc_failure_gc.try_set();
-  }
-  return _alloc_failure_gc.try_set();
-}
-
-bool ShenandoahController::is_alloc_failure_gc() {
-  return _alloc_failure_gc.is_set();
-}
-