8350621: Code cache stops scheduling GC

Co-authored-by: Thomas Schatzl <[email protected]>
Co-authored-by: Alexandre Jacob <[email protected]>
Reviewed-by: kbarrett, ayang

Author: Thomas Schatzl

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

@@ -1732,6 +1732,66 @@ static bool gc_counter_less_than(uint x, uint y) {
 #define LOG_COLLECT_CONCURRENTLY_COMPLETE(cause, result) \
   LOG_COLLECT_CONCURRENTLY(cause, "complete %s", BOOL_TO_STR(result))
 
+bool G1CollectedHeap::wait_full_mark_finished(GCCause::Cause cause,
+                                              uint old_marking_started_before,
+                                              uint old_marking_started_after,
+                                              uint old_marking_completed_after) {
+  // Request is finished if a full collection (concurrent or stw)
+  // was started after this request and has completed, e.g.
+  // started_before < completed_after.
+  if (gc_counter_less_than(old_marking_started_before,
+                           old_marking_completed_after)) {
+    LOG_COLLECT_CONCURRENTLY_COMPLETE(cause, true);
+    return true;
+  }
+
+  if (old_marking_started_after != old_marking_completed_after) {
+    // If there is an in-progress cycle (possibly started by us), then
+    // wait for that cycle to complete, e.g.
+    // while completed_now < started_after.
+    LOG_COLLECT_CONCURRENTLY(cause, "wait");
+    MonitorLocker ml(G1OldGCCount_lock);
+    while (gc_counter_less_than(_old_marking_cycles_completed,
+                                old_marking_started_after)) {
+      ml.wait();
+    }
+    // Request is finished if the collection we just waited for was
+    // started after this request.
+    if (old_marking_started_before != old_marking_started_after) {
+      LOG_COLLECT_CONCURRENTLY(cause, "complete after wait");
+      return true;
+    }
+  }
+  return false;
+}
+
+// After calling wait_full_mark_finished(), this method determines whether we
+// previously failed for ordinary reasons (concurrent cycle in progress, whitebox
+// has control). Returns if this has been such an ordinary reason.
+static bool should_retry_vm_op(GCCause::Cause cause,
+                               VM_G1TryInitiateConcMark* op) {
+  if (op->cycle_already_in_progress()) {
+    // If VMOp failed because a cycle was already in progress, it
+    // is now complete.  But it didn't finish this user-requested
+    // GC, so try again.
+    LOG_COLLECT_CONCURRENTLY(cause, "retry after in-progress");
+    return true;
+  } else if (op->whitebox_attached()) {
+    // If WhiteBox wants control, wait for notification of a state
+    // change in the controller, then try again.  Don't wait for
+    // release of control, since collections may complete while in
+    // control.  Note: This won't recognize a STW full collection
+    // while waiting; we can't wait on multiple monitors.
+    LOG_COLLECT_CONCURRENTLY(cause, "whitebox control stall");
+    MonitorLocker ml(ConcurrentGCBreakpoints::monitor());
+    if (ConcurrentGCBreakpoints::is_controlled()) {
+      ml.wait();
+    }
+    return true;
+  }
+  return false;
+}
+
 bool G1CollectedHeap::try_collect_concurrently(GCCause::Cause cause,
                                                uint gc_counter,
                                                uint old_marking_started_before) {
@@ -1792,7 +1852,45 @@ bool G1CollectedHeap::try_collect_concurrently(GCCause::Cause cause,
         LOG_COLLECT_CONCURRENTLY(cause, "ignoring STW full GC");
         old_marking_started_before = old_marking_started_after;
       }
+    } else if (GCCause::is_codecache_requested_gc(cause)) {
+      // For a CodeCache requested GC, before marking, progress is ensured as the
+      // following Remark pause unloads code (and signals the requester such).
+      // Otherwise we must ensure that it is restarted.
+      //
+      // For a CodeCache requested GC, a successful GC operation means that
+      // (1) marking is in progress. I.e. the VMOp started the marking or a
+      //     Remark pause is pending from a different VM op; we will potentially
+      //     abort a mixed phase if needed.
+      // (2) a new cycle was started (by this thread or some other), or
+      // (3) a Full GC was performed.
+      //
+      // Cases (2) and (3) are detected together by a change to
+      // _old_marking_cycles_started.
+      //
+      // Compared to other "automatic" GCs (see below), we do not consider being
+      // in whitebox as sufficient too because we might be anywhere within that
+      // cycle and we need to make progress.
+      if (op.mark_in_progress() ||
+          (old_marking_started_before != old_marking_started_after)) {
+        LOG_COLLECT_CONCURRENTLY_COMPLETE(cause, true);
+        return true;
+      }
+
+      if (wait_full_mark_finished(cause,
+                                  old_marking_started_before,
+                                  old_marking_started_after,
+                                  old_marking_completed_after)) {
+        return true;
+      }
+
+      if (should_retry_vm_op(cause, &op)) {
+        continue;
+      }
     } else if (!GCCause::is_user_requested_gc(cause)) {
+      assert(cause == GCCause::_g1_humongous_allocation ||
+             cause == GCCause::_g1_periodic_collection,
+             "Unsupported cause %s", GCCause::to_string(cause));
+
       // For an "automatic" (not user-requested) collection, we just need to
       // ensure that progress is made.
       //
@@ -1804,11 +1902,6 @@ bool G1CollectedHeap::try_collect_concurrently(GCCause::Cause cause,
       // (5) a Full GC was performed.
       // Cases (4) and (5) are detected together by a change to
       // _old_marking_cycles_started.
-      //
-      // Note that (1) does not imply (4).  If we're still in the mixed
-      // phase of an earlier concurrent collection, the request to make the
-      // collection a concurrent start won't be honored.  If we don't check for
-      // both conditions we'll spin doing back-to-back collections.
       if (op.gc_succeeded() ||
           op.cycle_already_in_progress() ||
           op.whitebox_attached() ||
@@ -1832,56 +1925,20 @@ bool G1CollectedHeap::try_collect_concurrently(GCCause::Cause cause,
              BOOL_TO_STR(op.gc_succeeded()),
              old_marking_started_before, old_marking_started_after);
 
-      // Request is finished if a full collection (concurrent or stw)
-      // was started after this request and has completed, e.g.
-      // started_before < completed_after.
-      if (gc_counter_less_than(old_marking_started_before,
-                               old_marking_completed_after)) {
-        LOG_COLLECT_CONCURRENTLY_COMPLETE(cause, true);
+      if (wait_full_mark_finished(cause,
+                                  old_marking_started_before,
+                                  old_marking_started_after,
+                                  old_marking_completed_after)) {
         return true;
       }
 
-      if (old_marking_started_after != old_marking_completed_after) {
-        // If there is an in-progress cycle (possibly started by us), then
-        // wait for that cycle to complete, e.g.
-        // while completed_now < started_after.
-        LOG_COLLECT_CONCURRENTLY(cause, "wait");
-        MonitorLocker ml(G1OldGCCount_lock);
-        while (gc_counter_less_than(_old_marking_cycles_completed,
-                                    old_marking_started_after)) {
-          ml.wait();
-        }
-        // Request is finished if the collection we just waited for was
-        // started after this request.
-        if (old_marking_started_before != old_marking_started_after) {
-          LOG_COLLECT_CONCURRENTLY(cause, "complete after wait");
-          return true;
-        }
-      }
-
       // If VMOp was successful then it started a new cycle that the above
       // wait &etc should have recognized as finishing this request.  This
       // differs from a non-user-request, where gc_succeeded does not imply
       // a new cycle was started.
       assert(!op.gc_succeeded(), "invariant");
 
-      if (op.cycle_already_in_progress()) {
-        // If VMOp failed because a cycle was already in progress, it
-        // is now complete.  But it didn't finish this user-requested
-        // GC, so try again.
-        LOG_COLLECT_CONCURRENTLY(cause, "retry after in-progress");
-        continue;
-      } else if (op.whitebox_attached()) {
-        // If WhiteBox wants control, wait for notification of a state
-        // change in the controller, then try again.  Don't wait for
-        // release of control, since collections may complete while in
-        // control.  Note: This won't recognize a STW full collection
-        // while waiting; we can't wait on multiple monitors.
-        LOG_COLLECT_CONCURRENTLY(cause, "whitebox control stall");
-        MonitorLocker ml(ConcurrentGCBreakpoints::monitor());
-        if (ConcurrentGCBreakpoints::is_controlled()) {
-          ml.wait();
-        }
+      if (should_retry_vm_op(cause, &op)) {
         continue;
       }
     }

src/hotspot/share/gc/g1/g1CollectedHeap.hpp

@@ -274,6 +274,14 @@ class G1CollectedHeap : public CollectedHeap {
   // (e) cause == _g1_periodic_collection and +G1PeriodicGCInvokesConcurrent.
   bool should_do_concurrent_full_gc(GCCause::Cause cause);
 
+  // Wait until a full mark (either currently in progress or one that completed
+  // after the current request) has finished. Returns whether that full mark started
+  // after this request. If so, we typically do not need another one.
+  bool wait_full_mark_finished(GCCause::Cause cause,
+                               uint old_marking_started_before,
+                               uint old_marking_started_after,
+                               uint old_marking_completed_after);
+
   // Attempt to start a concurrent cycle with the indicated cause.
   // precondition: should_do_concurrent_full_gc(cause)
   bool try_collect_concurrently(GCCause::Cause cause,

src/hotspot/share/gc/g1/g1CollectorState.hpp

@@ -60,6 +60,9 @@ class G1CollectorState {
   // do the concurrent start phase work.
   volatile bool _initiate_conc_mark_if_possible;
 
+  // Marking is in progress. Set from start of the concurrent start pause to the
+  // end of the Remark pause.
+  bool _mark_in_progress;
   // Marking or rebuilding remembered set work is in progress. Set from the end
   // of the concurrent start pause to the end of the Cleanup pause.
   bool _mark_or_rebuild_in_progress;
@@ -78,6 +81,7 @@ class G1CollectorState {
     _in_concurrent_start_gc(false),
     _initiate_conc_mark_if_possible(false),
 
+    _mark_in_progress(false),
     _mark_or_rebuild_in_progress(false),
     _clear_bitmap_in_progress(false),
     _in_full_gc(false) { }
@@ -92,6 +96,7 @@ class G1CollectorState {
 
   void set_initiate_conc_mark_if_possible(bool v) { _initiate_conc_mark_if_possible = v; }
 
+  void set_mark_in_progress(bool v) { _mark_in_progress = v; }
   void set_mark_or_rebuild_in_progress(bool v) { _mark_or_rebuild_in_progress = v; }
   void set_clear_bitmap_in_progress(bool v) { _clear_bitmap_in_progress = v; }
 
@@ -106,6 +111,7 @@ class G1CollectorState {
 
   bool initiate_conc_mark_if_possible() const { return _initiate_conc_mark_if_possible; }
 
+  bool mark_in_progress() const { return _mark_in_progress; }
   bool mark_or_rebuild_in_progress() const { return _mark_or_rebuild_in_progress; }
   bool clear_bitmap_in_progress() const { return _clear_bitmap_in_progress; }
 

src/hotspot/share/gc/g1/g1Policy.cpp

@@ -591,6 +591,7 @@ void G1Policy::record_full_collection_end() {
   collector_state()->set_in_young_gc_before_mixed(false);
   collector_state()->set_initiate_conc_mark_if_possible(need_to_start_conc_mark("end of Full GC"));
   collector_state()->set_in_concurrent_start_gc(false);
+  collector_state()->set_mark_in_progress(false);
   collector_state()->set_mark_or_rebuild_in_progress(false);
   collector_state()->set_clear_bitmap_in_progress(false);
 
@@ -703,6 +704,7 @@ void G1Policy::record_concurrent_mark_remark_end() {
   double elapsed_time_ms = (end_time_sec - start_time_sec) * 1000.0;
   _analytics->report_concurrent_mark_remark_times_ms(elapsed_time_ms);
   record_pause(G1GCPauseType::Remark, start_time_sec, end_time_sec);
+  collector_state()->set_mark_in_progress(false);
 }
 
 G1CollectionSetCandidates* G1Policy::candidates() const {
@@ -936,6 +938,7 @@ void G1Policy::record_young_collection_end(bool concurrent_operation_is_full_mar
   assert(!(G1GCPauseTypeHelper::is_concurrent_start_pause(this_pause) && collector_state()->mark_or_rebuild_in_progress()),
          "If the last pause has been concurrent start, we should not have been in the marking window");
   if (G1GCPauseTypeHelper::is_concurrent_start_pause(this_pause)) {
+    collector_state()->set_mark_in_progress(concurrent_operation_is_full_mark);
     collector_state()->set_mark_or_rebuild_in_progress(concurrent_operation_is_full_mark);
   }
 
@@ -1222,6 +1225,17 @@ void G1Policy::initiate_conc_mark() {
   collector_state()->set_initiate_conc_mark_if_possible(false);
 }
 
+static const char* requester_for_mixed_abort(GCCause::Cause cause) {
+  if (cause == GCCause::_wb_breakpoint) {
+    return "run_to breakpoint";
+  } else if (GCCause::is_codecache_requested_gc(cause)) {
+    return "codecache";
+  } else {
+    assert(G1CollectedHeap::heap()->is_user_requested_concurrent_full_gc(cause), "must be");
+    return "user";
+  }
+}
+
 void G1Policy::decide_on_concurrent_start_pause() {
   // We are about to decide on whether this pause will be a
   // concurrent start pause.
@@ -1254,8 +1268,7 @@ void G1Policy::decide_on_concurrent_start_pause() {
       initiate_conc_mark();
       log_debug(gc, ergo)("Initiate concurrent cycle (concurrent cycle initiation requested)");
     } else if (_g1h->is_user_requested_concurrent_full_gc(cause) ||
-               (cause == GCCause::_codecache_GC_threshold) ||
-               (cause == GCCause::_codecache_GC_aggressive) ||
+               GCCause::is_codecache_requested_gc(cause) ||
                (cause == GCCause::_wb_breakpoint)) {
       // Initiate a concurrent start.  A concurrent start must be a young only
       // GC, so the collector state must be updated to reflect this.
@@ -1270,7 +1283,7 @@ void G1Policy::decide_on_concurrent_start_pause() {
       abort_time_to_mixed_tracking();
       initiate_conc_mark();
       log_debug(gc, ergo)("Initiate concurrent cycle (%s requested concurrent cycle)",
-                          (cause == GCCause::_wb_breakpoint) ? "run_to breakpoint" : "user");
+                          requester_for_mixed_abort(cause));
     } else {
       // The concurrent marking thread is still finishing up the
       // previous cycle. If we start one right now the two cycles

src/hotspot/share/gc/g1/g1VMOperations.cpp

@@ -59,6 +59,7 @@ VM_G1TryInitiateConcMark::VM_G1TryInitiateConcMark(uint gc_count_before,
                                                    GCCause::Cause gc_cause) :
   VM_GC_Collect_Operation(gc_count_before, gc_cause),
   _transient_failure(false),
+  _mark_in_progress(false),
   _cycle_already_in_progress(false),
   _whitebox_attached(false),
   _terminating(false),
@@ -83,6 +84,9 @@ void VM_G1TryInitiateConcMark::doit() {
   // Record for handling by caller.
   _terminating = g1h->concurrent_mark_is_terminating();
 
+  _mark_in_progress = g1h->collector_state()->mark_in_progress();
+  _cycle_already_in_progress = g1h->concurrent_mark()->cm_thread()->in_progress();
+
   if (_terminating && GCCause::is_user_requested_gc(_gc_cause)) {
     // When terminating, the request to initiate a concurrent cycle will be
     // ignored by do_collection_pause_at_safepoint; instead it will just do
@@ -91,9 +95,8 @@ void VM_G1TryInitiateConcMark::doit() {
     // requests the alternative GC might still be needed.
   } else if (!g1h->policy()->force_concurrent_start_if_outside_cycle(_gc_cause)) {
     // Failure to force the next GC pause to be a concurrent start indicates
-    // there is already a concurrent marking cycle in progress.  Set flag
-    // to notify the caller and return immediately.
-    _cycle_already_in_progress = true;
+    // there is already a concurrent marking cycle in progress. Flags to indicate
+    // that were already set, so return immediately.
   } else if ((_gc_cause != GCCause::_wb_breakpoint) &&
              ConcurrentGCBreakpoints::is_controlled()) {
     // WhiteBox wants to be in control of concurrent cycles, so don't try to

src/hotspot/share/gc/g1/g1VMOperations.hpp

@@ -45,6 +45,7 @@ class VM_G1CollectFull : public VM_GC_Collect_Operation {
 
 class VM_G1TryInitiateConcMark : public VM_GC_Collect_Operation {
   bool _transient_failure;
+  bool _mark_in_progress;
   bool _cycle_already_in_progress;
   bool _whitebox_attached;
   bool _terminating;
@@ -59,6 +60,7 @@ class VM_G1TryInitiateConcMark : public VM_GC_Collect_Operation {
   virtual bool doit_prologue();
   virtual void doit();
   bool transient_failure() const { return _transient_failure; }
+  bool mark_in_progress() const { return _mark_in_progress; }
   bool cycle_already_in_progress() const { return _cycle_already_in_progress; }
   bool whitebox_attached() const { return _whitebox_attached; }
   bool terminating() const { return _terminating; }

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

@@ -104,6 +104,11 @@ class GCCause : public AllStatic {
             cause == GCCause::_heap_dump);
   }
 
+  inline static bool is_codecache_requested_gc(GCCause::Cause cause) {
+      return (cause == _codecache_GC_threshold  ||
+              cause == _codecache_GC_aggressive);
+  }
+
   // Causes for collection of the tenured generation
   inline static bool is_tenured_allocation_failure_gc(GCCause::Cause cause) {
     // _allocation_failure is the generic cause a collection which could result