8324553: Shenandoah: Move periodic tasks closer to their collaborators

Reviewed-by: kdnilsen, shade

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

@@ -50,31 +50,12 @@ ShenandoahControlThread::ShenandoahControlThread() :
   ConcurrentGCThread(),
   _alloc_failure_waiters_lock(Mutex::safepoint-2, "ShenandoahAllocFailureGC_lock", true),
   _gc_waiters_lock(Mutex::safepoint-2, "ShenandoahRequestedGC_lock", true),
-  _periodic_task(this),
   _requested_gc_cause(GCCause::_no_cause_specified),
   _degen_point(ShenandoahGC::_degenerated_outside_cycle),
   _allocs_seen(0) {
   set_name("Shenandoah Control Thread");
   reset_gc_id();
   create_and_start();
-  _periodic_task.enroll();
-  if (ShenandoahPacing) {
-    _periodic_pacer_notify_task.enroll();
-  }
-}
-
-ShenandoahControlThread::~ShenandoahControlThread() {
-  // This is here so that super is called.
-}
-
-void ShenandoahPeriodicTask::task() {
-  _thread->handle_force_counters_update();
-  _thread->handle_counters_update();
-}
-
-void ShenandoahPeriodicPacerNotify::task() {
-  assert(ShenandoahPacing, "Should not be here otherwise");
-  ShenandoahHeap::heap()->pacer()->notify_waiters();
 }
 
 void ShenandoahControlThread::run_service() {
@@ -195,7 +176,7 @@ void ShenandoahControlThread::run_service() {
 
       // If GC was requested, we are sampling the counters even without actual triggers
       // from allocation machinery. This captures GC phases more accurately.
-      set_forced_counters_update(true);
+      heap->set_forced_counters_update(true);
 
       // If GC was requested, we better dump freeset data for performance debugging
       {
@@ -236,16 +217,16 @@ void ShenandoahControlThread::run_service() {
         // Notify Universe about new heap usage. This has implications for
         // global soft refs policy, and we better report it every time heap
         // usage goes down.
-        Universe::heap()->update_capacity_and_used_at_gc();
+        heap->update_capacity_and_used_at_gc();
 
         // Signal that we have completed a visit to all live objects.
-        Universe::heap()->record_whole_heap_examined_timestamp();
+        heap->record_whole_heap_examined_timestamp();
       }
 
       // Disable forced counters update, and update counters one more time
       // to capture the state at the end of GC session.
-      handle_force_counters_update();
-      set_forced_counters_update(false);
+      heap->handle_force_counters_update();
+      heap->set_forced_counters_update(false);
 
       // Retract forceful part of soft refs policy
       heap->soft_ref_policy()->set_should_clear_all_soft_refs(false);
@@ -573,28 +554,8 @@ void ShenandoahControlThread::notify_gc_waiters() {
   ml.notify_all();
 }
 
-void ShenandoahControlThread::handle_counters_update() {
-  if (_do_counters_update.is_set()) {
-    _do_counters_update.unset();
-    ShenandoahHeap::heap()->monitoring_support()->update_counters();
-  }
-}
-
-void ShenandoahControlThread::handle_force_counters_update() {
-  if (_force_counters_update.is_set()) {
-    _do_counters_update.unset(); // reset these too, we do update now!
-    ShenandoahHeap::heap()->monitoring_support()->update_counters();
-  }
-}
-
 void ShenandoahControlThread::notify_heap_changed() {
   // This is called from allocation path, and thus should be fast.
-
-  // Update monitoring counters when we took a new region. This amortizes the
-  // update costs on slow path.
-  if (_do_counters_update.is_unset()) {
-    _do_counters_update.set();
-  }
   // Notify that something had changed.
   if (_heap_changed.is_unset()) {
     _heap_changed.set();
@@ -606,10 +567,6 @@ void ShenandoahControlThread::pacing_notify_alloc(size_t words) {
   Atomic::add(&_allocs_seen, words, memory_order_relaxed);
 }
 
-void ShenandoahControlThread::set_forced_counters_update(bool value) {
-  _force_counters_update.set_cond(value);
-}
-
 void ShenandoahControlThread::reset_gc_id() {
   Atomic::store(&_gc_id, (size_t)0);
 }

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

@@ -31,28 +31,8 @@
 #include "gc/shenandoah/shenandoahHeap.hpp"
 #include "gc/shenandoah/shenandoahPadding.hpp"
 #include "gc/shenandoah/shenandoahSharedVariables.hpp"
-#include "runtime/task.hpp"
 #include "utilities/ostream.hpp"
 
-// Periodic task is useful for doing asynchronous things that do not require (heap) locks,
-// or synchronization with other parts of collector. These could run even when ShenandoahConcurrentThread
-// is busy driving the GC cycle.
-class ShenandoahPeriodicTask : public PeriodicTask {
-private:
-  ShenandoahControlThread* _thread;
-public:
-  ShenandoahPeriodicTask(ShenandoahControlThread* thread) :
-          PeriodicTask(100), _thread(thread) {}
-  virtual void task();
-};
-
-// Periodic task to notify blocked paced waiters.
-class ShenandoahPeriodicPacerNotify : public PeriodicTask {
-public:
-  ShenandoahPeriodicPacerNotify() : PeriodicTask(PeriodicTask::min_interval) {}
-  virtual void task();
-};
-
 class ShenandoahControlThread: public ConcurrentGCThread {
   friend class VMStructs;
 
@@ -69,8 +49,6 @@ private:
   // to make complete explicit cycle for for demanding customers.
   Monitor _alloc_failure_waiters_lock;
   Monitor _gc_waiters_lock;
-  ShenandoahPeriodicTask _periodic_task;
-  ShenandoahPeriodicPacerNotify _periodic_pacer_notify_task;
 
 public:
   void run_service();
@@ -81,8 +59,6 @@ private:
   ShenandoahSharedFlag _alloc_failure_gc;
   ShenandoahSharedFlag _graceful_shutdown;
   ShenandoahSharedFlag _heap_changed;
-  ShenandoahSharedFlag _do_counters_update;
-  ShenandoahSharedFlag _force_counters_update;
   GCCause::Cause       _requested_gc_cause;
   ShenandoahGC::ShenandoahDegenPoint _degen_point;
 
@@ -119,7 +95,6 @@ private:
 public:
   // Constructor
   ShenandoahControlThread();
-  ~ShenandoahControlThread();
 
   // Handle allocation failure from a mutator allocation.
   // Optionally blocks while collector is handling the failure. If the GC
@@ -132,10 +107,6 @@ public:
 
   void request_gc(GCCause::Cause cause);
 
-  void handle_counters_update();
-  void handle_force_counters_update();
-  void set_forced_counters_update(bool value);
-
   void notify_heap_changed();
 
   void pacing_notify_alloc(size_t words);

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

@@ -432,8 +432,6 @@ jint ShenandoahHeap::initialize() {
   if (ShenandoahPacing) {
     _pacer = new ShenandoahPacer(this);
     _pacer->setup_for_idle();
-  } else {
-    _pacer = nullptr;
   }
 
   _control_thread = new ShenandoahControlThread();
@@ -782,10 +780,25 @@ void ShenandoahHeap::op_uncommit(double shrink_before, size_t shrink_until) {
   }
 
   if (count > 0) {
-    control_thread()->notify_heap_changed();
+    notify_heap_changed();
   }
 }
 
+void ShenandoahHeap::notify_heap_changed() {
+  // Update monitoring counters when we took a new region. This amortizes the
+  // update costs on slow path.
+  monitoring_support()->notify_heap_changed();
+  control_thread()->notify_heap_changed();
+}
+
+void ShenandoahHeap::set_forced_counters_update(bool value) {
+  monitoring_support()->set_forced_counters_update(value);
+}
+
+void ShenandoahHeap::handle_force_counters_update() {
+  monitoring_support()->handle_force_counters_update();
+}
+
 HeapWord* ShenandoahHeap::allocate_from_gclab_slow(Thread* thread, size_t size) {
   // New object should fit the GCLAB size
   size_t min_size = MAX2(size, PLAB::min_size());
@@ -915,7 +928,7 @@ HeapWord* ShenandoahHeap::allocate_memory(ShenandoahAllocRequest& req) {
   }
 
   if (in_new_region) {
-    control_thread()->notify_heap_changed();
+    notify_heap_changed();
   }
 
   if (result != nullptr) {

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

@@ -207,6 +207,15 @@ public:
 
   void set_soft_max_capacity(size_t v);
 
+// ---------- Periodic Tasks
+//
+private:
+  void notify_heap_changed();
+
+public:
+  void set_forced_counters_update(bool value);
+  void handle_force_counters_update();
+
 // ---------- Workers handling
 //
 private:

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

@@ -37,7 +37,7 @@ public:
   ShenandoahYoungGenerationCounters() :
           GenerationCounters("Young", 0, 0, 0, (size_t)0, (size_t)0) {};
 
-  virtual void update_all() {
+  void update_all() override {
     // no update
   }
 };
@@ -46,19 +46,20 @@ class ShenandoahGenerationCounters : public GenerationCounters {
 private:
   ShenandoahHeap* _heap;
 public:
-  ShenandoahGenerationCounters(ShenandoahHeap* heap) :
+  explicit ShenandoahGenerationCounters(ShenandoahHeap* heap) :
           GenerationCounters("Heap", 1, 1, heap->initial_capacity(), heap->max_capacity(), heap->capacity()),
           _heap(heap)
   {};
 
-  virtual void update_all() {
+  void update_all() override {
     _current_size->set_value(_heap->capacity());
   }
 };
 
 ShenandoahMonitoringSupport::ShenandoahMonitoringSupport(ShenandoahHeap* heap) :
         _partial_counters(nullptr),
-        _full_counters(nullptr)
+        _full_counters(nullptr),
+        _counters_update_task(this)
 {
   // Collection counters do not fit Shenandoah very well.
   // We record partial cycles as "young", and full cycles (including full STW GC) as "old".
@@ -71,6 +72,8 @@ ShenandoahMonitoringSupport::ShenandoahMonitoringSupport(ShenandoahHeap* heap) :
   _space_counters = new HSpaceCounters(_heap_counters->name_space(), "Heap", 0, heap->max_capacity(), heap->initial_capacity());
 
   _heap_region_counters = new ShenandoahHeapRegionCounters();
+
+  _counters_update_task.enroll();
 }
 
 CollectorCounters* ShenandoahMonitoringSupport::stw_collection_counters() {
@@ -103,3 +106,44 @@ void ShenandoahMonitoringSupport::update_counters() {
     MetaspaceCounters::update_performance_counters();
   }
 }
+
+void ShenandoahMonitoringSupport::notify_heap_changed() {
+  _counters_update_task.notify_heap_changed();
+}
+
+void ShenandoahMonitoringSupport::set_forced_counters_update(bool value) {
+  _counters_update_task.set_forced_counters_update(value);
+}
+
+void ShenandoahMonitoringSupport::handle_force_counters_update() {
+  _counters_update_task.handle_force_counters_update();
+}
+
+void ShenandoahPeriodicCountersUpdateTask::task() {
+  handle_force_counters_update();
+  handle_counters_update();
+}
+
+void ShenandoahPeriodicCountersUpdateTask::handle_counters_update() {
+  if (_do_counters_update.is_set()) {
+    _do_counters_update.unset();
+    _monitoring_support->update_counters();
+  }
+}
+
+void ShenandoahPeriodicCountersUpdateTask::handle_force_counters_update() {
+  if (_force_counters_update.is_set()) {
+    _do_counters_update.unset(); // reset these too, we do update now!
+    _monitoring_support->update_counters();
+  }
+}
+
+void ShenandoahPeriodicCountersUpdateTask::notify_heap_changed() {
+  if (_do_counters_update.is_unset()) {
+    _do_counters_update.set();
+  }
+}
+
+void ShenandoahPeriodicCountersUpdateTask::set_forced_counters_update(bool value) {
+  _force_counters_update.set_cond(value);
+}

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

@@ -25,13 +25,35 @@
 #ifndef SHARE_GC_SHENANDOAH_SHENANDOAHMONITORINGSUPPORT_HPP
 #define SHARE_GC_SHENANDOAH_SHENANDOAHMONITORINGSUPPORT_HPP
 
+#include "gc/shenandoah/shenandoahSharedVariables.hpp"
 #include "memory/allocation.hpp"
+#include "runtime/task.hpp"
 
 class GenerationCounters;
 class HSpaceCounters;
 class ShenandoahHeap;
 class CollectorCounters;
 class ShenandoahHeapRegionCounters;
+class ShenandoahMonitoringSupport;
+
+class ShenandoahPeriodicCountersUpdateTask : public PeriodicTask {
+private:
+  ShenandoahSharedFlag _do_counters_update;
+  ShenandoahSharedFlag _force_counters_update;
+  ShenandoahMonitoringSupport* const _monitoring_support;
+
+public:
+  explicit ShenandoahPeriodicCountersUpdateTask(ShenandoahMonitoringSupport* monitoring_support) :
+    PeriodicTask(100),
+    _monitoring_support(monitoring_support) { }
+
+  void task() override;
+
+  void handle_counters_update();
+  void handle_force_counters_update();
+  void set_forced_counters_update(bool value);
+  void notify_heap_changed();
+};
 
 class ShenandoahMonitoringSupport : public CHeapObj<mtGC> {
 private:
@@ -44,14 +66,20 @@ private:
   HSpaceCounters* _space_counters;
 
   ShenandoahHeapRegionCounters* _heap_region_counters;
+  ShenandoahPeriodicCountersUpdateTask _counters_update_task;
 
 public:
- ShenandoahMonitoringSupport(ShenandoahHeap* heap);
- CollectorCounters* stw_collection_counters();
- CollectorCounters* full_stw_collection_counters();
- CollectorCounters* concurrent_collection_counters();
- CollectorCounters* partial_collection_counters();
- void update_counters();
+  explicit ShenandoahMonitoringSupport(ShenandoahHeap* heap);
+  CollectorCounters* stw_collection_counters();
+  CollectorCounters* full_stw_collection_counters();
+  CollectorCounters* concurrent_collection_counters();
+  CollectorCounters* partial_collection_counters();
+
+  void notify_heap_changed();
+  void set_forced_counters_update(bool value);
+  void handle_force_counters_update();
+
+  void update_counters();
 };
 
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHMONITORINGSUPPORT_HPP

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

@@ -338,3 +338,8 @@ void ShenandoahPacer::print_cycle_on(outputStream* out) {
   }
   out->cr();
 }
+
+void ShenandoahPeriodicPacerNotifyTask::task() {
+  assert(ShenandoahPacing, "Should not be here otherwise");
+  _pacer->notify_waiters();
+}

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

@@ -29,8 +29,24 @@
 #include "gc/shenandoah/shenandoahPadding.hpp"
 #include "gc/shenandoah/shenandoahSharedVariables.hpp"
 #include "memory/allocation.hpp"
+#include "runtime/task.hpp"
 
 class ShenandoahHeap;
+class ShenandoahPacer;
+
+
+// Periodic task to notify blocked paced waiters.
+class ShenandoahPeriodicPacerNotifyTask : public PeriodicTask {
+private:
+  ShenandoahPacer* const _pacer;
+public:
+  explicit ShenandoahPeriodicPacerNotifyTask(ShenandoahPacer* pacer) :
+    PeriodicTask(PeriodicTask::min_interval),
+    _pacer(pacer) { }
+
+  void task() override;
+};
+
 
 #define PACING_PROGRESS_UNINIT (-1)
 #define PACING_PROGRESS_ZERO   ( 0)
@@ -48,6 +64,7 @@ private:
   TruncatedSeq* _progress_history;
   Monitor* _wait_monitor;
   ShenandoahSharedFlag _need_notify_waiters;
+  ShenandoahPeriodicPacerNotifyTask _notify_waiters_task;
 
   // Set once per phase
   volatile intptr_t _epoch;
@@ -64,15 +81,18 @@ private:
   shenandoah_padding(3);
 
 public:
-  ShenandoahPacer(ShenandoahHeap* heap) :
+  explicit ShenandoahPacer(ShenandoahHeap* heap) :
           _heap(heap),
           _last_time(os::elapsedTime()),
           _progress_history(new TruncatedSeq(5)),
           _wait_monitor(new Monitor(Mutex::safepoint-1, "ShenandoahWaitMonitor_lock", true)),
+          _notify_waiters_task(this),
           _epoch(0),
           _tax_rate(1),
           _budget(0),
-          _progress(PACING_PROGRESS_UNINIT) {}
+          _progress(PACING_PROGRESS_UNINIT) {
+    _notify_waiters_task.enroll();
+  }
 
   void setup_for_idle();
   void setup_for_mark();