# HG changeset patch
# User rkennke
# Date 1576099148 -3600
#      Wed Dec 11 22:19:08 2019 +0100
# Node ID aa7b71106064e1b05b92f53251dd8dc830679d7d
# Parent  0acd96e392e34eb7a7adc64067e5caec646e6ee3
8234974: Shenandoah: Do concurrent roots even when no evacuation is necessary

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp
@@ -264,13 +264,8 @@
   }
 
   ShenandoahMarkingContext* const marking_context = _heap->marking_context();
-  if (_heap->is_evacuation_in_progress() && !marking_context->is_marked(obj)) {
-    Thread* thr = Thread::current();
-    if (thr->is_Java_thread()) {
-      return NULL;
-    } else {
-      return obj;
-    }
+  if (_heap->is_concurrent_root_in_progress() && !marking_context->is_marked(obj)) {
+    return NULL;
   }
 
   oop fwd = load_reference_barrier_not_null(obj);
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSetNMethod.cpp b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSetNMethod.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSetNMethod.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSetNMethod.cpp
@@ -57,8 +57,10 @@
   }
 
   // Heal oops and disarm
-  ShenandoahEvacOOMScope scope;
-  ShenandoahNMethod::heal_nmethod(nm);
+  if (ShenandoahHeap::heap()->is_evacuation_in_progress()) {
+    ShenandoahEvacOOMScope scope;
+    ShenandoahNMethod::heal_nmethod(nm);
+  }
   ShenandoahNMethod::disarm_nmethod(nm);
   return true;
 }
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp b/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp
@@ -111,12 +111,13 @@
 
 template <class T>
 void ShenandoahEvacuateUpdateRootsClosure::do_oop_work(T* p) {
-  assert(_heap->is_evacuation_in_progress(), "Only do this when evacuation is in progress");
+  assert(_heap->is_concurrent_root_in_progress(), "Only do this when evacuation is in progress");
 
   T o = RawAccess<>::oop_load(p);
   if (! CompressedOops::is_null(o)) {
     oop obj = CompressedOops::decode_not_null(o);
     if (_heap->in_collection_set(obj)) {
+      assert(_heap->is_evacuation_in_progress(), "Only do this when evacuation is in progress");
       shenandoah_assert_marked(p, obj);
       oop resolved = ShenandoahBarrierSet::resolve_forwarded_not_null(obj);
       if (resolved == obj) {
@@ -139,11 +140,12 @@
 }
 
 void ShenandoahEvacUpdateOopStorageRootsClosure::do_oop(oop* p) {
-  assert(_heap->is_evacuation_in_progress(), "Only do this when evacuation is in progress");
+  assert(_heap->is_concurrent_root_in_progress(), "Only do this when evacuation is in progress");
 
   oop obj = RawAccess<>::oop_load(p);
   if (! CompressedOops::is_null(obj)) {
     if (_heap->in_collection_set(obj)) {
+      assert(_heap->is_evacuation_in_progress(), "Only do this when evacuation is in progress");
       shenandoah_assert_marked(p, obj);
       oop resolved = ShenandoahBarrierSet::resolve_forwarded_not_null(obj);
       if (resolved == obj) {
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp b/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp
@@ -224,7 +224,9 @@
 
     // Heal oops and disarm
     ShenandoahEvacOOMScope evac_scope;
-    ShenandoahNMethod::heal_nmethod(nm);
+    if (_heap->is_evacuation_in_progress()) {
+      ShenandoahNMethod::heal_nmethod(nm);
+    }
     ShenandoahNMethod::disarm_nmethod(nm);
 
     // Clear compiled ICs and exception caches
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
@@ -542,6 +542,7 @@
   if (is_degenerated_gc_in_progress())       st->print("degenerated gc, ");
   if (is_full_gc_in_progress())              st->print("full gc, ");
   if (is_full_gc_move_in_progress())         st->print("full gc move, ");
+  if (is_concurrent_root_in_progress())      st->print("concurrent roots, ");
 
   if (cancelled_gc()) {
     st->print("cancelled");
@@ -1540,6 +1541,11 @@
       _free_set->rebuild();
     }
 
+    if (!is_degenerated_gc_in_progress()) {
+      prepare_concurrent_roots();
+      prepare_concurrent_unloading();
+    }
+
     // If collection set has candidates, start evacuation.
     // Otherwise, bypass the rest of the cycle.
     if (!collection_set()->is_empty()) {
@@ -1554,8 +1560,6 @@
       set_has_forwarded_objects(true);
 
       if (!is_degenerated_gc_in_progress()) {
-        prepare_concurrent_roots();
-        prepare_concurrent_unloading();
         evacuate_and_update_roots();
       }
 
@@ -1669,12 +1673,12 @@
 };
 
 void ShenandoahHeap::op_roots() {
-  if (is_evacuation_in_progress()) {
+  if (is_concurrent_root_in_progress()) {
     if (ShenandoahConcurrentRoots::should_do_concurrent_class_unloading()) {
       _unloader.unload();
     }
 
-    if (ShenandoahConcurrentRoots::should_do_concurrent_roots()) {
+    if (ShenandoahConcurrentRoots::should_do_concurrent_roots() && is_concurrent_root_in_progress()) {
       ShenandoahConcurrentRootsEvacUpdateTask task;
       workers()->run_task(&task);
     }
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahUnload.cpp b/src/hotspot/share/gc/shenandoah/shenandoahUnload.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahUnload.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahUnload.cpp
@@ -79,7 +79,7 @@
 public:
   virtual bool is_unloading(CompiledMethod* method) const {
     nmethod* const nm = method->as_nmethod();
-    guarantee(ShenandoahHeap::heap()->is_evacuation_in_progress(), "Only this phase");
+    guarantee(ShenandoahHeap::heap()->is_concurrent_root_in_progress(), "Only this phase");
     ShenandoahNMethod* data = ShenandoahNMethod::gc_data(nm);
     ShenandoahReentrantLocker locker(data->lock());
     ShenandoahIsUnloadingOopClosure cl;
@@ -166,7 +166,7 @@
 
 void ShenandoahUnload::unload() {
   assert(ShenandoahConcurrentRoots::can_do_concurrent_class_unloading(), "Why we here?");
-  if (!ShenandoahHeap::heap()->is_evacuation_in_progress()) {
+  if (!ShenandoahHeap::heap()->is_concurrent_root_in_progress()) {
     return;
   }
 
# HG changeset patch
# User rkennke
# Date 1576101750 -3600
#      Wed Dec 11 23:02:30 2019 +0100
# Node ID 2ad39334050281d321b0802e387336f056422a91
# Parent  aa7b71106064e1b05b92f53251dd8dc830679d7d
[mq]: JDK-8234974-changes.patch

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp
@@ -265,7 +265,12 @@
 
   ShenandoahMarkingContext* const marking_context = _heap->marking_context();
   if (_heap->is_concurrent_root_in_progress() && !marking_context->is_marked(obj)) {
-    return NULL;
+    Thread* thr = Thread::current();
+    if (thr->is_Java_thread()) {
+      return NULL;
+    } else {
+      return obj;
+    }
   }
 
   oop fwd = load_reference_barrier_not_null(obj);
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSetNMethod.cpp b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSetNMethod.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSetNMethod.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSetNMethod.cpp
@@ -57,10 +57,8 @@
   }
 
   // Heal oops and disarm
-  if (ShenandoahHeap::heap()->is_evacuation_in_progress()) {
-    ShenandoahEvacOOMScope scope;
-    ShenandoahNMethod::heal_nmethod(nm);
-  }
+  ShenandoahEvacOOMScope scope;
+  ShenandoahNMethod::heal_nmethod(nm);
   ShenandoahNMethod::disarm_nmethod(nm);
   return true;
 }
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp b/src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp
@@ -94,12 +94,13 @@
   inline void do_oop_work(T* p);
 };
 
-class ShenandoahEvacUpdateOopStorageRootsClosure : public BasicOopIterateClosure {
+class ShenandoahEvacUpdateCleanupRootsClosure : public BasicOopIterateClosure {
 private:
   ShenandoahHeap* _heap;
   Thread* _thread;
+  const ShenandoahMarkingContext* const _context;
 public:
-  inline ShenandoahEvacUpdateOopStorageRootsClosure();
+  inline ShenandoahEvacUpdateCleanupRootsClosure();
   inline void do_oop(oop* p);
   inline void do_oop(narrowOop* p);
 };
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp b/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp
@@ -135,29 +135,30 @@
   do_oop_work(p);
 }
 
-ShenandoahEvacUpdateOopStorageRootsClosure::ShenandoahEvacUpdateOopStorageRootsClosure() :
-  _heap(ShenandoahHeap::heap()), _thread(Thread::current()) {
+ShenandoahEvacUpdateCleanupRootsClosure::ShenandoahEvacUpdateCleanupRootsClosure() :
+  _heap(ShenandoahHeap::heap()), _thread(Thread::current()), _context(_heap->complete_marking_context()) {
 }
 
-void ShenandoahEvacUpdateOopStorageRootsClosure::do_oop(oop* p) {
+void ShenandoahEvacUpdateCleanupRootsClosure::do_oop(oop* p) {
   assert(_heap->is_concurrent_root_in_progress(), "Only do this when evacuation is in progress");
 
   oop obj = RawAccess<>::oop_load(p);
   if (! CompressedOops::is_null(obj)) {
-    if (_heap->in_collection_set(obj)) {
+    if (!_context->is_marked(obj)) {
+      Atomic::cmpxchg(p, obj, (oop)NULL);
+    } else if (_heap->in_collection_set(obj)) {
       assert(_heap->is_evacuation_in_progress(), "Only do this when evacuation is in progress");
       shenandoah_assert_marked(p, obj);
       oop resolved = ShenandoahBarrierSet::resolve_forwarded_not_null(obj);
       if (resolved == obj) {
         resolved = _heap->evacuate_object(obj, _thread);
       }
-
       Atomic::cmpxchg(p, obj, resolved);
     }
   }
 }
 
-void ShenandoahEvacUpdateOopStorageRootsClosure::do_oop(narrowOop* p) {
+void ShenandoahEvacUpdateCleanupRootsClosure::do_oop(narrowOop* p) {
   ShouldNotReachHere();
 }
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
@@ -1555,6 +1555,10 @@
         verifier()->verify_before_evacuation();
       }
 
+      if (ShenandoahConcurrentRoots::should_do_concurrent_class_unloading()) {
+        ShenandoahCodeRoots::prepare_concurrent_unloading();
+      }
+
       set_evacuation_in_progress(true);
       // From here on, we need to update references.
       set_has_forwarded_objects(true);
@@ -1656,16 +1660,10 @@
 
   void work(uint worker_id) {
     ShenandoahEvacOOMScope oom;
+    ShenandoahEvacUpdateCleanupRootsClosure cl;
     {
-      // jni_roots and weak_roots are OopStorage backed roots, concurrent iteration
-      // may race against OopStorage::release() calls.
-      ShenandoahEvacUpdateOopStorageRootsClosure cl;
-      _vm_roots.oops_do<ShenandoahEvacUpdateOopStorageRootsClosure>(&cl);
-      _weak_roots.oops_do<ShenandoahEvacUpdateOopStorageRootsClosure>(&cl);
-    }
-
-    {
-      ShenandoahEvacuateUpdateRootsClosure cl;
+      _vm_roots.oops_do<>(&cl);
+      _weak_roots.oops_do<>(&cl);
       CLDToOopClosure clds(&cl, ClassLoaderData::_claim_strong);
       _cld_roots.cld_do(&clds);
     }
@@ -1674,14 +1672,13 @@
 
 void ShenandoahHeap::op_roots() {
   if (is_concurrent_root_in_progress()) {
+    if (ShenandoahConcurrentRoots::should_do_concurrent_roots()) {
+      ShenandoahConcurrentRootsEvacUpdateTask task;
+      workers()->run_task(&task);
+    }
     if (ShenandoahConcurrentRoots::should_do_concurrent_class_unloading()) {
       _unloader.unload();
     }
-
-    if (ShenandoahConcurrentRoots::should_do_concurrent_roots() && is_concurrent_root_in_progress()) {
-      ShenandoahConcurrentRootsEvacUpdateTask task;
-      workers()->run_task(&task);
-    }
   }
 
   set_concurrent_root_in_progress(false);
@@ -2239,7 +2236,6 @@
 void ShenandoahHeap::prepare_concurrent_unloading() {
   assert(SafepointSynchronize::is_at_safepoint(), "Must be at a safepoint");
   if (ShenandoahConcurrentRoots::should_do_concurrent_class_unloading()) {
-    ShenandoahCodeRoots::prepare_concurrent_unloading();
     _unloader.prepare();
   }
 }