8256415: Shenandoah: Should evacuate/update codecache concurrently wh…

…en class unloading is off

Reviewed-by: rkennke

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

@@ -44,19 +44,11 @@
 class ShenandoahBarrierSetC1;
 class ShenandoahBarrierSetC2;
 
-static BarrierSetNMethod* make_barrier_set_nmethod(ShenandoahHeap* heap) {
-  // NMethod barriers are only used when concurrent nmethod unloading is enabled
-  if (!ShenandoahConcurrentRoots::can_do_concurrent_class_unloading()) {
-    return NULL;
-  }
-  return new ShenandoahBarrierSetNMethod(heap);
-}
-
 ShenandoahBarrierSet::ShenandoahBarrierSet(ShenandoahHeap* heap) :
   BarrierSet(make_barrier_set_assembler<ShenandoahBarrierSetAssembler>(),
              make_barrier_set_c1<ShenandoahBarrierSetC1>(),
              make_barrier_set_c2<ShenandoahBarrierSetC2>(),
-             make_barrier_set_nmethod(heap),
+             new ShenandoahBarrierSetNMethod(heap),
              BarrierSet::FakeRtti(BarrierSet::ShenandoahBarrierSet)),
   _heap(heap),
   _satb_mark_queue_buffer_allocator("SATB Buffer Allocator", ShenandoahSATBBufferSize),

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

@@ -420,7 +420,8 @@ void ShenandoahControlThread::service_concurrent_normal_cycle(GCCause::Cause cau
   }
 
   // Perform concurrent class unloading
-  if (heap->is_concurrent_weak_root_in_progress()) {
+  if (heap->is_concurrent_weak_root_in_progress() &&
+      ShenandoahConcurrentRoots::should_do_concurrent_class_unloading()) {
     heap->entry_class_unloading();
   }
 

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

@@ -1142,9 +1142,7 @@ void ShenandoahHeap::evacuate_and_update_roots() {
   {
     // Include concurrent roots if current cycle can not process those roots concurrently
     ShenandoahRootEvacuator rp(workers()->active_workers(),
-                               ShenandoahPhaseTimings::init_evac,
-                               !ShenandoahConcurrentRoots::should_do_concurrent_roots(),
-                               !ShenandoahConcurrentRoots::should_do_concurrent_class_unloading());
+                               ShenandoahPhaseTimings::init_evac);
     ShenandoahEvacuateUpdateRootsTask roots_task(&rp);
     workers()->run_task(&roots_task);
   }
@@ -1744,9 +1742,8 @@ void ShenandoahHeap::op_final_mark() {
       set_has_forwarded_objects(true);
 
       if (!is_degenerated_gc_in_progress()) {
-        if (ShenandoahConcurrentRoots::should_do_concurrent_class_unloading()) {
-          ShenandoahCodeRoots::arm_nmethods();
-        }
+        // Arm nmethods for concurrent codecache processing.
+        ShenandoahCodeRoots::arm_nmethods();
         evacuate_and_update_roots();
       }
 
@@ -1757,17 +1754,10 @@ void ShenandoahHeap::op_final_mark() {
       if (ShenandoahVerify) {
         // If OOM while evacuating/updating of roots, there is no guarantee of their consistencies
         if (!cancelled_gc()) {
-          ShenandoahRootVerifier::RootTypes types = ShenandoahRootVerifier::None;
-          if (ShenandoahConcurrentRoots::should_do_concurrent_roots()) {
-            types = ShenandoahRootVerifier::combine(ShenandoahRootVerifier::JNIHandleRoots, ShenandoahRootVerifier::WeakRoots);
-            types = ShenandoahRootVerifier::combine(types, ShenandoahRootVerifier::CLDGRoots);
-            types = ShenandoahRootVerifier::combine(types, ShenandoahRootVerifier::StringDedupRoots);
-          }
-
-          if (ShenandoahConcurrentRoots::should_do_concurrent_class_unloading()) {
-            types = ShenandoahRootVerifier::combine(types, ShenandoahRootVerifier::CodeRoots);
-          }
-          verifier()->verify_roots_no_forwarded_except(types);
+          // We only evacuate/update thread and serial weak roots at this pause
+          ShenandoahRootVerifier::RootTypes types = ShenandoahRootVerifier::combine(ShenandoahRootVerifier::ThreadRoots,
+                                                                                    ShenandoahRootVerifier::SerialWeakRoots);
+          verifier()->verify_roots_no_forwarded(types);
         }
         verifier()->verify_during_evacuation();
       }
@@ -1840,16 +1830,53 @@ void ShenandoahHeap::op_cleanup_complete() {
   free_set()->recycle_trash();
 }
 
+class ShenandoahEvacUpdateCodeCacheClosure : public NMethodClosure {
+private:
+  BarrierSetNMethod* const               _bs;
+  ShenandoahEvacuateUpdateRootsClosure<> _cl;
+
+public:
+  ShenandoahEvacUpdateCodeCacheClosure() :
+    _bs(BarrierSet::barrier_set()->barrier_set_nmethod()),
+    _cl() {
+  }
+
+  void do_nmethod(nmethod* n) {
+    ShenandoahNMethod* data = ShenandoahNMethod::gc_data(n);
+    ShenandoahReentrantLocker locker(data->lock());
+    data->oops_do(&_cl, true/*fix relocation*/);
+    _bs->disarm(n);
+  }
+};
+
 class ShenandoahConcurrentRootsEvacUpdateTask : public AbstractGangTask {
 private:
+  ShenandoahPhaseTimings::Phase                 _phase;
   ShenandoahVMRoots<true /*concurrent*/>        _vm_roots;
   ShenandoahClassLoaderDataRoots<true /*concurrent*/, false /*single threaded*/> _cld_roots;
+  ShenandoahConcurrentNMethodIterator           _nmethod_itr;
+  bool                                          _process_codecache;
 
 public:
   ShenandoahConcurrentRootsEvacUpdateTask(ShenandoahPhaseTimings::Phase phase) :
     AbstractGangTask("Shenandoah Evacuate/Update Concurrent Strong Roots"),
+    _phase(phase),
     _vm_roots(phase),
-    _cld_roots(phase, ShenandoahHeap::heap()->workers()->active_workers()) {}
+    _cld_roots(phase, ShenandoahHeap::heap()->workers()->active_workers()),
+    _nmethod_itr(ShenandoahCodeRoots::table()),
+    _process_codecache(!ShenandoahConcurrentRoots::should_do_concurrent_class_unloading()) {
+    if (_process_codecache) {
+      MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
+      _nmethod_itr.nmethods_do_begin();
+    }
+  }
+
+  ~ShenandoahConcurrentRootsEvacUpdateTask() {
+    if (_process_codecache) {
+      MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
+      _nmethod_itr.nmethods_do_end();
+    }
+  }
 
   void work(uint worker_id) {
     ShenandoahConcurrentWorkerSession worker_session(worker_id);
@@ -1866,6 +1893,12 @@ class ShenandoahConcurrentRootsEvacUpdateTask : public AbstractGangTask {
       CLDToOopClosure clds(&cl, ClassLoaderData::_claim_strong);
       _cld_roots.cld_do(&clds, worker_id);
     }
+
+    if (_process_codecache) {
+      ShenandoahWorkerTimingsTracker timer(_phase, ShenandoahPhaseTimings::CodeCacheRoots, worker_id);
+      ShenandoahEvacUpdateCodeCacheClosure cl;
+      _nmethod_itr.nmethods_do(&cl);
+    }
   }
 };
 

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

@@ -147,13 +147,6 @@ ShenandoahNMethod* ShenandoahNMethod::for_nmethod(nmethod* nm) {
   GrowableArray<oop*> oops;
 
   detect_reloc_oops(nm, oops, non_immediate_oops);
-
-  // No embedded oops
-  if(!ShenandoahConcurrentRoots::can_do_concurrent_class_unloading() &&
-    oops.is_empty() && nm->oops_begin() >= nm->oops_end()) {
-    return NULL;
-  }
-
   return new ShenandoahNMethod(nm, oops, non_immediate_oops);
 }
 
@@ -195,7 +188,8 @@ void ShenandoahNMethod::heal_nmethod(nmethod* nm) {
       ShenandoahKeepNMethodMetadataAliveClosure<false> cl;
       data->oops_do(&cl);
     }
-  } else if (heap->is_concurrent_weak_root_in_progress()) {
+  } else if (heap->is_concurrent_weak_root_in_progress() ||
+             heap->is_concurrent_strong_root_in_progress()) {
     ShenandoahEvacOOMScope evac_scope;
     heal_nmethod_metadata(data);
   } else {
@@ -288,12 +282,6 @@ void ShenandoahNMethod::assert_same_oops(bool allow_dead) {
           oops->length(), _oops_count, count, p2i(nm()->oops_begin()), p2i(nm()->oops_end()), debug_stream.as_string());
   }
 }
-
-void ShenandoahNMethod::assert_no_oops(nmethod* nm, bool allow_dead) {
-  ShenandoahNMethodOopDetector detector;
-  nm->oops_do(&detector, allow_dead);
-  assert(detector.oops()->length() == 0, "Should not have oops");
-}
 #endif
 
 ShenandoahNMethodTable::ShenandoahNMethodTable() :
@@ -321,11 +309,7 @@ void ShenandoahNMethodTable::register_nmethod(nmethod* nm) {
     data->update();
   } else {
     data = ShenandoahNMethod::for_nmethod(nm);
-    if (data == NULL) {
-      assert(!ShenandoahConcurrentRoots::can_do_concurrent_class_unloading(),
-             "Only possible when concurrent class unloading is off");
-      return;
-    }
+    assert(data != NULL, "Sanity");
     ShenandoahNMethod::attach_gc_data(nm, data);
     ShenandoahLocker locker(&_lock);
     log_register_nmethod(nm);
@@ -339,13 +323,7 @@ void ShenandoahNMethodTable::unregister_nmethod(nmethod* nm) {
   assert_locked_or_safepoint(CodeCache_lock);
 
   ShenandoahNMethod* data = ShenandoahNMethod::gc_data(nm);
-  if (data == NULL) {
-    assert(!ShenandoahConcurrentRoots::can_do_concurrent_class_unloading(),
-           "Only possible when concurrent class unloading is off");
-    ShenandoahNMethod::assert_no_oops(nm, true /*allow_dead*/);
-    return;
-  }
-
+  assert(data != NULL, "Sanity");
   if (Thread::current()->is_Code_cache_sweeper_thread()) {
     wait_until_concurrent_iteration_done();
   }
@@ -361,12 +339,7 @@ void ShenandoahNMethodTable::flush_nmethod(nmethod* nm) {
   assert(CodeCache_lock->owned_by_self(), "Must have CodeCache_lock held");
   assert(Thread::current()->is_Code_cache_sweeper_thread(), "Must from Sweep thread");
   ShenandoahNMethod* data = ShenandoahNMethod::gc_data(nm);
-  assert(data != NULL || !ShenandoahConcurrentRoots::can_do_concurrent_class_unloading(),
-         "Only possible when concurrent class unloading is off");
-  if (data == NULL) {
-    ShenandoahNMethod::assert_no_oops(nm, true /*allow_dead*/);
-    return;
-  }
+  assert(data != NULL, "Sanity");
 
   // Can not alter the array when iteration is in progress
   wait_until_concurrent_iteration_done();
@@ -598,8 +571,6 @@ ShenandoahConcurrentNMethodIterator::ShenandoahConcurrentNMethodIterator(Shenand
 
 void ShenandoahConcurrentNMethodIterator::nmethods_do_begin() {
   assert(CodeCache_lock->owned_by_self(), "Lock must be held");
-  assert(ShenandoahConcurrentRoots::can_do_concurrent_class_unloading(),
-         "Only for concurrent class unloading");
   _table_snapshot = _table->snapshot_for_iteration();
 }
 
@@ -610,8 +581,6 @@ void ShenandoahConcurrentNMethodIterator::nmethods_do(NMethodClosure* cl) {
 
 void ShenandoahConcurrentNMethodIterator::nmethods_do_end() {
   assert(CodeCache_lock->owned_by_self(), "Lock must be held");
-  assert(ShenandoahConcurrentRoots::can_do_concurrent_class_unloading(),
-         "Only for concurrent class unloading");
   _table->finish_iteration(_table_snapshot);
   CodeCache_lock->notify_all();
 }

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

@@ -75,7 +75,6 @@ class ShenandoahNMethod : public CHeapObj<mtGC> {
 
   void assert_correct() NOT_DEBUG_RETURN;
   void assert_same_oops(bool allow_dead = false) NOT_DEBUG_RETURN;
-  static void assert_no_oops(nmethod* nm, bool allow_dea = false) NOT_DEBUG_RETURN;
 
 private:
   bool has_non_immed_oops() const { return _has_non_immed_oops; }

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

@@ -78,10 +78,6 @@ void ShenandoahNMethod::heal_nmethod_metadata(ShenandoahNMethod* nmethod_data) {
 }
 
 void ShenandoahNMethod::disarm_nmethod(nmethod* nm) {
-  if (!ShenandoahConcurrentRoots::can_do_concurrent_class_unloading()) {
-    return;
-  }
-
   BarrierSetNMethod* const bs = BarrierSet::barrier_set()->barrier_set_nmethod();
   assert(bs != NULL, "Sanity");
   if (bs->is_armed(nm)) {

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

@@ -182,50 +182,26 @@ void ShenandoahRootScanner::roots_do(uint worker_id, OopClosure* oops, CodeBlobC
 }
 
 ShenandoahRootEvacuator::ShenandoahRootEvacuator(uint n_workers,
-                                                 ShenandoahPhaseTimings::Phase phase,
-                                                 bool stw_roots_processing,
-                                                 bool stw_class_unloading) :
+                                                 ShenandoahPhaseTimings::Phase phase) :
   ShenandoahRootProcessor(phase),
-  _vm_roots(phase),
-  _cld_roots(phase, n_workers),
   _thread_roots(phase, n_workers > 1),
-  _serial_weak_roots(phase),
-  _weak_roots(phase),
-  _dedup_roots(phase),
-  _code_roots(phase),
-  _stw_roots_processing(stw_roots_processing),
-  _stw_class_unloading(stw_class_unloading) {
+  _serial_weak_roots(phase) {
+  nmethod::oops_do_marking_prologue();
 }
 
-void ShenandoahRootEvacuator::roots_do(uint worker_id, OopClosure* oops) {
-  MarkingCodeBlobClosure blobsCl(oops, CodeBlobToOopClosure::FixRelocations);
-  ShenandoahCodeBlobAndDisarmClosure blobs_and_disarm_Cl(oops);
-  CodeBlobToOopClosure* codes_cl = ShenandoahConcurrentRoots::can_do_concurrent_class_unloading() ?
-                                   static_cast<CodeBlobToOopClosure*>(&blobs_and_disarm_Cl) :
-                                   static_cast<CodeBlobToOopClosure*>(&blobsCl);
-  AlwaysTrueClosure always_true;
+ShenandoahRootEvacuator::~ShenandoahRootEvacuator() {
+  nmethod::oops_do_marking_epilogue();
+}
 
+void ShenandoahRootEvacuator::roots_do(uint worker_id, OopClosure* oops) {
+  // Always disarm on-stack nmethods, because we are evacuating/updating them
+  // here
+  ShenandoahCodeBlobAndDisarmClosure codeblob_cl(oops);
   // Process serial-claiming roots first
   _serial_weak_roots.weak_oops_do(oops, worker_id);
 
   // Process light-weight/limited parallel roots then
-  if (_stw_roots_processing) {
-    _vm_roots.oops_do<OopClosure>(oops, worker_id);
-    _weak_roots.oops_do<OopClosure>(oops, worker_id);
-    _dedup_roots.oops_do(&always_true, oops, worker_id);
-  }
-  if (_stw_class_unloading) {
-    CLDToOopClosure clds(oops, ClassLoaderData::_claim_strong);
-    _cld_roots.cld_do(&clds, worker_id);
-  }
-
-  // Process heavy-weight/fully parallel roots the last
-  if (_stw_class_unloading) {
-    _code_roots.code_blobs_do(codes_cl, worker_id);
-    _thread_roots.oops_do(oops, NULL, worker_id);
-  } else {
-    _thread_roots.oops_do(oops, codes_cl, worker_id);
-  }
+  _thread_roots.oops_do(oops, &codeblob_cl, worker_id);
 }
 
 ShenandoahRootUpdater::ShenandoahRootUpdater(uint n_workers, ShenandoahPhaseTimings::Phase phase) :

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

@@ -232,19 +232,11 @@ class ShenandoahHeapIterationRootScanner : public ShenandoahRootProcessor {
 // Evacuate all roots at a safepoint
 class ShenandoahRootEvacuator : public ShenandoahRootProcessor {
 private:
-  ShenandoahVMRoots<false /*concurrent*/>                   _vm_roots;
-  ShenandoahClassLoaderDataRoots<false /*concurrent*/, false /*single threaded*/>
-                                                            _cld_roots;
   ShenandoahThreadRoots                                     _thread_roots;
   ShenandoahSerialWeakRoots                                 _serial_weak_roots;
-  ShenandoahVMWeakRoots<false /*concurrent*/>               _weak_roots;
-  ShenandoahStringDedupRoots                                _dedup_roots;
-  ShenandoahCodeCacheRoots                                  _code_roots;
-  bool                                                      _stw_roots_processing;
-  bool                                                      _stw_class_unloading;
 public:
-  ShenandoahRootEvacuator(uint n_workers, ShenandoahPhaseTimings::Phase phase,
-                          bool stw_roots_processing, bool stw_class_unloading);
+  ShenandoahRootEvacuator(uint n_workers, ShenandoahPhaseTimings::Phase phase);
+  ~ShenandoahRootEvacuator();
 
   void roots_do(uint worker_id, OopClosure* oops);
 };

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

@@ -1010,6 +1010,12 @@ void ShenandoahVerifier::verify_roots_no_forwarded() {
   verifier.oops_do(&cl);
 }
 
+void ShenandoahVerifier::verify_roots_no_forwarded(ShenandoahRootVerifier::RootTypes types) {
+  ShenandoahRootVerifier verifier(types);
+  ShenandoahVerifyNoForwared cl;
+  verifier.oops_do(&cl);
+}
+
 void ShenandoahVerifier::verify_roots_no_forwarded_except(ShenandoahRootVerifier::RootTypes types) {
   ShenandoahRootVerifier verifier;
   verifier.excludes(types);

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

@@ -196,6 +196,7 @@ class ShenandoahVerifier : public CHeapObj<mtGC> {
   void verify_roots_in_to_space_except(ShenandoahRootVerifier::RootTypes types);
 
   void verify_roots_no_forwarded();
+  void verify_roots_no_forwarded(ShenandoahRootVerifier::RootTypes types);
   void verify_roots_no_forwarded_except(ShenandoahRootVerifier::RootTypes types);
 };