# HG changeset patch
# User zgu
# Date 1553616769 14400
#      Tue Mar 26 12:12:49 2019 -0400
# Node ID 6e176ad4e0694230795680d75a438d9c56afd533
# Parent  3908850f5027b25cbd85e420286e06f45a3bab2c
[backport] 8221435: Shenandoah should not mark through weak roots
Reviewed-by: rkennke, shade

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
@@ -121,11 +121,10 @@
 
     CLDToOopClosure clds_cl(oops);
     MarkingCodeBlobClosure blobs_cl(oops, ! CodeBlobToOopClosure::FixRelocations);
-    OopClosure* weak_oops = _process_refs ? NULL : oops;
 
     ResourceMark m;
     if (heap->unload_classes()) {
-      _rp->process_strong_roots(oops, weak_oops, &clds_cl, NULL, &blobs_cl, NULL, worker_id);
+      _rp->process_strong_roots(oops, &clds_cl, NULL, &blobs_cl, NULL, worker_id);
     } else {
       if (ShenandoahConcurrentScanCodeRoots) {
         CodeBlobClosure* code_blobs = NULL;
@@ -138,9 +137,9 @@
           code_blobs = &assert_to_space;
         }
 #endif
-        _rp->process_all_roots(oops, weak_oops, &clds_cl, code_blobs, NULL, worker_id);
+        _rp->process_all_roots(oops, &clds_cl, code_blobs, NULL, worker_id);
       } else {
-        _rp->process_all_roots(oops, weak_oops, &clds_cl, &blobs_cl, NULL, worker_id);
+        _rp->process_all_roots(oops, &clds_cl, &blobs_cl, NULL, worker_id);
       }
     }
   }
@@ -178,7 +177,7 @@
         DEBUG_ONLY(&assert_to_space)
         NOT_DEBUG(NULL);
     }
-    _rp->process_all_roots(&cl, &cl, &cldCl, code_blobs, NULL, worker_id);
+    _rp->update_all_roots(&cl, &cldCl, code_blobs, NULL, worker_id);
   }
 };
 
@@ -456,11 +455,17 @@
     weak_refs_work(full_gc);
   }
 
+  weak_roots_work();
+
   // And finally finish class unloading
   if (_heap->unload_classes()) {
     _heap->unload_classes_and_cleanup_tables(full_gc);
   }
-
+  if (ShenandoahStringDedup::is_enabled()) {
+    ShenandoahIsAliveSelector alive;
+    BoolObjectClosure* is_alive = alive.is_alive_closure();
+    ShenandoahStringDedup::unlink_or_oops_do(is_alive, NULL, false);
+  }
   assert(task_queues()->is_empty(), "Should be empty");
   TASKQUEUE_STATS_ONLY(task_queues()->print_taskqueue_stats());
   TASKQUEUE_STATS_ONLY(task_queues()->reset_taskqueue_stats());
@@ -565,11 +570,13 @@
 private:
   template <class T>
   inline void do_oop_work(T* p) {
+#ifdef ASSERT
     T o = RawAccess<>::oop_load(p);
     if (!CompressedOops::is_null(o)) {
       oop obj = CompressedOops::decode_not_null(o);
       shenandoah_assert_not_forwarded(p, obj);
     }
+#endif
   }
 
 public:
@@ -658,6 +665,22 @@
 
 }
 
+// Process leftover weak oops: update them, if needed or assert they do not
+// need updating otherwise.
+// Weak processor API requires us to visit the oops, even if we are not doing
+// anything to them.
+void ShenandoahConcurrentMark::weak_roots_work() {
+  ShenandoahIsAliveSelector is_alive;
+
+  if (_heap->has_forwarded_objects()) {
+    ShenandoahWeakUpdateClosure cl;
+    WeakProcessor::weak_oops_do(is_alive.is_alive_closure(), &cl);
+  } else {
+    ShenandoahWeakAssertNotForwardedClosure cl;
+    WeakProcessor::weak_oops_do(is_alive.is_alive_closure(), &cl);
+  }
+}
+
 void ShenandoahConcurrentMark::weak_refs_work_doit(bool full_gc) {
   ReferenceProcessor* rp = _heap->ref_processor();
 
@@ -699,26 +722,18 @@
     ShenandoahGCPhase phase(phase_process);
     ShenandoahTerminationTracker phase_term(phase_process_termination);
 
-    // Process leftover weak oops: update them, if needed, or assert they do not
-    // need updating otherwise. This JDK version does not have parallel WeakProcessor.
-    // Weak processor API requires us to visit the oops, even if we are not doing
-    // anything to them.
     if (_heap->has_forwarded_objects()) {
       ShenandoahCMKeepAliveUpdateClosure keep_alive(get_queue(serial_worker_id));
       rp->process_discovered_references(is_alive.is_alive_closure(), &keep_alive,
                                         &complete_gc, &executor,
                                         &pt);
 
-      ShenandoahWeakUpdateClosure cl;
-      WeakProcessor::weak_oops_do(is_alive.is_alive_closure(), &cl);
     } else {
       ShenandoahCMKeepAliveClosure keep_alive(get_queue(serial_worker_id));
       rp->process_discovered_references(is_alive.is_alive_closure(), &keep_alive,
                                         &complete_gc, &executor,
                                         &pt);
 
-      ShenandoahWeakAssertNotForwardedClosure cl;
-      WeakProcessor::weak_oops_do(is_alive.is_alive_closure(), &cl);
     }
 
     pt.print_all_references();
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.hpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.hpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2018, Red Hat, Inc. All rights reserved.
+ * Copyright (c) 2013, 2019, Red Hat, Inc. All rights reserved.
  *
  * This code is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License version 2 only, as
@@ -86,6 +86,8 @@
   void weak_refs_work(bool full_gc);
   void weak_refs_work_doit(bool full_gc);
 
+  void weak_roots_work();
+
 public:
   void preclean_weak_refs();
 
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
@@ -1117,16 +1117,6 @@
 #endif
 }
 
-void ShenandoahHeap::roots_iterate(OopClosure* cl) {
-  assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Only iterate roots while world is stopped");
-
-  CodeBlobToOopClosure blobsCl(cl, false);
-  CLDToOopClosure cldCl(cl);
-
-  ShenandoahRootProcessor rp(this, 1, ShenandoahPhaseTimings::_num_phases);
-  rp.process_all_roots(cl, NULL, &cldCl, &blobsCl, NULL, 0);
-}
-
 // Returns size in bytes
 size_t ShenandoahHeap::unsafe_max_tlab_alloc(Thread *thread) const {
   if (ShenandoahElasticTLAB) {
@@ -1335,7 +1325,7 @@
   ObjectIterateScanRootClosure oops(&_aux_bit_map, &oop_stack);
   CLDToOopClosure clds(&oops, false);
   CodeBlobToOopClosure blobs(&oops, false);
-  rp.process_all_roots(&oops, &oops, &clds, &blobs, NULL, 0);
+  rp.process_all_roots(&oops, &clds, &blobs, NULL, 0);
 
   // Work through the oop stack to traverse heap.
   while (! oop_stack.is_empty()) {
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp
@@ -705,8 +705,6 @@
 
   void stop_concurrent_marking();
 
-  void roots_iterate(OopClosure* cl);
-
 private:
   void trash_cset_regions();
   void update_heap_references(bool concurrent);
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp b/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp
@@ -575,9 +575,9 @@
     MarkingCodeBlobClosure adjust_code_closure(&cl,
                                              CodeBlobToOopClosure::FixRelocations);
 
-    _rp->process_all_roots(&cl, &cl,
-                           &adjust_cld_closure,
-                           &adjust_code_closure, NULL, worker_id);
+    _rp->update_all_roots(&cl,
+                          &adjust_cld_closure,
+                          &adjust_code_closure, NULL, worker_id);
   }
 };
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
@@ -76,15 +76,10 @@
   Management::oops_do(oops);
   JvmtiExport::oops_do(oops);
   JNIHandles::oops_do(oops);
-  WeakProcessor::oops_do(oops);
   ObjectSynchronizer::oops_do(oops);
   SystemDictionary::oops_do(oops);
   StringTable::oops_do(oops);
 
-  if (ShenandoahStringDedup::is_enabled()) {
-    ShenandoahStringDedup::oops_do_slow(oops);
-  }
-
   // Do thread roots the last. This allows verification code to find
   // any broken objects from those special roots first, not the accidental
   // dangling reference from the thread root.
@@ -92,7 +87,6 @@
 }
 
 void ShenandoahRootProcessor::process_strong_roots(OopClosure* oops,
-                                                   OopClosure* weak_oops,
                                                    CLDClosure* clds,
                                                    CLDClosure* weak_clds,
                                                    CodeBlobClosure* blobs,
@@ -100,13 +94,12 @@
                                                    uint worker_id) {
 
   process_java_roots(oops, clds, weak_clds, blobs, thread_cl, worker_id);
-  process_vm_roots(oops, NULL, weak_oops, worker_id);
+  process_vm_roots(oops, worker_id);
 
   _process_strong_tasks->all_tasks_completed(n_workers());
 }
 
 void ShenandoahRootProcessor::process_all_roots(OopClosure* oops,
-                                                OopClosure* weak_oops,
                                                 CLDClosure* clds,
                                                 CodeBlobClosure* blobs,
                                                 ThreadClosure* thread_cl,
@@ -114,14 +107,63 @@
 
   ShenandoahWorkerTimings* worker_times = ShenandoahHeap::heap()->phase_timings()->worker_times();
   process_java_roots(oops, clds, clds, blobs, thread_cl, worker_id);
-  process_vm_roots(oops, oops, weak_oops, worker_id);
+  process_vm_roots(oops, worker_id);
 
   if (blobs != NULL) {
     ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::CodeCacheRoots, worker_id);
     _coderoots_all_iterator.possibly_parallel_blobs_do(blobs);
   }
 
+  // All threads execute the following. A specific chunk of buckets
+  // from the StringTable are the individual tasks.
+  {
+    ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::StringTableRoots, worker_id);
+    StringTable::possibly_parallel_oops_do(&_par_state_string, oops);
+  }
+
   _process_strong_tasks->all_tasks_completed(n_workers());
+
+}
+
+void ShenandoahRootProcessor::update_all_roots(OopClosure* oops,
+                                               CLDClosure* clds,
+                                               CodeBlobClosure* blobs,
+                                               ThreadClosure* thread_cl,
+                                               uint worker_id) {
+  ShenandoahWorkerTimings* worker_times = ShenandoahHeap::heap()->phase_timings()->worker_times();
+  AlwaysTrueClosure always_true;
+
+  if (!_process_strong_tasks->is_task_claimed(SHENANDOAH_RP_PS_JNIHandles_weak_oops_do)) {
+    ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::JNIWeakRoots, worker_id);
+    WeakProcessor::weak_oops_do(&always_true, oops);
+  }
+
+  process_all_roots(oops, clds, blobs, thread_cl, worker_id);
+
+  if (ShenandoahStringDedup::is_enabled()) {
+    ShenandoahStringDedup::parallel_oops_do(&always_true, oops, worker_id);
+  }
+}
+
+void ShenandoahRootProcessor::traversal_update_all_roots(OopClosure* oops,
+                                                         CLDClosure* clds,
+                                                         CodeBlobClosure* blobs,
+                                                         ThreadClosure* thread_cl,
+                                                         uint worker_id) {
+  ShenandoahWorkerTimings* worker_times = ShenandoahHeap::heap()->phase_timings()->worker_times();
+  ShenandoahIsAliveSelector is_alive;
+  BoolObjectClosure* is_alive_closure = is_alive.is_alive_closure();
+
+  if (!_process_strong_tasks->is_task_claimed(SHENANDOAH_RP_PS_JNIHandles_weak_oops_do)) {
+    ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::JNIWeakRoots, worker_id);
+    WeakProcessor::weak_oops_do(is_alive_closure, oops);
+  }
+
+  process_all_roots(oops, clds, blobs, thread_cl, worker_id);
+
+  if (ShenandoahStringDedup::is_enabled()) {
+    ShenandoahStringDedup::parallel_oops_do(is_alive_closure, oops, worker_id);
+  }
 }
 
 class ShenandoahParallelOopsDoThreadClosure : public ThreadClosure {
@@ -167,10 +209,7 @@
 }
 
 void ShenandoahRootProcessor::process_vm_roots(OopClosure* strong_roots,
-                                               OopClosure* weak_roots,
-                                               OopClosure* jni_weak_roots,
-                                               uint worker_id)
-{
+                                               uint worker_id) {
   ShenandoahWorkerTimings* worker_times = ShenandoahHeap::heap()->phase_timings()->worker_times();
   if (!_process_strong_tasks->is_task_claimed(SHENANDOAH_RP_PS_Universe_oops_do)) {
     ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::UniverseRoots, worker_id);
@@ -193,16 +232,6 @@
     ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::SystemDictionaryRoots, worker_id);
     SystemDictionary::oops_do(strong_roots);
   }
-  if (jni_weak_roots != NULL) {
-    if (!_process_strong_tasks->is_task_claimed(SHENANDOAH_RP_PS_JNIHandles_weak_oops_do)) {
-      ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::JNIWeakRoots, worker_id);
-      WeakProcessor::oops_do(jni_weak_roots);
-    }
-  }
-
-  if (ShenandoahStringDedup::is_enabled() && weak_roots != NULL) {
-    ShenandoahStringDedup::parallel_oops_do(weak_roots, worker_id);
-  }
 
   {
     ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::ObjectSynchronizerRoots, worker_id);
@@ -210,13 +239,6 @@
        ObjectSynchronizer::oops_do(strong_roots);
     }
   }
-
-  // All threads execute the following. A specific chunk of buckets
-  // from the StringTable are the individual tasks.
-  if (weak_roots != NULL) {
-    ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::StringTableRoots, worker_id);
-    StringTable::possibly_parallel_oops_do(&_par_state_string, weak_roots);
-  }
 }
 
 uint ShenandoahRootProcessor::n_workers() const {
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp
@@ -69,8 +69,6 @@
                           uint worker_i);
 
   void process_vm_roots(OopClosure* scan_non_heap_roots,
-                        OopClosure* scan_non_heap_weak_roots,
-                        OopClosure* weak_jni_roots,
                         uint worker_i);
 
 public:
@@ -78,21 +76,40 @@
                           ShenandoahPhaseTimings::Phase phase);
   ~ShenandoahRootProcessor();
 
-  // Apply oops, clds and blobs to all strongly reachable roots in the system
-  void process_strong_roots(OopClosure* oops, OopClosure* weak_oops,
+  // Apply oops, clds and blobs to all strongly reachable roots in the system.
+  // Optionally, apply class loader closure to weak clds, depending on class unloading
+  // for the particular GC cycles.
+  void process_strong_roots(OopClosure* oops,
                             CLDClosure* clds,
                             CLDClosure* weak_clds,
                             CodeBlobClosure* blobs,
                             ThreadClosure* thread_cl,
                             uint worker_id);
 
-  // Apply oops, clds and blobs to strongly and weakly reachable roots in the system
-  void process_all_roots(OopClosure* oops, OopClosure* weak_oops,
+  // Apply oops, clds and blobs to strongly reachable roots in the system
+  void process_all_roots(OopClosure* oops,
                          CLDClosure* clds,
                          CodeBlobClosure* blobs,
                          ThreadClosure* thread_cl,
                          uint worker_id);
 
+  // Apply oops, clds and blobs to strongly and weakly reachable roots in the system
+  void update_all_roots(OopClosure* oops,
+                        CLDClosure* clds,
+                        CodeBlobClosure* blobs,
+                        ThreadClosure* thread_cl,
+                        uint worker_id);
+
+
+  // Apply oops, clds and blobs to strongly and weakly reachable roots in the system
+  // during traversal GC.
+  // It cleans up and updates weak roots in one iteration.
+  void traversal_update_all_roots(OopClosure* oops,
+                                  CLDClosure* clds,
+                                  CodeBlobClosure* blobs,
+                                  ThreadClosure* thread_cl,
+                                  uint worker_id);
+
   // For slow debug/verification code
   void process_all_roots_slow(OopClosure* oops);
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.cpp b/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.cpp
@@ -71,13 +71,13 @@
   StringDedupTable::deduplicate(java_string, &dummy);
 }
 
-void ShenandoahStringDedup::parallel_oops_do(OopClosure* cl, uint worker_id) {
+void ShenandoahStringDedup::parallel_oops_do(BoolObjectClosure* is_alive, OopClosure* cl, uint worker_id) {
   assert(SafepointSynchronize::is_at_safepoint(), "Must be at a safepoint");
   assert(is_enabled(), "String deduplication not enabled");
 
   ShenandoahWorkerTimings* worker_times = ShenandoahHeap::heap()->phase_timings()->worker_times();
 
-  StringDedupUnlinkOrOopsDoClosure sd_cl(NULL, cl);
+  StringDedupUnlinkOrOopsDoClosure sd_cl(is_alive, cl);
 
   {
     ShenandoahWorkerTimingsTracker x(worker_times, ShenandoahPhaseTimings::StringDedupQueueRoots, worker_id);
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.hpp b/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.hpp
@@ -38,14 +38,14 @@
   // Deduplicate a string, the call is lock-free
   static void deduplicate(oop java_string);
 
-  static void parallel_oops_do(OopClosure* cl, uint worker_id);
+  static void parallel_oops_do(BoolObjectClosure* is_alive, OopClosure* cl, uint worker_id);
   static void oops_do_slow(OopClosure* cl);
 
   // Parallel cleanup string dedup queues/table
   static void parallel_cleanup();
 
   static inline bool is_candidate(oop obj);
-private:
+
   static void unlink_or_oops_do(BoolObjectClosure* is_alive,
                                 OopClosure* keep_alive,
                                 bool allow_resize_and_rehash);
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
@@ -192,13 +192,13 @@
       ShenandoahMarkCLDClosure cld_cl(&roots_cl);
       MarkingCodeBlobClosure code_cl(&roots_cl, CodeBlobToOopClosure::FixRelocations);
       if (unload_classes) {
-        _rp->process_strong_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, NULL, NULL, NULL, worker_id);
+        _rp->process_strong_roots(&roots_cl, &cld_cl, NULL, NULL, NULL, worker_id);
         // Need to pre-evac code roots here. Otherwise we might see from-space constants.
         ShenandoahWorkerTimings* worker_times = _heap->phase_timings()->worker_times();
         ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::CodeCacheRoots, worker_id);
         _cset_coderoots->possibly_parallel_blobs_do(&code_cl);
       } else {
-        _rp->process_all_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, &code_cl, NULL, worker_id);
+        _rp->process_all_roots(&roots_cl, &cld_cl, &code_cl, NULL, worker_id);
       }
     }
   }
@@ -274,9 +274,9 @@
       ShenandoahTraversalSATBThreadsClosure tc(&satb_cl);
       if (unload_classes) {
         ShenandoahRemarkCLDClosure weak_cld_cl(&roots_cl);
-        _rp->process_strong_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, &weak_cld_cl, NULL, &tc, worker_id);
+        _rp->process_strong_roots(&roots_cl, &cld_cl, &weak_cld_cl, NULL, &tc, worker_id);
       } else {
-        _rp->process_all_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, NULL, &tc, worker_id);
+        _rp->process_all_roots(&roots_cl, &cld_cl, NULL, &tc, worker_id);
       }
     } else {
       ShenandoahTraversalDegenClosure roots_cl(q, rp);
@@ -284,9 +284,9 @@
       ShenandoahTraversalSATBThreadsClosure tc(&satb_cl);
       if (unload_classes) {
         ShenandoahRemarkCLDClosure weak_cld_cl(&roots_cl);
-        _rp->process_strong_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, &weak_cld_cl, NULL, &tc, worker_id);
+        _rp->process_strong_roots(&roots_cl, &cld_cl, &weak_cld_cl, NULL, &tc, worker_id);
       } else {
-        _rp->process_all_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, NULL, &tc, worker_id);
+        _rp->process_all_roots(&roots_cl, &cld_cl, NULL, &tc, worker_id);
       }
     }
 
@@ -605,8 +605,11 @@
     weak_refs_work();
   }
 
-  if (!_heap->cancelled_gc() && _heap->unload_classes()) {
-    _heap->unload_classes_and_cleanup_tables(false);
+  if (!_heap->cancelled_gc()) {
+    if (_heap->unload_classes()) {
+      _heap->unload_classes_and_cleanup_tables(false);
+    }
+
     fixup_roots();
   }
 
@@ -709,7 +712,7 @@
     ShenandoahTraversalFixRootsClosure cl;
     MarkingCodeBlobClosure blobsCl(&cl, CodeBlobToOopClosure::FixRelocations);
     CLDToOopClosure cldCl(&cl);
-    _rp->process_all_roots(&cl, &cl, &cldCl, &blobsCl, NULL, worker_id);
+    _rp->traversal_update_all_roots(&cl, &cldCl, &blobsCl, NULL, worker_id);
   }
 };
 
# HG changeset patch
# User zgu
# Date 1553795621 14400
#      Thu Mar 28 13:53:41 2019 -0400
# Node ID 3a1e23d1eadfd9de68b21d57f617ec47fafbee07
# Parent  6e176ad4e0694230795680d75a438d9c56afd533
[backport] 8221629: Shenandoah: Cleanup class unloading logic
Reviewed-by: rkennke

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
@@ -37,7 +37,7 @@
 #include "gc/shenandoah/shenandoahConcurrentMark.inline.hpp"
 #include "gc/shenandoah/shenandoahMarkCompact.hpp"
 #include "gc/shenandoah/shenandoahHeap.inline.hpp"
-#include "gc/shenandoah/shenandoahRootProcessor.hpp"
+#include "gc/shenandoah/shenandoahRootProcessor.inline.hpp"
 #include "gc/shenandoah/shenandoahOopClosures.inline.hpp"
 #include "gc/shenandoah/shenandoahTaskqueue.inline.hpp"
 #include "gc/shenandoah/shenandoahTimingTracker.hpp"
@@ -124,7 +124,7 @@
 
     ResourceMark m;
     if (heap->unload_classes()) {
-      _rp->process_strong_roots(oops, &clds_cl, NULL, &blobs_cl, NULL, worker_id);
+      _rp->process_strong_roots(oops, &clds_cl, &blobs_cl, NULL, worker_id);
     } else {
       if (ShenandoahConcurrentScanCodeRoots) {
         CodeBlobClosure* code_blobs = NULL;
@@ -177,7 +177,7 @@
         DEBUG_ONLY(&assert_to_space)
         NOT_DEBUG(NULL);
     }
-    _rp->update_all_roots(&cl, &cldCl, code_blobs, NULL, worker_id);
+    _rp->update_all_roots<AlwaysTrueClosure>(&cl, &cldCl, code_blobs, NULL, worker_id);
   }
 };
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp b/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp
@@ -35,7 +35,7 @@
 #include "gc/shenandoah/shenandoahHeap.inline.hpp"
 #include "gc/shenandoah/shenandoahHeuristics.hpp"
 #include "gc/shenandoah/shenandoahMarkingContext.inline.hpp"
-#include "gc/shenandoah/shenandoahRootProcessor.hpp"
+#include "gc/shenandoah/shenandoahRootProcessor.inline.hpp"
 #include "gc/shenandoah/shenandoahTraversalGC.hpp"
 #include "gc/shenandoah/shenandoahTaskqueue.inline.hpp"
 #include "gc/shenandoah/shenandoahUtils.hpp"
@@ -575,9 +575,9 @@
     MarkingCodeBlobClosure adjust_code_closure(&cl,
                                              CodeBlobToOopClosure::FixRelocations);
 
-    _rp->update_all_roots(&cl,
-                          &adjust_cld_closure,
-                          &adjust_code_closure, NULL, worker_id);
+    _rp->update_all_roots<AlwaysTrueClosure>(&cl,
+                                             &adjust_cld_closure,
+                                             &adjust_code_closure, NULL, worker_id);
   }
 };
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
@@ -88,12 +88,11 @@
 
 void ShenandoahRootProcessor::process_strong_roots(OopClosure* oops,
                                                    CLDClosure* clds,
-                                                   CLDClosure* weak_clds,
                                                    CodeBlobClosure* blobs,
                                                    ThreadClosure* thread_cl,
                                                    uint worker_id) {
 
-  process_java_roots(oops, clds, weak_clds, blobs, thread_cl, worker_id);
+  process_java_roots(oops, clds, NULL, blobs, thread_cl, worker_id);
   process_vm_roots(oops, worker_id);
 
   _process_strong_tasks->all_tasks_completed(n_workers());
@@ -125,47 +124,6 @@
 
 }
 
-void ShenandoahRootProcessor::update_all_roots(OopClosure* oops,
-                                               CLDClosure* clds,
-                                               CodeBlobClosure* blobs,
-                                               ThreadClosure* thread_cl,
-                                               uint worker_id) {
-  ShenandoahWorkerTimings* worker_times = ShenandoahHeap::heap()->phase_timings()->worker_times();
-  AlwaysTrueClosure always_true;
-
-  if (!_process_strong_tasks->is_task_claimed(SHENANDOAH_RP_PS_JNIHandles_weak_oops_do)) {
-    ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::JNIWeakRoots, worker_id);
-    WeakProcessor::weak_oops_do(&always_true, oops);
-  }
-
-  process_all_roots(oops, clds, blobs, thread_cl, worker_id);
-
-  if (ShenandoahStringDedup::is_enabled()) {
-    ShenandoahStringDedup::parallel_oops_do(&always_true, oops, worker_id);
-  }
-}
-
-void ShenandoahRootProcessor::traversal_update_all_roots(OopClosure* oops,
-                                                         CLDClosure* clds,
-                                                         CodeBlobClosure* blobs,
-                                                         ThreadClosure* thread_cl,
-                                                         uint worker_id) {
-  ShenandoahWorkerTimings* worker_times = ShenandoahHeap::heap()->phase_timings()->worker_times();
-  ShenandoahIsAliveSelector is_alive;
-  BoolObjectClosure* is_alive_closure = is_alive.is_alive_closure();
-
-  if (!_process_strong_tasks->is_task_claimed(SHENANDOAH_RP_PS_JNIHandles_weak_oops_do)) {
-    ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::JNIWeakRoots, worker_id);
-    WeakProcessor::weak_oops_do(is_alive_closure, oops);
-  }
-
-  process_all_roots(oops, clds, blobs, thread_cl, worker_id);
-
-  if (ShenandoahStringDedup::is_enabled()) {
-    ShenandoahStringDedup::parallel_oops_do(is_alive_closure, oops, worker_id);
-  }
-}
-
 class ShenandoahParallelOopsDoThreadClosure : public ThreadClosure {
 private:
   OopClosure* _f;
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp
@@ -81,7 +81,6 @@
   // for the particular GC cycles.
   void process_strong_roots(OopClosure* oops,
                             CLDClosure* clds,
-                            CLDClosure* weak_clds,
                             CodeBlobClosure* blobs,
                             ThreadClosure* thread_cl,
                             uint worker_id);
@@ -94,22 +93,13 @@
                          uint worker_id);
 
   // Apply oops, clds and blobs to strongly and weakly reachable roots in the system
+  template <typename IsAlive>
   void update_all_roots(OopClosure* oops,
                         CLDClosure* clds,
                         CodeBlobClosure* blobs,
                         ThreadClosure* thread_cl,
                         uint worker_id);
 
-
-  // Apply oops, clds and blobs to strongly and weakly reachable roots in the system
-  // during traversal GC.
-  // It cleans up and updates weak roots in one iteration.
-  void traversal_update_all_roots(OopClosure* oops,
-                                  CLDClosure* clds,
-                                  CodeBlobClosure* blobs,
-                                  ThreadClosure* thread_cl,
-                                  uint worker_id);
-
   // For slow debug/verification code
   void process_all_roots_slow(OopClosure* oops);
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
@@ -39,7 +39,7 @@
 #include "gc/shenandoah/shenandoahHeuristics.hpp"
 #include "gc/shenandoah/shenandoahMarkingContext.inline.hpp"
 #include "gc/shenandoah/shenandoahOopClosures.inline.hpp"
-#include "gc/shenandoah/shenandoahRootProcessor.hpp"
+#include "gc/shenandoah/shenandoahRootProcessor.inline.hpp"
 #include "gc/shenandoah/shenandoahStringDedup.hpp"
 #include "gc/shenandoah/shenandoahTaskqueue.inline.hpp"
 #include "gc/shenandoah/shenandoahTimingTracker.hpp"
@@ -192,7 +192,7 @@
       ShenandoahMarkCLDClosure cld_cl(&roots_cl);
       MarkingCodeBlobClosure code_cl(&roots_cl, CodeBlobToOopClosure::FixRelocations);
       if (unload_classes) {
-        _rp->process_strong_roots(&roots_cl, &cld_cl, NULL, NULL, NULL, worker_id);
+        _rp->process_strong_roots(&roots_cl, &cld_cl, NULL, NULL, worker_id);
         // Need to pre-evac code roots here. Otherwise we might see from-space constants.
         ShenandoahWorkerTimings* worker_times = _heap->phase_timings()->worker_times();
         ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::CodeCacheRoots, worker_id);
@@ -273,8 +273,8 @@
       CLDToOopClosure cld_cl(&roots_cl);
       ShenandoahTraversalSATBThreadsClosure tc(&satb_cl);
       if (unload_classes) {
-        ShenandoahRemarkCLDClosure weak_cld_cl(&roots_cl);
-        _rp->process_strong_roots(&roots_cl, &cld_cl, &weak_cld_cl, NULL, &tc, worker_id);
+        ShenandoahRemarkCLDClosure remark_cld_cl(&roots_cl);
+        _rp->process_strong_roots(&roots_cl, &remark_cld_cl, NULL, &tc, worker_id);
       } else {
         _rp->process_all_roots(&roots_cl, &cld_cl, NULL, &tc, worker_id);
       }
@@ -283,8 +283,8 @@
       CLDToOopClosure cld_cl(&roots_cl);
       ShenandoahTraversalSATBThreadsClosure tc(&satb_cl);
       if (unload_classes) {
-        ShenandoahRemarkCLDClosure weak_cld_cl(&roots_cl);
-        _rp->process_strong_roots(&roots_cl, &cld_cl, &weak_cld_cl, NULL, &tc, worker_id);
+        ShenandoahRemarkCLDClosure remark_cld_cl(&roots_cl);
+        _rp->process_strong_roots(&roots_cl, &remark_cld_cl, NULL, &tc, worker_id);
       } else {
         _rp->process_all_roots(&roots_cl, &cld_cl, NULL, &tc, worker_id);
       }
@@ -705,14 +705,16 @@
 public:
   ShenandoahTraversalFixRootsTask(ShenandoahRootProcessor* rp) :
     AbstractGangTask("Shenandoah traversal fix roots"),
-    _rp(rp) {}
+    _rp(rp) {
+    assert(ShenandoahHeap::heap()->has_forwarded_objects(), "Must be");
+  }
 
   void work(uint worker_id) {
     ShenandoahParallelWorkerSession worker_session(worker_id);
     ShenandoahTraversalFixRootsClosure cl;
     MarkingCodeBlobClosure blobsCl(&cl, CodeBlobToOopClosure::FixRelocations);
     CLDToOopClosure cldCl(&cl);
-    _rp->traversal_update_all_roots(&cl, &cldCl, &blobsCl, NULL, worker_id);
+    _rp->update_all_roots<ShenandoahForwardedIsAliveClosure>(&cl, &cldCl, &blobsCl, NULL, worker_id);
   }
 };