Allow old collections to span degenerated young collections

Reviewed-by: kdnilsen

Author: William Kemper

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

@@ -309,14 +309,6 @@ void ShenandoahConcurrentGC::entry_init_mark() {
                               ShenandoahWorkerPolicy::calc_workers_for_init_marking(),
                               "init marking");
 
-  if (ShenandoahHeap::heap()->mode()->is_generational()
-    && (_generation->generation_mode() == YOUNG || (_generation->generation_mode() == GLOBAL && ShenandoahVerify))) {
-    // The current implementation of swap_remembered_set() copies the write-card-table
-    // to the read-card-table. The remembered sets are also swapped for GLOBAL collections
-    // so that the verifier works with the correct copy of the card table when verifying.
-    _generation->swap_remembered_set();
-  }
-
   op_init_mark();
 }
 
@@ -607,6 +599,25 @@ void ShenandoahConcurrentGC::op_init_mark() {
   assert(!_generation->is_mark_complete(), "should not be complete");
   assert(!heap->has_forwarded_objects(), "No forwarded objects on this path");
 
+
+  if (heap->mode()->is_generational()) {
+      if (_generation->generation_mode() == YOUNG || (_generation->generation_mode() == GLOBAL && ShenandoahVerify)) {
+      // The current implementation of swap_remembered_set() copies the write-card-table
+      // to the read-card-table. The remembered sets are also swapped for GLOBAL collections
+      // so that the verifier works with the correct copy of the card table when verifying.
+      _generation->swap_remembered_set();
+    }
+
+    if (_generation->generation_mode() == GLOBAL) {
+      heap->cancel_old_gc();
+    } else if (heap->is_concurrent_old_mark_in_progress()) {
+      // Purge the SATB buffers, transferring any valid, old pointers to the
+      // old generation mark queue. Any pointers in a young region will be
+      // abandoned.
+      heap->transfer_old_pointers_from_satb();
+    }
+  }
+
   if (ShenandoahVerify) {
     heap->verifier()->verify_before_concmark();
   }
@@ -1131,13 +1142,6 @@ void ShenandoahConcurrentGC::op_final_updaterefs() {
 
   heap->update_heap_region_states(true /*concurrent*/);
 
-  if (heap->is_concurrent_old_mark_in_progress()) {
-    // Purge the SATB buffers, transferring any valid, old pointers to the
-    // old generation mark queue. From here on, no mutator will have access
-    // to anything that will be trashed and recycled.
-    heap->purge_old_satb_buffers(false /* abandon */);
-  }
-
   heap->set_update_refs_in_progress(false);
   heap->set_has_forwarded_objects(false);
 

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

@@ -467,7 +467,11 @@ void ShenandoahControlThread::service_concurrent_old_cycle(const ShenandoahHeap*
   young_generation->set_mark_incomplete();
   old_generation->set_mark_incomplete();
   service_concurrent_cycle(young_generation, cause, true);
-  if (!heap->cancelled_gc()) {
+  if (heap->cancelled_gc()) {
+    // Young generation bootstrap cycle has failed. Concurrent mark for old generation
+    // is not going to resume after degenerated young cycle completes.
+    log_info(gc)("Bootstrap cycle for old generation was cancelled.");
+  } else {
     // Reset the degenerated point. Normally this would happen at the top
     // of the control loop, but here we have just completed a young cycle
     // which has bootstrapped the old concurrent marking.
@@ -663,15 +667,13 @@ bool ShenandoahControlThread::service_stw_degenerated_cycle(GCCause::Cause cause
   ShenandoahGCSession session(cause, _degen_generation);
 
   ShenandoahDegenGC gc(point, _degen_generation);
-
-  // Just in case degenerated cycle preempted old-gen marking, clear the old-gen task queues.
-  heap->young_generation()->set_old_gen_task_queues(NULL);
-
   gc.collect(cause);
 
   assert(heap->young_generation()->task_queues()->is_empty(), "Unexpected young generation marking tasks");
-  assert(heap->old_generation()->task_queues()->is_empty(), "Unexpected old generation marking tasks");
-  assert(heap->global_generation()->task_queues()->is_empty(), "Unexpected global generation marking tasks");
+  if (_degen_generation->generation_mode() == GLOBAL) {
+    assert(heap->old_generation()->task_queues()->is_empty(), "Unexpected old generation marking tasks");
+    assert(heap->global_generation()->task_queues()->is_empty(), "Unexpected global generation marking tasks");
+  }
 
   _degen_generation->heuristics()->record_success_degenerated();
   heap->shenandoah_policy()->record_success_degenerated();

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

@@ -90,13 +90,21 @@ void ShenandoahDegenGC::op_degenerated() {
   // some phase, we have to upgrade the Degenerate GC to Full GC.
   heap->clear_cancelled_gc(true /* clear oom handler */);
 
-  // We can't easily clear the old mark in progress flag because it must be done
-  // on a safepoint (not sure if that is a hard requirement). At any rate, once
-  // we are in a degenerated cycle, there should be no more old marking.
-  if (heap->is_concurrent_old_mark_in_progress()) {
-    heap->old_generation()->cancel_marking();
+#ifdef ASSERT
+  if (heap->mode()->is_generational()) {
+    if (_generation->generation_mode() == GenerationMode::GLOBAL) {
+      // We can only get to a degenerated global cycle _after_ a concurrent global cycle
+      // has been cancelled. In which case, we expect the concurrent global cycle to have
+      // cancelled the old gc already.
+      assert(!heap->is_old_gc_active(), "Old GC should not be active during global cycle.");
+    }
+
+    if (!heap->is_concurrent_old_mark_in_progress()) {
+      // If we are not marking the old generation, there should be nothing in the old mark queues
+      assert(heap->old_generation()->task_queues()->is_empty(), "Old gen task queues should be empty.");
+    }
   }
-  assert(heap->old_generation()->task_queues()->is_empty(), "Old gen task queues should be empty.");
+#endif
 
   ShenandoahMetricsSnapshot metrics;
   metrics.snap_before();
@@ -113,6 +121,14 @@ void ShenandoahDegenGC::op_degenerated() {
       // we can do the most aggressive degen cycle, which includes processing references and
       // class unloading, unless those features are explicitly disabled.
 
+      if (heap->is_concurrent_old_mark_in_progress()) {
+        // We have come straight into a degenerated cycle without running a concurrent cycle
+        // first and the SATB barrier is enabled to support concurrent old marking. The SATB buffer
+        // may hold a mix of old and young pointers. The old pointers need to be transferred
+        // to the old generation mark queues and the young pointers are _not_ part of this
+        // snapshot, so they must be dropped here.
+        heap->transfer_old_pointers_from_satb();
+      }
 
       // Note that we can only do this for "outside-cycle" degens, otherwise we would risk
       // changing the cycle parameters mid-cycle during concurrent -> degenerated handover.
@@ -125,8 +141,17 @@ void ShenandoahDegenGC::op_degenerated() {
 
     case _degenerated_roots:
       // Degenerated from concurrent root mark, reset the flag for STW mark
-      if (heap->is_concurrent_mark_in_progress()) {
-        heap->cancel_concurrent_mark();
+      if (!heap->mode()->is_generational()) {
+        if (heap->is_concurrent_mark_in_progress()) {
+          heap->cancel_concurrent_mark();
+        }
+      } else {
+        if (_generation->is_concurrent_mark_in_progress()) {
+          // We want to allow old generation marking to be punctuated by young collections
+          // (even if they have degenerated). If this is a global cycle, we'd have cancelled
+          // the entire old gc before coming into this switch.
+          _generation->cancel_marking();
+        }
       }
 
       if (_degen_point == ShenandoahDegenPoint::_degenerated_roots) {
@@ -293,7 +318,7 @@ void ShenandoahDegenGC::op_reset() {
 }
 
 void ShenandoahDegenGC::op_mark() {
-  assert(!ShenandoahHeap::heap()->is_concurrent_mark_in_progress(), "Should be reset");
+  assert(!_generation->is_concurrent_mark_in_progress(), "Should be reset");
   ShenandoahGCPhase phase(ShenandoahPhaseTimings::degen_gc_stw_mark);
   ShenandoahSTWMark mark(_generation, false /*full gc*/);
   mark.mark();

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

@@ -194,6 +194,11 @@ void ShenandoahFullGC::do_it(GCCause::Cause gc_cause) {
   heap->reset_old_evac_expended();
   heap->set_promotion_reserve(0);
 
+  if (heap->mode()->is_generational()) {
+    // Full GC supersedes any marking or coalescing in old generation.
+    heap->cancel_old_gc();
+  }
+
   if (ShenandoahVerify) {
     heap->verifier()->verify_before_fullgc();
   }

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

@@ -397,12 +397,11 @@ ShenandoahMarkingContext* ShenandoahGeneration::complete_marking_context() {
 
 void ShenandoahGeneration::cancel_marking() {
   if (is_concurrent_mark_in_progress()) {
-    set_concurrent_mark_in_progress(false);
+    set_mark_incomplete();
   }
-  set_mark_incomplete();
   _task_queues->clear();
-
   ref_processor()->abandon_partial_discovery();
+  set_concurrent_mark_in_progress(false);
 }
 
 ShenandoahGeneration::ShenandoahGeneration(GenerationMode generation_mode,

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

@@ -109,13 +109,13 @@ class ShenandoahGeneration : public CHeapObj<mtGC> {
   void merge_write_table();
 
   // Used by concurrent and degenerated GC to reset regions.
-  virtual void prepare_gc(bool do_old_gc_bootstrap);
+  void prepare_gc(bool do_old_gc_bootstrap);
 
   // Return true iff prepared collection set includes at least one old-gen HeapRegion.
   virtual bool prepare_regions_and_collection_set(bool concurrent);
 
   // Cancel marking (used by Full collect and when cancelling cycle).
-  void cancel_marking();
+  virtual void cancel_marking();
 
   // Return true if this region is affiliated with this generation.
   virtual bool contains(ShenandoahHeapRegion* region) const = 0;

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

@@ -58,14 +58,12 @@ size_t ShenandoahGlobalGeneration::available() const {
 
 void ShenandoahGlobalGeneration::set_concurrent_mark_in_progress(bool in_progress) {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
-  if (in_progress && heap->is_concurrent_old_mark_in_progress()) {
+  if (in_progress && heap->mode()->is_generational()) {
     // Global collection has preempted an old generation mark. This is fine
     // because the global generation includes the old generation, but we
     // want the global collect to start from a clean slate and we don't want
     // any stale state in the old generation.
-    heap->purge_old_satb_buffers(true /* abandon */);
-    heap->old_generation()->cancel_marking();
-    heap->young_generation()->set_old_gen_task_queues(nullptr);
+    heap->cancel_old_gc();
   }
 
   heap->set_concurrent_young_mark_in_progress(in_progress);
@@ -88,13 +86,3 @@ bool ShenandoahGlobalGeneration::is_concurrent_mark_in_progress() {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   return heap->is_concurrent_mark_in_progress();
 }
-
-void ShenandoahGlobalGeneration::prepare_gc(bool do_old_gc_bootstrap) {
-  ShenandoahGeneration::prepare_gc(do_old_gc_bootstrap);
-
-  ShenandoahHeap* heap = ShenandoahHeap::heap();
-  if (heap->mode()->is_generational()) {
-    heap->cancel_mixed_collections();
-  }
-}
-

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

@@ -42,8 +42,6 @@ class ShenandoahGlobalGeneration : public ShenandoahGeneration {
   virtual size_t used() const override;
   virtual size_t available() const override;
 
-  virtual void prepare_gc(bool do_old_gc_bootstrap) override;
-
   virtual void set_concurrent_mark_in_progress(bool in_progress)  override;
 
   bool contains(ShenandoahHeapRegion* region) const  override;

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

@@ -981,9 +981,26 @@ void ShenandoahHeap::retire_plab(PLAB* plab) {
   }
 }
 
-void ShenandoahHeap::cancel_mixed_collections() {
+void ShenandoahHeap::cancel_old_gc() {
+  shenandoah_assert_safepoint();
   assert(_old_generation != NULL, "Should only have mixed collections in generation mode.");
+  log_info(gc)("Terminating old gc cycle.");
+
+  // Stop marking
+  old_generation()->cancel_marking();
+  // Stop coalescing undead objects
+  set_concurrent_prep_for_mixed_evacuation_in_progress(false);
+  // Stop tracking old regions
   old_heuristics()->abandon_collection_candidates();
+  // Remove old generation access to young generation mark queues
+  young_generation()->set_old_gen_task_queues(nullptr);
+}
+
+bool ShenandoahHeap::is_old_gc_active() {
+  return is_concurrent_old_mark_in_progress()
+      || is_concurrent_prep_for_mixed_evacuation_in_progress()
+      || old_heuristics()->unprocessed_old_or_hidden_collection_candidates() > 0
+      || young_generation()->old_gen_task_queues() != nullptr;
 }
 
 void ShenandoahHeap::coalesce_and_fill_old_regions() {
@@ -2111,18 +2128,10 @@ bool ShenandoahHeap::try_cancel_gc() {
     assert(prev == NOT_CANCELLED, "must be NOT_CANCELLED");
     Thread* thread = Thread::current();
     if (thread->is_Java_thread()) {
-      JavaThread* java_thread = JavaThread::cast(thread);
-      if (java_thread->thread_state() == _thread_in_Java) {
-        // ThreadBlockInVM requires thread state to be _thread_in_vm.  If we are in Java, safely transition thread state.
-        ThreadInVMfromJava transition(java_thread);
-        // We need to provide a safepoint here.  Otherwise we might spin forever if a SP is pending.
-        ThreadBlockInVM sp(JavaThread::cast(thread));
-        SpinPause();
-      } else {
-        // We need to provide a safepoint here.  Otherwise we might spin forever if a SP is pending.
-        ThreadBlockInVM sp(JavaThread::cast(thread));
-        SpinPause();
-      }
+      // We need to provide a safepoint here, otherwise we might
+      // spin forever if a SP is pending.
+      ThreadBlockInVM sp(JavaThread::cast(thread));
+      SpinPause();
     }
   }
 }
@@ -2810,8 +2819,8 @@ void ShenandoahHeap::flush_liveness_cache(uint worker_id) {
   }
 }
 
-void ShenandoahHeap::purge_old_satb_buffers(bool abandon) {
-  ((ShenandoahOldGeneration*)_old_generation)->purge_satb_buffers(abandon);
+void ShenandoahHeap::transfer_old_pointers_from_satb() {
+  ((ShenandoahOldGeneration*) _old_generation)->transfer_pointers_from_satb();
 }
 
 template<>

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

@@ -810,7 +810,8 @@ class ShenandoahHeap : public CollectedHeap {
   void clear_cards(HeapWord* start, HeapWord* end);
   void mark_card_as_dirty(void* location);
   void retire_plab(PLAB* plab);
-  void cancel_mixed_collections();
+  void cancel_old_gc();
+  bool is_old_gc_active();
   void coalesce_and_fill_old_regions();
 
 // ---------- Helper functions
@@ -838,7 +839,7 @@ class ShenandoahHeap : public CollectedHeap {
 
   static inline void increase_object_age(oop obj, uint additional_age);
 
-  void purge_old_satb_buffers(bool abandon);
+  void transfer_old_pointers_from_satb();
 private:
   void trash_cset_regions();
 

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

@@ -825,7 +825,7 @@ void ShenandoahHeapRegion::set_affiliation(ShenandoahRegionAffiliation new_affil
   {
     ShenandoahMarkingContext* const ctx = heap->complete_marking_context();
     log_debug(gc)("Setting affiliation of Region " SIZE_FORMAT " from %s to %s, top: " PTR_FORMAT ", TAMS: " PTR_FORMAT
-                  ", watermark: " PTR_FORMAT ", top_bitmap: " PTR_FORMAT "\n",
+                  ", watermark: " PTR_FORMAT ", top_bitmap: " PTR_FORMAT,
                   index(), affiliation_name(_affiliation), affiliation_name(new_affiliation),
                   p2i(top()), p2i(ctx->top_at_mark_start(this)), p2i(_update_watermark), p2i(ctx->top_bitmap(this)));
   }

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

@@ -300,8 +300,10 @@ inline void ShenandoahMark::mark_through_ref(T *p, ShenandoahObjToScanQueue* q,
       // Old mark, found a young pointer.
       // TODO:  Rethink this: may be redundant with dirtying of cards identified during young-gen remembered set scanning
       // and by mutator write barriers.  Assert
-      assert(heap->is_in_young(obj), "Expected young object.");
-      heap->mark_card_as_dirty(p);
+      if (heap->is_in(p)) {
+        assert(heap->is_in_young(obj), "Expected young object.");
+        heap->mark_card_as_dirty(p);
+      }
     }
   }
 }

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

@@ -89,7 +89,7 @@ inline void ShenandoahMarkingContext::capture_top_at_mark_start(ShenandoahHeapRe
            "Region " SIZE_FORMAT ", bitmap should be clear while adjusting TAMS: " PTR_FORMAT " -> " PTR_FORMAT,
            idx, p2i(old_tams), p2i(new_tams));
 
-    log_debug(gc)("Capturing TAMS for %s Region " SIZE_FORMAT ", was: %llx, now: %llx\n",
+    log_debug(gc)("Capturing TAMS for %s Region " SIZE_FORMAT ", was: %llx, now: %llx",
                   affiliation_name(r->affiliation()), idx, (unsigned long long) old_tams, (unsigned long long) new_tams);
 
     if ((old_tams == r->bottom()) && (new_tams > old_tams)) {