From 9d230ea87dac793a10b256aa773d7fa2262057f0 Mon Sep 17 00:00:00 2001
From: Stefan Karlsson <stefank@openjdk.org>
Date: Tue, 13 Oct 2020 09:05:52 +0000
Subject: [PATCH] 8254562: ZGC: Remove ZMarkRootsTask

Reviewed-by: pliden
---
 src/hotspot/share/gc/z/zHeapIterator.cpp  |  2 --
 src/hotspot/share/gc/z/zHeapIterator.hpp  |  1 -
 src/hotspot/share/gc/z/zMark.cpp          | 38 -----------------------
 src/hotspot/share/gc/z/zRelocate.cpp      | 14 +++++----
 src/hotspot/share/gc/z/zRootsIterator.cpp | 16 +---------
 src/hotspot/share/gc/z/zRootsIterator.hpp | 13 ++------
 src/hotspot/share/gc/z/zVerify.cpp        |  5 ---
 src/hotspot/share/gc/z/zVerify.hpp        |  1 -
 src/hotspot/share/gc/z/zWorkers.cpp       |  4 +++
 src/hotspot/share/gc/z/zWorkers.hpp       |  1 +
 10 files changed, 16 insertions(+), 79 deletions(-)

diff --git a/src/hotspot/share/gc/z/zHeapIterator.cpp b/src/hotspot/share/gc/z/zHeapIterator.cpp
index fbc8c6a9503..c199638dd74 100644
--- a/src/hotspot/share/gc/z/zHeapIterator.cpp
+++ b/src/hotspot/share/gc/z/zHeapIterator.cpp
@@ -176,7 +176,6 @@ ZHeapIterator::ZHeapIterator(uint nworkers, bool visit_weaks) :
     _bitmaps_lock(),
     _queues(nworkers),
     _array_queues(nworkers),
-    _roots(),
     _concurrent_roots(),
     _weak_roots(),
     _concurrent_weak_roots(),
@@ -341,7 +340,6 @@ void ZHeapIterator::drain_and_steal(const ZHeapIteratorContext& context, ObjectC
 
 template <bool VisitWeaks>
 void ZHeapIterator::object_iterate_inner(const ZHeapIteratorContext& context, ObjectClosure* cl) {
-  push_roots<false /* Concurrent */, false /* Weak */>(context, _roots);
   push_roots<true  /* Concurrent */, false /* Weak */>(context, _concurrent_roots);
   if (VisitWeaks) {
     push_roots<false /* Concurrent */, true  /* Weak */>(context, _weak_roots);
diff --git a/src/hotspot/share/gc/z/zHeapIterator.hpp b/src/hotspot/share/gc/z/zHeapIterator.hpp
index 2a5cdf9631e..598cb7d7c59 100644
--- a/src/hotspot/share/gc/z/zHeapIterator.hpp
+++ b/src/hotspot/share/gc/z/zHeapIterator.hpp
@@ -52,7 +52,6 @@ private:
   ZLock                              _bitmaps_lock;
   ZHeapIteratorQueues                _queues;
   ZHeapIteratorArrayQueues           _array_queues;
-  ZRootsIterator                     _roots;
   ZConcurrentRootsIteratorClaimOther _concurrent_roots;
   ZWeakRootsIterator                 _weak_roots;
   ZConcurrentWeakRootsIterator       _concurrent_weak_roots;
diff --git a/src/hotspot/share/gc/z/zMark.cpp b/src/hotspot/share/gc/z/zMark.cpp
index bb7d52f904e..6806be9af7e 100644
--- a/src/hotspot/share/gc/z/zMark.cpp
+++ b/src/hotspot/share/gc/z/zMark.cpp
@@ -123,40 +123,6 @@ void ZMark::prepare_mark() {
   }
 }
 
-class ZMarkRootsIteratorClosure : public ZRootsIteratorClosure {
-public:
-  virtual void do_oop(oop* p) {
-    ZBarrier::mark_barrier_on_root_oop_field(p);
-  }
-
-  virtual void do_oop(narrowOop* p) {
-    ShouldNotReachHere();
-  }
-};
-
-class ZMarkRootsTask : public ZTask {
-private:
-  ZMark* const              _mark;
-  ZRootsIterator            _roots;
-  ZMarkRootsIteratorClosure _cl;
-
-public:
-  ZMarkRootsTask(ZMark* mark) :
-      ZTask("ZMarkRootsTask"),
-      _mark(mark),
-      _roots(false /* visit_jvmti_weak_export */) {}
-
-  virtual void work() {
-    _roots.oops_do(&_cl);
-
-    // Flush and free worker stacks. Needed here since
-    // the set of workers executing during root scanning
-    // can be different from the set of workers executing
-    // during mark.
-    _mark->flush_and_free();
-  }
-};
-
 void ZMark::start() {
   // Verification
   if (ZVerifyMarking) {
@@ -165,10 +131,6 @@ void ZMark::start() {
 
   // Prepare for concurrent mark
   prepare_mark();
-
-  // Mark roots
-  ZMarkRootsTask task(this);
-  _workers->run_parallel(&task);
 }
 
 void ZMark::prepare_work() {
diff --git a/src/hotspot/share/gc/z/zRelocate.cpp b/src/hotspot/share/gc/z/zRelocate.cpp
index d67871f213b..d8432909730 100644
--- a/src/hotspot/share/gc/z/zRelocate.cpp
+++ b/src/hotspot/share/gc/z/zRelocate.cpp
@@ -43,7 +43,7 @@ static const ZStatCounter ZCounterRelocationContention("Contention", "Relocation
 ZRelocate::ZRelocate(ZWorkers* workers) :
     _workers(workers) {}
 
-class ZRelocateRootsIteratorClosure : public ZRootsIteratorClosure {
+class ZRelocateRootsIteratorClosure : public OopClosure {
 public:
   virtual void do_oop(oop* p) {
     ZBarrier::relocate_barrier_on_root_oop_field(p);
@@ -56,24 +56,26 @@ public:
 
 class ZRelocateRootsTask : public ZTask {
 private:
-  ZRootsIterator                _roots;
   ZRelocateRootsIteratorClosure _cl;
 
 public:
   ZRelocateRootsTask() :
-      ZTask("ZRelocateRootsTask"),
-      _roots(true /* visit_jvmti_weak_export */) {}
+      ZTask("ZRelocateRootsTask") {}
 
   virtual void work() {
+    // Allocation path assumes that relocating GC threads are ZWorkers
+    assert(ZThread::is_worker(), "Relocation code needs to be run as a worker");
+    assert(ZThread::worker_id() == 0, "No multi-thread support");
+
     // During relocation we need to visit the JVMTI
     // export weak roots to rehash the JVMTI tag map
-    _roots.oops_do(&_cl);
+    ZRelocateRoots::oops_do(&_cl);
   }
 };
 
 void ZRelocate::start() {
   ZRelocateRootsTask task;
-  _workers->run_parallel(&task);
+  _workers->run_serial(&task);
 }
 
 uintptr_t ZRelocate::relocate_object_inner(ZForwarding* forwarding, uintptr_t from_index, uintptr_t from_offset) const {
diff --git a/src/hotspot/share/gc/z/zRootsIterator.cpp b/src/hotspot/share/gc/z/zRootsIterator.cpp
index f6a03b22e25..ac17f4bf14b 100644
--- a/src/hotspot/share/gc/z/zRootsIterator.cpp
+++ b/src/hotspot/share/gc/z/zRootsIterator.cpp
@@ -54,7 +54,6 @@
 #include "runtime/vmThread.hpp"
 #include "utilities/debug.hpp"
 
-static const ZStatSubPhase ZSubPhasePauseRoots("Pause Roots");
 static const ZStatSubPhase ZSubPhasePauseRootsJVMTIWeakExport("Pause Roots JVMTIWeakExport");
 
 static const ZStatSubPhase ZSubPhaseConcurrentRootsSetup("Concurrent Roots Setup");
@@ -141,25 +140,12 @@ void ZJavaThreadsIterator::threads_do(ThreadClosure* cl) {
   }
 }
 
-ZRootsIterator::ZRootsIterator(bool visit_jvmti_weak_export) :
-    _visit_jvmti_weak_export(visit_jvmti_weak_export),
-    _jvmti_weak_export(this) {
-  assert(SafepointSynchronize::is_at_safepoint(), "Should be at safepoint");
-}
-
-void ZRootsIterator::do_jvmti_weak_export(ZRootsIteratorClosure* cl) {
+void ZRelocateRoots::oops_do(OopClosure* cl) {
   ZStatTimer timer(ZSubPhasePauseRootsJVMTIWeakExport);
   AlwaysTrueClosure always_alive;
   JvmtiExport::weak_oops_do(&always_alive, cl);
 }
 
-void ZRootsIterator::oops_do(ZRootsIteratorClosure* cl) {
-  ZStatTimer timer(ZSubPhasePauseRoots);
-  if (_visit_jvmti_weak_export) {
-    _jvmti_weak_export.oops_do(cl);
-  }
-}
-
 ZConcurrentRootsIterator::ZConcurrentRootsIterator(int cld_claim) :
     _oop_storage_set_iter(),
     _java_threads_iter(),
diff --git a/src/hotspot/share/gc/z/zRootsIterator.hpp b/src/hotspot/share/gc/z/zRootsIterator.hpp
index 309b0a22578..36a864be6e5 100644
--- a/src/hotspot/share/gc/z/zRootsIterator.hpp
+++ b/src/hotspot/share/gc/z/zRootsIterator.hpp
@@ -105,18 +105,9 @@ public:
   void threads_do(ThreadClosure* cl);
 };
 
-class ZRootsIterator {
-private:
-  const bool _visit_jvmti_weak_export;
-
-  void do_jvmti_weak_export(ZRootsIteratorClosure* cl);
-
-  ZSerialOopsDo<ZRootsIterator, &ZRootsIterator::do_jvmti_weak_export>   _jvmti_weak_export;
-
+class ZRelocateRoots : public AllStatic {
 public:
-  ZRootsIterator(bool visit_jvmti_weak_export = false);
-
-  void oops_do(ZRootsIteratorClosure* cl);
+  static void oops_do(OopClosure* cl);
 };
 
 class ZConcurrentRootsIterator {
diff --git a/src/hotspot/share/gc/z/zVerify.cpp b/src/hotspot/share/gc/z/zVerify.cpp
index a94e3264e49..1a7cb231ad0 100644
--- a/src/hotspot/share/gc/z/zVerify.cpp
+++ b/src/hotspot/share/gc/z/zVerify.cpp
@@ -229,10 +229,6 @@ void ZVerify::roots(bool verify_fixed) {
   }
 }
 
-void ZVerify::roots_strong() {
-  roots<ZRootsIterator>(true /* verify_fixed */);
-}
-
 void ZVerify::roots_weak() {
   roots<ZWeakRootsIterator>(true /* verify_fixed */);
 }
@@ -246,7 +242,6 @@ void ZVerify::roots_concurrent_weak() {
 }
 
 void ZVerify::roots(bool verify_concurrent_strong, bool verify_weaks) {
-  roots_strong();
   roots_concurrent_strong(verify_concurrent_strong);
   if (verify_weaks) {
     roots_weak();
diff --git a/src/hotspot/share/gc/z/zVerify.hpp b/src/hotspot/share/gc/z/zVerify.hpp
index 3100a16ad77..4d405b957bb 100644
--- a/src/hotspot/share/gc/z/zVerify.hpp
+++ b/src/hotspot/share/gc/z/zVerify.hpp
@@ -33,7 +33,6 @@ class ZVerify : public AllStatic {
 private:
   template <typename RootsIterator> static void roots(bool verify_fixed);
 
-  static void roots_strong();
   static void roots_weak();
   static void roots_concurrent_strong(bool verify_fixed);
   static void roots_concurrent_weak();
diff --git a/src/hotspot/share/gc/z/zWorkers.cpp b/src/hotspot/share/gc/z/zWorkers.cpp
index f3c8cb7c176..91f957ab435 100644
--- a/src/hotspot/share/gc/z/zWorkers.cpp
+++ b/src/hotspot/share/gc/z/zWorkers.cpp
@@ -101,6 +101,10 @@ void ZWorkers::run(ZTask* task, uint nworkers) {
   _workers.run_task(task->gang_task());
 }
 
+void ZWorkers::run_serial(ZTask* task) {
+  run(task, 1  /* nworkers */);
+}
+
 void ZWorkers::run_parallel(ZTask* task) {
   run(task, nparallel());
 }
diff --git a/src/hotspot/share/gc/z/zWorkers.hpp b/src/hotspot/share/gc/z/zWorkers.hpp
index c677a438c6c..f283c4f9d62 100644
--- a/src/hotspot/share/gc/z/zWorkers.hpp
+++ b/src/hotspot/share/gc/z/zWorkers.hpp
@@ -48,6 +48,7 @@ public:
 
   void set_boost(bool boost);
 
+  void run_serial(ZTask* task);
   void run_parallel(ZTask* task);
   void run_concurrent(ZTask* task);