8227866: Shenandoah: Split weak root processing and class unloading in parallel cleaning task

Reviewed-by: rkennke

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

@@ -446,22 +446,11 @@ void ShenandoahConcurrentMark::finish_mark_from_roots(bool full_gc) {
     weak_refs_work(full_gc);
   }
 
-  weak_roots_work();
+  _heap->parallel_cleaning(full_gc);
 
-  // And finally finish class unloading
-  if (_heap->unload_classes()) {
-    _heap->unload_classes_and_cleanup_tables(full_gc);
-  } else 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
-  MetaspaceGC::compute_new_size();
 }
 
 // Weak Reference Closures
@@ -556,26 +545,6 @@ public:
   void do_oop(oop* p)       { do_oop_work(p); }
 };
 
-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() {}
-
-  void do_oop(narrowOop* p) { do_oop_work(p); }
-  void do_oop(oop* p)       { do_oop_work(p); }
-};
-
 class ShenandoahRefProcTaskProxy : public AbstractGangTask {
 private:
   AbstractRefProcTaskExecutor::ProcessTask& _proc_task;
@@ -655,21 +624,6 @@ void ShenandoahConcurrentMark::weak_refs_work(bool full_gc) {
 
 }
 
-// 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() {
-  WorkGang* workers = _heap->workers();
-  OopClosure* keep_alive = &do_nothing_cl;
-#ifdef ASSERT
-  ShenandoahWeakAssertNotForwardedClosure verify_cl;
-  keep_alive = &verify_cl;
-#endif
-  ShenandoahIsAliveClosure is_alive;
-  WeakProcessor::weak_oops_do(workers, &is_alive, keep_alive, 1);
-}
-
 void ShenandoahConcurrentMark::weak_refs_work_doit(bool full_gc) {
   ReferenceProcessor* rp = _heap->ref_processor();
 

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

@@ -87,8 +87,6 @@ private:
   void weak_refs_work(bool full_gc);
   void weak_refs_work_doit(bool full_gc);
 
-  void weak_roots_work();
-
 public:
   void preclean_weak_refs();
 

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

@@ -29,7 +29,6 @@
 #include "gc/shared/gcTimer.hpp"
 #include "gc/shared/gcTraceTime.inline.hpp"
 #include "gc/shared/memAllocator.hpp"
-#include "gc/shared/parallelCleaning.hpp"
 #include "gc/shared/plab.hpp"
 
 #include "gc/shenandoah/shenandoahAllocTracker.hpp"
@@ -53,6 +52,7 @@
 #include "gc/shenandoah/shenandoahNormalMode.hpp"
 #include "gc/shenandoah/shenandoahOopClosures.inline.hpp"
 #include "gc/shenandoah/shenandoahPacer.inline.hpp"
+#include "gc/shenandoah/shenandoahParallelCleaning.inline.hpp"
 #include "gc/shenandoah/shenandoahPassiveMode.hpp"
 #include "gc/shenandoah/shenandoahRootProcessor.inline.hpp"
 #include "gc/shenandoah/shenandoahStringDedup.hpp"
@@ -1950,16 +1950,8 @@ void ShenandoahHeap::stop() {
   }
 }
 
-void ShenandoahHeap::unload_classes_and_cleanup_tables(bool full_gc) {
-  assert(heuristics()->can_unload_classes(), "Class unloading should be enabled");
-
-  ShenandoahGCPhase root_phase(full_gc ?
-                               ShenandoahPhaseTimings::full_gc_purge :
-                               ShenandoahPhaseTimings::purge);
-
-  ShenandoahIsAliveSelector alive;
-  BoolObjectClosure* is_alive = alive.is_alive_closure();
-
+void ShenandoahHeap::stw_unload_classes(bool full_gc) {
+  if (!unload_classes()) return;
   bool purged_class;
 
   // Unload classes and purge SystemDictionary.
@@ -1974,17 +1966,61 @@ void ShenandoahHeap::unload_classes_and_cleanup_tables(bool full_gc) {
     ShenandoahGCPhase phase(full_gc ?
                             ShenandoahPhaseTimings::full_gc_purge_par :
                             ShenandoahPhaseTimings::purge_par);
-    uint active = _workers->active_workers();
-    ParallelCleaningTask unlink_task(is_alive, active, purged_class, true);
+    ShenandoahIsAliveSelector is_alive;
+    uint num_workers = _workers->active_workers();
+    ShenandoahClassUnloadingTask unlink_task(is_alive.is_alive_closure(), num_workers, purged_class);
     _workers->run_task(&unlink_task);
   }
 
   {
     ShenandoahGCPhase phase(full_gc ?
-                      ShenandoahPhaseTimings::full_gc_purge_cldg :
-                      ShenandoahPhaseTimings::purge_cldg);
+                            ShenandoahPhaseTimings::full_gc_purge_cldg :
+                            ShenandoahPhaseTimings::purge_cldg);
     ClassLoaderDataGraph::purge();
   }
+  // Resize and verify metaspace
+  MetaspaceGC::compute_new_size();
+  MetaspaceUtils::verify_metrics();
+}
+
+// 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 ShenandoahHeap::stw_process_weak_roots(bool full_gc) {
+  ShenandoahGCPhase root_phase(full_gc ?
+                               ShenandoahPhaseTimings::full_gc_purge :
+                               ShenandoahPhaseTimings::purge);
+  uint num_workers = _workers->active_workers();
+  ShenandoahPhaseTimings::Phase timing_phase = full_gc ?
+                                               ShenandoahPhaseTimings::full_gc_purge_par :
+                                               ShenandoahPhaseTimings::purge_par;
+  // Cleanup weak roots
+  ShenandoahGCPhase phase(timing_phase);
+  if (has_forwarded_objects()) {
+    ShenandoahForwardedIsAliveClosure is_alive;
+    ShenandoahUpdateRefsClosure keep_alive;
+    ShenandoahParallelWeakRootsCleaningTask<ShenandoahForwardedIsAliveClosure, ShenandoahUpdateRefsClosure>
+      cleaning_task(&is_alive, &keep_alive, num_workers);
+    _workers->run_task(&cleaning_task);
+  } else {
+    ShenandoahIsAliveClosure is_alive;
+#ifdef ASSERT
+  ShenandoahAssertNotForwardedClosure verify_cl;
+  ShenandoahParallelWeakRootsCleaningTask<ShenandoahIsAliveClosure, ShenandoahAssertNotForwardedClosure>
+    cleaning_task(&is_alive, &verify_cl, num_workers);
+#else
+  ShenandoahParallelWeakRootsCleaningTask<ShenandoahIsAliveClosure, DoNothingClosure>
+    cleaning_task(&is_alive, &do_nothing_cl, num_workers);
+#endif
+    _workers->run_task(&cleaning_task);
+  }
+}
+
+void ShenandoahHeap::parallel_cleaning(bool full_gc) {
+  assert(SafepointSynchronize::is_at_safepoint(), "Must be at a safepoint");
+  stw_process_weak_roots(full_gc);
+  stw_unload_classes(full_gc);
 }
 
 void ShenandoahHeap::set_has_forwarded_objects(bool cond) {

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

@@ -512,9 +512,12 @@ public:
   void set_unload_classes(bool uc);
   bool unload_classes() const;
 
-  // Delete entries for dead interned string and clean up unreferenced symbols
-  // in symbol table, possibly in parallel.
-  void unload_classes_and_cleanup_tables(bool full_gc);
+  // Perform STW class unloading and weak root cleaning
+  void parallel_cleaning(bool full_gc);
+
+private:
+  void stw_unload_classes(bool full_gc);
+  void stw_process_weak_roots(bool full_gc);
 
 // ---------- Generic interface hooks
 // Minor things that super-interface expects us to implement to play nice with

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

@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 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
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+
+#include "precompiled.hpp"
+
+#include "gc/shenandoah/shenandoahClosures.inline.hpp"
+#include "gc/shenandoah/shenandoahCodeRoots.hpp"
+#include "gc/shenandoah/shenandoahEvacOOMHandler.hpp"
+#include "gc/shenandoah/shenandoahParallelCleaning.hpp"
+#include "runtime/safepoint.hpp"
+
+ShenandoahClassUnloadingTask::ShenandoahClassUnloadingTask(BoolObjectClosure* is_alive,
+                                                           uint num_workers,
+                                                           bool unloading_occurred) :
+  AbstractGangTask("Parallel Class Unloading Task"),
+  _unloading_occurred(unloading_occurred),
+  _code_cache_task(num_workers, is_alive, unloading_occurred),
+  _klass_cleaning_task() {
+  assert(SafepointSynchronize::is_at_safepoint(), "Must be at a safepoint");
+}
+
+void ShenandoahClassUnloadingTask::work(uint worker_id) {
+  ShenandoahEvacOOMScope scope;
+  _code_cache_task.work(worker_id);
+  // Clean all klasses that were not unloaded.
+  // The weak metadata in klass doesn't need to be
+  // processed if there was no unloading.
+  if (_unloading_occurred) {
+    _klass_cleaning_task.work();
+  }
+}

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

@@ -0,0 +1,61 @@
+/*
+ * Copyright (c) 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
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#ifndef SHARE_GC_SHENANDOAH_SHENANDOAHPARALLELCLEANING_HPP
+#define SHARE_GC_SHENANDOAH_SHENANDOAHPARALLELCLEANING_HPP
+
+#include "gc/shared/parallelCleaning.hpp"
+#include "gc/shared/weakProcessor.hpp"
+#include "gc/shared/weakProcessorPhaseTimes.hpp"
+#include "gc/shared/workgroup.hpp"
+#include "memory/iterator.hpp"
+
+// Perform weak root cleaning at a pause
+template <typename IsAlive, typename KeepAlive>
+class ShenandoahParallelWeakRootsCleaningTask : public AbstractGangTask {
+protected:
+  WeakProcessor::Task     _weak_processing_task;
+  IsAlive*                _is_alive;
+  KeepAlive*              _keep_alive;
+public:
+  ShenandoahParallelWeakRootsCleaningTask(IsAlive* is_alive, KeepAlive* keep_alive, uint num_workers);
+  ~ShenandoahParallelWeakRootsCleaningTask();
+
+  void work(uint worker_id);
+};
+
+// Perform class unloading at a pause
+class ShenandoahClassUnloadingTask : public AbstractGangTask {
+private:
+  bool                            _unloading_occurred;
+  CodeCacheUnloadingTask          _code_cache_task;
+  KlassCleaningTask               _klass_cleaning_task;
+public:
+  ShenandoahClassUnloadingTask(BoolObjectClosure* is_alive,
+                               uint num_workers,
+                               bool unloading_occurred);
+
+  void work(uint worker_id);
+};
+
+#endif // SHARE_GC_SHENANDOAH_SHENANDOAHPARALLELCLEANING_HPP

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

@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 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
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#ifndef SHARE_GC_SHENANDOAH_SHENANDOAHPARALLELCLEANING_INLINE_HPP
+#define SHARE_GC_SHENANDOAH_SHENANDOAHPARALLELCLEANING_INLINE_HPP
+
+#include "gc/shared/weakProcessor.inline.hpp"
+#include "gc/shenandoah/shenandoahHeap.hpp"
+#include "gc/shenandoah/shenandoahParallelCleaning.hpp"
+#include "gc/shenandoah/shenandoahUtils.hpp"
+#include "runtime/thread.hpp"
+#include "runtime/safepoint.hpp"
+
+template<typename IsAlive, typename KeepAlive>
+ShenandoahParallelWeakRootsCleaningTask<IsAlive, KeepAlive>::ShenandoahParallelWeakRootsCleaningTask(IsAlive* is_alive,
+                                                                                                     KeepAlive* keep_alive,
+                                                                                                     uint num_workers) :
+  AbstractGangTask("Parallel Weak Root Cleaning Task"),
+  _weak_processing_task(num_workers),
+  _is_alive(is_alive), _keep_alive(keep_alive) {
+  assert(SafepointSynchronize::is_at_safepoint(), "Must be at a safepoint");
+
+  if (ShenandoahStringDedup::is_enabled()) {
+    StringDedup::gc_prologue(false);
+  }
+}
+
+template<typename IsAlive, typename KeepAlive>
+ShenandoahParallelWeakRootsCleaningTask<IsAlive, KeepAlive>::~ShenandoahParallelWeakRootsCleaningTask() {
+  if (StringDedup::is_enabled()) {
+    StringDedup::gc_epilogue();
+  }
+}
+
+template<typename IsAlive, typename KeepAlive>
+void ShenandoahParallelWeakRootsCleaningTask<IsAlive, KeepAlive>::work(uint worker_id) {
+  _weak_processing_task.work<IsAlive, KeepAlive>(worker_id, _is_alive, _keep_alive);
+
+  if (ShenandoahStringDedup::is_enabled()) {
+    ShenandoahStringDedup::parallel_oops_do(_is_alive, _keep_alive, worker_id);
+  }
+}
+
+#endif // SHARE_GC_SHENANDOAH_SHENANDOAHPARALLELCLEANING_INLINE_HPP

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

@@ -75,16 +75,20 @@ void ShenandoahStringDedup::parallel_oops_do(BoolObjectClosure* is_alive, OopClo
   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(is_alive, cl);
+  if (ShenandoahGCPhase::is_root_work_phase()) {
+    ShenandoahWorkerTimings* worker_times = ShenandoahHeap::heap()->phase_timings()->worker_times();
+    {
+      ShenandoahWorkerTimingsTracker x(worker_times, ShenandoahPhaseTimings::StringDedupQueueRoots, worker_id);
+      StringDedupQueue::unlink_or_oops_do(&sd_cl);
+    }
 
-  {
-    ShenandoahWorkerTimingsTracker x(worker_times, ShenandoahPhaseTimings::StringDedupQueueRoots, worker_id);
+    {
+      ShenandoahWorkerTimingsTracker x(worker_times, ShenandoahPhaseTimings::StringDedupTableRoots, worker_id);
+      StringDedupTable::unlink_or_oops_do(&sd_cl, worker_id);
+    }
+  } else {
     StringDedupQueue::unlink_or_oops_do(&sd_cl);
-  }
-  {
-    ShenandoahWorkerTimingsTracker x(worker_times, ShenandoahPhaseTimings::StringDedupTableRoots, worker_id);
     StringDedupTable::unlink_or_oops_do(&sd_cl, worker_id);
   }
 }

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

@@ -28,7 +28,6 @@
 #include "gc/shared/referenceProcessor.hpp"
 #include "gc/shared/referenceProcessorPhaseTimes.hpp"
 #include "gc/shared/workgroup.hpp"
-#include "gc/shared/weakProcessor.inline.hpp"
 #include "gc/shenandoah/shenandoahBarrierSet.hpp"
 #include "gc/shenandoah/shenandoahClosures.inline.hpp"
 #include "gc/shenandoah/shenandoahCodeRoots.hpp"
@@ -596,9 +595,7 @@ void ShenandoahTraversalGC::final_traversal_collection() {
 
   if (!_heap->cancelled_gc()) {
     fixup_roots();
-    if (_heap->unload_classes()) {
-      _heap->unload_classes_and_cleanup_tables(false);
-    }
+    _heap->parallel_cleaning(false);
   }
 
   if (!_heap->cancelled_gc()) {