# HG changeset patch
# User zgu
# Date 1571759982 14400
# Node ID ef2b7575083878e8aef515bbbc5728fc85ced3d4
# Parent  a39cdab8fac14ea13714235e847b97d06827c512
8232747: Shenandoah: Concurrent GC should deactivate SATB before processing weak roots
Reviewed-by: shade

diff -r a39cdab8fac1 -r ef2b75750838 src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp	Tue Oct 22 14:05:56 2019 +0200
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp	Tue Oct 22 11:59:42 2019 -0400
@@ -442,8 +442,6 @@
     weak_refs_work(full_gc);
   }
 
-  _heap->parallel_cleaning(full_gc);
-
   assert(task_queues()->is_empty(), "Should be empty");
   TASKQUEUE_STATS_ONLY(task_queues()->print_taskqueue_stats());
   TASKQUEUE_STATS_ONLY(task_queues()->reset_taskqueue_stats());
diff -r a39cdab8fac1 -r ef2b75750838 src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp	Tue Oct 22 14:05:56 2019 +0200
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp	Tue Oct 22 11:59:42 2019 -0400
@@ -1478,6 +1478,12 @@
   if (!cancelled_gc()) {
     concurrent_mark()->finish_mark_from_roots(/* full_gc = */ false);
 
+    // Marking is completed, deactivate SATB barrier
+    set_concurrent_mark_in_progress(false);
+    mark_complete_marking_context();
+
+    parallel_cleaning(false /* full gc*/);
+
     if (has_forwarded_objects()) {
       // Degen may be caused by failed evacuation of roots
       if (is_degenerated_gc_in_progress()) {
@@ -1485,14 +1491,12 @@
       } else {
         concurrent_mark()->update_thread_roots(ShenandoahPhaseTimings::update_roots);
       }
+      set_has_forwarded_objects(false);
    }
 
     if (ShenandoahVerify) {
       verifier()->verify_roots_no_forwarded();
     }
-
-    stop_concurrent_marking();
-
     // All allocations past TAMS are implicitly live, adjust the region data.
     // Bitmaps/TAMS are swapped at this point, so we need to poll complete bitmap.
     {
@@ -1575,8 +1579,10 @@
     }
 
   } else {
+    // If this cycle was updating references, we need to keep the has_forwarded_objects
+    // flag on, for subsequent phases to deal with it.
     concurrent_mark()->cancel();
-    stop_concurrent_marking();
+    set_concurrent_mark_in_progress(false);
 
     if (process_references()) {
       // Abandon reference processing right away: pre-cleaning must have failed.
@@ -1872,17 +1878,6 @@
   op_full(GCCause::_shenandoah_upgrade_to_full_gc);
 }
 
-void ShenandoahHeap::stop_concurrent_marking() {
-  assert(is_concurrent_mark_in_progress(), "How else could we get here?");
-  set_concurrent_mark_in_progress(false);
-  if (!cancelled_gc()) {
-    // If we needed to update refs, and concurrent marking has been cancelled,
-    // we need to finish updating references.
-    set_has_forwarded_objects(false);
-    mark_complete_marking_context();
-  }
-}
-
 void ShenandoahHeap::force_satb_flush_all_threads() {
   if (!is_concurrent_mark_in_progress() && !is_concurrent_traversal_in_progress()) {
     // No need to flush SATBs
diff -r a39cdab8fac1 -r ef2b75750838 src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp	Tue Oct 22 14:05:56 2019 +0200
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp	Tue Oct 22 11:59:42 2019 -0400
@@ -717,8 +717,6 @@
 
   void deduplicate_string(oop str);
 
-  void stop_concurrent_marking();
-
 private:
   void trash_cset_regions();
   void update_heap_references(bool concurrent);
diff -r a39cdab8fac1 -r ef2b75750838 src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp	Tue Oct 22 14:05:56 2019 +0200
+++ b/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp	Tue Oct 22 11:59:42 2019 -0400
@@ -110,7 +110,7 @@
     // b. Cancel concurrent mark, if in progress
     if (heap->is_concurrent_mark_in_progress()) {
       heap->concurrent_mark()->cancel();
-      heap->stop_concurrent_marking();
+      heap->set_concurrent_mark_in_progress(false);
     }
     assert(!heap->is_concurrent_mark_in_progress(), "sanity");
 
@@ -243,8 +243,8 @@
   cm->update_roots(ShenandoahPhaseTimings::full_gc_roots);
   cm->mark_roots(ShenandoahPhaseTimings::full_gc_roots);
   cm->finish_mark_from_roots(/* full_gc = */ true);
-
   heap->mark_complete_marking_context();
+  heap->parallel_cleaning(true /* full_gc */);
 }
 
 class ShenandoahPrepareForCompactionObjectClosure : public ObjectClosure {