# HG changeset patch
# User pliden
# Date 1548753818 -3600
# Node ID 4e325f8b50cefdd62f7acc725790fa57a92857f5
# Parent  9f75dc382445f9edacaa2bc6fb31fbe905e366fb
8217858: ZGC: Clean up ZDriver
Reviewed-by: stefank, eosterlund

diff -r 9f75dc382445 -r 4e325f8b50ce src/hotspot/share/gc/z/zDriver.cpp
--- a/src/hotspot/share/gc/z/zDriver.cpp	Tue Jan 29 10:23:38 2019 +0100
+++ b/src/hotspot/share/gc/z/zDriver.cpp	Tue Jan 29 10:23:38 2019 +0100
@@ -44,7 +44,6 @@
 static const ZStatPhaseConcurrent ZPhaseConcurrentProcessNonStrongReferences("Concurrent Process Non-Strong References");
 static const ZStatPhaseConcurrent ZPhaseConcurrentResetRelocationSet("Concurrent Reset Relocation Set");
 static const ZStatPhaseConcurrent ZPhaseConcurrentDestroyDetachedPages("Concurrent Destroy Detached Pages");
-static const ZStatPhasePause      ZPhasePauseVerify("Pause Verify");
 static const ZStatPhaseConcurrent ZPhaseConcurrentSelectRelocationSet("Concurrent Select Relocation Set");
 static const ZStatPhaseConcurrent ZPhaseConcurrentPrepareRelocationSet("Concurrent Prepare Relocation Set");
 static const ZStatPhasePause      ZPhasePauseRelocateStart("Pause Relocate Start");
@@ -52,41 +51,26 @@
 static const ZStatCriticalPhase   ZCriticalPhaseGCLockerStall("GC Locker Stall", false /* verbose */);
 static const ZStatSampler         ZSamplerJavaThreads("System", "Java Threads", ZStatUnitThreads);
 
-class ZOperationClosure : public StackObj {
+class VM_ZOperation : public VM_Operation {
+private:
+  const uint _gc_id;
+  bool       _gc_locked;
+  bool       _success;
+
 public:
-  virtual const char* name() const = 0;
+  VM_ZOperation() :
+      _gc_id(GCId::current()),
+      _gc_locked(false),
+      _success(false) {}
 
   virtual bool needs_inactive_gc_locker() const {
-    // An inactive GC locker is needed in operations where we change the good
-    // mask or move objects. Changing the good mask will invalidate all oops,
+    // An inactive GC locker is needed in operations where we change the bad
+    // mask or move objects. Changing the bad mask will invalidate all oops,
     // which makes it conceptually the same thing as moving all objects.
     return false;
   }
 
   virtual bool do_operation() = 0;
-};
-
-class VM_ZOperation : public VM_Operation {
-private:
-  ZOperationClosure* _cl;
-  uint               _gc_id;
-  bool               _gc_locked;
-  bool               _success;
-
-public:
-  VM_ZOperation(ZOperationClosure* cl) :
-      _cl(cl),
-      _gc_id(GCId::current()),
-      _gc_locked(false),
-      _success(false) {}
-
-  virtual VMOp_Type type() const {
-    return VMOp_ZOperation;
-  }
-
-  virtual const char* name() const {
-    return _cl->name();
-  }
 
   virtual bool doit_prologue() {
     Heap_lock->lock();
@@ -94,28 +78,28 @@
   }
 
   virtual void doit() {
-    assert(SafepointSynchronize::is_at_safepoint(), "Should be at safepoint");
-
-    ZStatSample(ZSamplerJavaThreads, Threads::number_of_threads());
-
-    // Setup GC id
-    GCIdMark gcid(_gc_id);
+    // Abort if GC locker state is incompatible
+    if (needs_inactive_gc_locker() && GCLocker::check_active_before_gc()) {
+      _gc_locked = true;
+      return;
+    }
 
-    if (_cl->needs_inactive_gc_locker() && GCLocker::check_active_before_gc()) {
-      // GC locker is active, bail out
-      _gc_locked = true;
-    } else {
-      // Execute operation
-      IsGCActiveMark mark;
-      _success = _cl->do_operation();
-    }
+    // Setup GC id and active marker
+    GCIdMark gc_id_mark(_gc_id);
+    IsGCActiveMark gc_active_mark;
+
+    // Execute operation
+    _success = do_operation();
+
+    // Update statistics
+    ZStatSample(ZSamplerJavaThreads, Threads::number_of_threads());
   }
 
   virtual void doit_epilogue() {
     Heap_lock->unlock();
   }
 
-  bool gc_locked() {
+  bool gc_locked() const {
     return _gc_locked;
   }
 
@@ -165,10 +149,10 @@
   return false;
 }
 
-class ZMarkStartClosure : public ZOperationClosure {
+class VM_ZMarkStart : public VM_ZOperation {
 public:
-  virtual const char* name() const {
-    return "ZMarkStart";
+  virtual VMOp_Type type() const {
+    return VMOp_ZMarkStart;
   }
 
   virtual bool needs_inactive_gc_locker() const {
@@ -194,37 +178,23 @@
   }
 };
 
-class ZMarkEndClosure : public ZOperationClosure {
+class VM_ZMarkEnd : public VM_ZOperation {
 public:
-  virtual const char* name() const {
-    return "ZMarkEnd";
+  virtual VMOp_Type type() const {
+    return VMOp_ZMarkEnd;
   }
 
   virtual bool do_operation() {
     ZStatTimer timer(ZPhasePauseMarkEnd);
     ZServiceabilityMarkEndTracer tracer;
-
     return ZHeap::heap()->mark_end();
   }
 };
 
-class ZVerifyClosure : public ZOperationClosure {
+class VM_ZRelocateStart : public VM_ZOperation {
 public:
-  virtual const char* name() const {
-    return "ZVerify";
-  }
-
-  virtual bool do_operation() {
-    ZStatTimer timer(ZPhasePauseVerify);
-    Universe::verify();
-    return true;
-  }
-};
-
-class ZRelocateStartClosure : public ZOperationClosure {
-public:
-  virtual const char* name() const {
-    return "ZRelocateStart";
+  virtual VMOp_Type type() const {
+    return VMOp_ZRelocateStart;
   }
 
   virtual bool needs_inactive_gc_locker() const {
@@ -234,7 +204,6 @@
   virtual bool do_operation() {
     ZStatTimer timer(ZPhasePauseRelocateStart);
     ZServiceabilityRelocateStartTracer tracer;
-
     ZHeap::heap()->relocate_start();
     return true;
   }
@@ -247,24 +216,6 @@
   create_and_start();
 }
 
-bool ZDriver::vm_operation(ZOperationClosure* cl) {
-  for (;;) {
-    VM_ZOperation op(cl);
-    VMThread::execute(&op);
-    if (op.gc_locked()) {
-      // Wait for GC to become unlocked and restart the VM operation
-      ZStatTimer timer(ZCriticalPhaseGCLockerStall);
-      _gc_locker_port.wait();
-      continue;
-    }
-
-    // Notify VM operation completed
-    _gc_locker_port.ack();
-
-    return op.success();
-  }
-}
-
 void ZDriver::collect(GCCause::Cause cause) {
   switch (cause) {
   case GCCause::_wb_young_gc:
@@ -302,19 +253,96 @@
   }
 }
 
-GCCause::Cause ZDriver::start_gc_cycle() {
-  // Wait for GC request
-  return _gc_cycle_port.receive();
+template <typename T>
+bool ZDriver::pause() {
+  for (;;) {
+    T op;
+    VMThread::execute(&op);
+    if (op.gc_locked()) {
+      // Wait for GC to become unlocked and restart the VM operation
+      ZStatTimer timer(ZCriticalPhaseGCLockerStall);
+      _gc_locker_port.wait();
+      continue;
+    }
+
+    // Notify VM operation completed
+    _gc_locker_port.ack();
+
+    return op.success();
+  }
+}
+
+void ZDriver::pause_mark_start() {
+  pause<VM_ZMarkStart>();
+}
+
+void ZDriver::concurrent_mark() {
+  ZStatTimer timer(ZPhaseConcurrentMark);
+  ZHeap::heap()->mark(true /* initial */);
+}
+
+bool ZDriver::pause_mark_end() {
+  return pause<VM_ZMarkEnd>();
+}
+
+void ZDriver::concurrent_mark_continue() {
+  ZStatTimer timer(ZPhaseConcurrentMarkContinue);
+  ZHeap::heap()->mark(false /* initial */);
+}
+
+void ZDriver::concurrent_process_non_strong_references() {
+  ZStatTimer timer(ZPhaseConcurrentProcessNonStrongReferences);
+  ZHeap::heap()->process_non_strong_references();
 }
 
-class ZDriverCycleScope : public StackObj {
+void ZDriver::concurrent_reset_relocation_set() {
+  ZStatTimer timer(ZPhaseConcurrentResetRelocationSet);
+  ZHeap::heap()->reset_relocation_set();
+}
+
+void ZDriver::concurrent_destroy_detached_pages() {
+  ZStatTimer timer(ZPhaseConcurrentDestroyDetachedPages);
+  ZHeap::heap()->destroy_detached_pages();
+}
+
+void ZDriver::pause_verify() {
+  if (VerifyBeforeGC || VerifyDuringGC || VerifyAfterGC) {
+    VM_Verify op;
+    VMThread::execute(&op);
+  }
+}
+
+void ZDriver::concurrent_select_relocation_set() {
+  ZStatTimer timer(ZPhaseConcurrentSelectRelocationSet);
+  ZHeap::heap()->select_relocation_set();
+}
+
+void ZDriver::concurrent_prepare_relocation_set() {
+  ZStatTimer timer(ZPhaseConcurrentPrepareRelocationSet);
+  ZHeap::heap()->prepare_relocation_set();
+}
+
+void ZDriver::pause_relocate_start() {
+  pause<VM_ZRelocateStart>();
+}
+
+void ZDriver::concurrent_relocate() {
+  ZStatTimer timer(ZPhaseConcurrentRelocated);
+  ZHeap::heap()->relocate();
+}
+
+void ZDriver::check_out_of_memory() {
+  ZHeap::heap()->check_out_of_memory();
+}
+
+class ZDriverGCScope : public StackObj {
 private:
   GCIdMark      _gc_id;
   GCCauseSetter _gc_cause_setter;
   ZStatTimer    _timer;
 
 public:
-  ZDriverCycleScope(GCCause::Cause cause) :
+  ZDriverGCScope(GCCause::Cause cause) :
       _gc_id(),
       _gc_cause_setter(ZCollectedHeap::heap(), cause),
       _timer(ZPhaseCycle) {
@@ -322,7 +350,7 @@
     ZStatCycle::at_start();
   }
 
-  ~ZDriverCycleScope() {
+  ~ZDriverGCScope() {
     // Calculate boost factor
     const double boost_factor = (double)ZHeap::heap()->nconcurrent_worker_threads() /
                                 (double)ZHeap::heap()->nconcurrent_no_boost_worker_threads();
@@ -335,96 +363,63 @@
   }
 };
 
-void ZDriver::run_gc_cycle(GCCause::Cause cause) {
-  ZDriverCycleScope scope(cause);
+void ZDriver::gc(GCCause::Cause cause) {
+  ZDriverGCScope scope(cause);
 
   // Phase 1: Pause Mark Start
-  {
-    ZMarkStartClosure cl;
-    vm_operation(&cl);
-  }
+  pause_mark_start();
 
   // Phase 2: Concurrent Mark
-  {
-    ZStatTimer timer(ZPhaseConcurrentMark);
-    ZHeap::heap()->mark(true /* initial */);
-  }
+  concurrent_mark();
 
   // Phase 3: Pause Mark End
-  {
-    ZMarkEndClosure cl;
-    while (!vm_operation(&cl)) {
-      // Phase 3.5: Concurrent Mark Continue
-      ZStatTimer timer(ZPhaseConcurrentMarkContinue);
-      ZHeap::heap()->mark(false /* initial */);
-    }
+  while (!pause_mark_end()) {
+    // Phase 3.5: Concurrent Mark Continue
+    concurrent_mark_continue();
   }
 
   // Phase 4: Concurrent Process Non-Strong References
-  {
-    ZStatTimer timer(ZPhaseConcurrentProcessNonStrongReferences);
-    ZHeap::heap()->process_non_strong_references();
-  }
+  concurrent_process_non_strong_references();
 
   // Phase 5: Concurrent Reset Relocation Set
-  {
-    ZStatTimer timer(ZPhaseConcurrentResetRelocationSet);
-    ZHeap::heap()->reset_relocation_set();
-  }
+  concurrent_reset_relocation_set();
 
   // Phase 6: Concurrent Destroy Detached Pages
-  {
-    ZStatTimer timer(ZPhaseConcurrentDestroyDetachedPages);
-    ZHeap::heap()->destroy_detached_pages();
-  }
+  concurrent_destroy_detached_pages();
 
   // Phase 7: Pause Verify
-  if (VerifyBeforeGC || VerifyDuringGC || VerifyAfterGC) {
-    ZVerifyClosure cl;
-    vm_operation(&cl);
-  }
+  pause_verify();
 
   // Phase 8: Concurrent Select Relocation Set
-  {
-    ZStatTimer timer(ZPhaseConcurrentSelectRelocationSet);
-    ZHeap::heap()->select_relocation_set();
-  }
+  concurrent_select_relocation_set();
 
   // Phase 9: Concurrent Prepare Relocation Set
-  {
-    ZStatTimer timer(ZPhaseConcurrentPrepareRelocationSet);
-    ZHeap::heap()->prepare_relocation_set();
-  }
+  concurrent_prepare_relocation_set();
 
   // Phase 10: Pause Relocate Start
-  {
-    ZRelocateStartClosure cl;
-    vm_operation(&cl);
-  }
+  pause_relocate_start();
 
   // Phase 11: Concurrent Relocate
-  {
-    ZStatTimer timer(ZPhaseConcurrentRelocated);
-    ZHeap::heap()->relocate();
-  }
-}
-
-void ZDriver::end_gc_cycle() {
-  // Notify GC cycle completed
-  _gc_cycle_port.ack();
-
-  // Check for out of memory condition
-  ZHeap::heap()->check_out_of_memory();
+  concurrent_relocate();
 }
 
 void ZDriver::run_service() {
   // Main loop
   while (!should_terminate()) {
-    const GCCause::Cause cause = start_gc_cycle();
-    if (cause != GCCause::_no_gc) {
-      run_gc_cycle(cause);
-      end_gc_cycle();
+    // Wait for GC request
+    const GCCause::Cause cause = _gc_cycle_port.receive();
+    if (cause == GCCause::_no_gc) {
+      continue;
     }
+
+    // Run GC
+    gc(cause);
+
+    // Notify GC completed
+    _gc_cycle_port.ack();
+
+    // Check for out of memory condition
+    check_out_of_memory();
   }
 }
 
diff -r 9f75dc382445 -r 4e325f8b50ce src/hotspot/share/gc/z/zDriver.hpp
--- a/src/hotspot/share/gc/z/zDriver.hpp	Tue Jan 29 10:23:38 2019 +0100
+++ b/src/hotspot/share/gc/z/zDriver.hpp	Tue Jan 29 10:23:38 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -28,18 +28,31 @@
 #include "gc/shared/gcCause.hpp"
 #include "gc/z/zMessagePort.hpp"
 
-class ZOperationClosure;
+class VM_ZOperation;
 
 class ZDriver : public ConcurrentGCThread {
 private:
   ZMessagePort<GCCause::Cause> _gc_cycle_port;
   ZRendezvousPort              _gc_locker_port;
 
-  bool vm_operation(ZOperationClosure* cl);
+  template <typename T> bool pause();
 
-  GCCause::Cause start_gc_cycle();
-  void run_gc_cycle(GCCause::Cause cause);
-  void end_gc_cycle();
+  void pause_mark_start();
+  void concurrent_mark();
+  bool pause_mark_end();
+  void concurrent_mark_continue();
+  void concurrent_process_non_strong_references();
+  void concurrent_reset_relocation_set();
+  void concurrent_destroy_detached_pages();
+  void pause_verify();
+  void concurrent_select_relocation_set();
+  void concurrent_prepare_relocation_set();
+  void pause_relocate_start();
+  void concurrent_relocate();
+
+  void check_out_of_memory();
+
+  void gc(GCCause::Cause cause);
 
 protected:
   virtual void run_service();
diff -r 9f75dc382445 -r 4e325f8b50ce src/hotspot/share/runtime/vmOperations.hpp
--- a/src/hotspot/share/runtime/vmOperations.hpp	Tue Jan 29 10:23:38 2019 +0100
+++ b/src/hotspot/share/runtime/vmOperations.hpp	Tue Jan 29 10:23:38 2019 +0100
@@ -68,7 +68,9 @@
   template(G1CollectForAllocation)                \
   template(G1CollectFull)                         \
   template(G1Concurrent)                          \
-  template(ZOperation)                            \
+  template(ZMarkStart)                            \
+  template(ZMarkEnd)                              \
+  template(ZRelocateStart)                        \
   template(HandshakeOneThread)                    \
   template(HandshakeAllThreads)                   \
   template(HandshakeFallback)                     \