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

rev 52753 : [backport] 8221435: Shenandoah should not mark through weak roots
Reviewed-by: rkennke, shade
rev 52754 : [backport] 8221629: Shenandoah: Cleanup class unloading logic
Reviewed-by: rkennke

@@ -35,11 +35,11 @@
 #include "gc/shenandoah/shenandoahBarrierSet.inline.hpp"
 #include "gc/shenandoah/shenandoahClosures.inline.hpp"
 #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"
 #include "gc/shenandoah/shenandoahUtils.hpp"
 #include "gc/shared/weakProcessor.hpp"

@@ -119,15 +119,14 @@
     //      and instead do that in concurrent phase under the relevant lock. This saves init mark
     //      pause time.
 
     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, &blobs_cl, NULL, worker_id);
     } else {
       if (ShenandoahConcurrentScanCodeRoots) {
         CodeBlobClosure* code_blobs = NULL;
 #ifdef ASSERT
         ShenandoahAssertToSpaceClosure assert_to_space_oops;

@@ -136,13 +135,13 @@
         // Otherwise, it should have to-space ptrs only if mark does not update refs.
         if (!heap->has_forwarded_objects()) {
           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);
       }
     }
   }
 };
 

@@ -176,11 +175,11 @@
     } else {
       code_blobs =
         DEBUG_ONLY(&assert_to_space)
         NOT_DEBUG(NULL);
     }
-    _rp->process_all_roots(&cl, &cl, &cldCl, code_blobs, NULL, worker_id);
+    _rp->update_all_roots<AlwaysTrueClosure>(&cl, &cldCl, code_blobs, NULL, worker_id);
   }
 };
 
 class ShenandoahConcurrentMarkingTask : public AbstractGangTask {
 private:

@@ -454,15 +453,21 @@
   // When we're done marking everything, we process weak references.
   if (_heap->process_references()) {
     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());
 
   // Resize Metaspace

@@ -563,15 +568,17 @@
 
 class ShenandoahWeakAssertNotForwardedClosure : public OopClosure {
 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:
   ShenandoahWeakAssertNotForwardedClosure() {}
 

@@ -656,10 +663,26 @@
   rp->verify_no_references_recorded();
   assert(!rp->discovery_enabled(), "Post condition");
 
 }
 
+// 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();
 
   ShenandoahPhaseTimings::Phase phase_process =
           full_gc ?

@@ -697,30 +720,22 @@
 
   {
     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();
 
     assert(task_queues()->is_empty(), "Should be empty");