8247593: Shenandoah: should not block pacing reporters

Reviewed-by: rkennke

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

@@ -53,6 +53,9 @@ ShenandoahControlThread::ShenandoahControlThread() :
   create_and_start(ShenandoahCriticalControlThreadPriority ? CriticalPriority : NearMaxPriority);
   _periodic_task.enroll();
   _periodic_satb_flush_task.enroll();
+  if (ShenandoahPacing) {
+    _periodic_pacer_notify_task.enroll();
+  }
 }
 
 ShenandoahControlThread::~ShenandoahControlThread() {
@@ -68,6 +71,11 @@ void ShenandoahPeriodicSATBFlushTask::task() {
   ShenandoahHeap::heap()->force_satb_flush_all_threads();
 }
 
+void ShenandoahPeriodicPacerNotify::task() {
+  assert(ShenandoahPacing, "Should not be here otherwise");
+  ShenandoahHeap::heap()->pacer()->notify_waiters();
+}
+
 void ShenandoahControlThread::run_service() {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
 

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

@@ -52,6 +52,13 @@ class ShenandoahPeriodicSATBFlushTask : public PeriodicTask {
   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;
 
@@ -70,6 +77,7 @@ class ShenandoahControlThread: public ConcurrentGCThread {
   Monitor _gc_waiters_lock;
   ShenandoahPeriodicTask _periodic_task;
   ShenandoahPeriodicSATBFlushTask _periodic_satb_flush_task;
+  ShenandoahPeriodicPacerNotify _periodic_pacer_notify_task;
 
 public:
   void run_service();

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

@@ -193,7 +193,7 @@ void ShenandoahPacer::restart_with(size_t non_taxable_bytes, double tax_rate) {
   Atomic::inc(&_epoch);
 
   // Shake up stalled waiters after budget update.
-  notify_waiters();
+  _need_notify_waiters.try_set();
 }
 
 bool ShenandoahPacer::claim_for_alloc(size_t words, bool force) {
@@ -222,8 +222,8 @@ void ShenandoahPacer::unpace_for_alloc(intptr_t epoch, size_t words) {
     return;
   }
 
-  intptr_t tax = MAX2<intptr_t>(1, words * Atomic::load(&_tax_rate));
-  Atomic::add(&_budget, tax);
+  size_t tax = MAX2<size_t>(1, words * Atomic::load(&_tax_rate));
+  add_budget(tax);
 }
 
 intptr_t ShenandoahPacer::epoch() {
@@ -288,8 +288,10 @@ void ShenandoahPacer::wait(size_t time_ms) {
 }
 
 void ShenandoahPacer::notify_waiters() {
-  MonitorLocker locker(_wait_monitor);
-  _wait_monitor->notify_all();
+  if (_need_notify_waiters.try_unset()) {
+    MonitorLocker locker(_wait_monitor);
+    _wait_monitor->notify_all();
+  }
 }
 
 void ShenandoahPacer::print_on(outputStream* out) const {

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

@@ -46,6 +46,7 @@ class ShenandoahPacer : public CHeapObj<mtGC> {
   BinaryMagnitudeSeq _delays;
   TruncatedSeq* _progress_history;
   Monitor* _wait_monitor;
+  ShenandoahSharedFlag _need_notify_waiters;
 
   // Set once per phase
   volatile intptr_t _epoch;
@@ -89,6 +90,8 @@ class ShenandoahPacer : public CHeapObj<mtGC> {
   void pace_for_alloc(size_t words);
   void unpace_for_alloc(intptr_t epoch, size_t words);
 
+  void notify_waiters();
+
   intptr_t epoch();
 
   void print_on(outputStream* out) const;
@@ -97,12 +100,12 @@ class ShenandoahPacer : public CHeapObj<mtGC> {
   inline void report_internal(size_t words);
   inline void report_progress_internal(size_t words);
 
+  inline void add_budget(size_t words);
   void restart_with(size_t non_taxable_bytes, double tax_rate);
 
   size_t update_and_get_progress_history();
 
   void wait(size_t time_ms);
-  void notify_waiters();
 };
 
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHPACER_HPP

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

@@ -47,21 +47,26 @@ inline void ShenandoahPacer::report_alloc(size_t words) {
 
 inline void ShenandoahPacer::report_internal(size_t words) {
   assert(ShenandoahPacing, "Only be here when pacing is enabled");
+  add_budget(words);
+}
+
+inline void ShenandoahPacer::report_progress_internal(size_t words) {
+  assert(ShenandoahPacing, "Only be here when pacing is enabled");
+  STATIC_ASSERT(sizeof(size_t) <= sizeof(intptr_t));
+  Atomic::add(&_progress, (intptr_t)words);
+}
+
+inline void ShenandoahPacer::add_budget(size_t words) {
   STATIC_ASSERT(sizeof(size_t) <= sizeof(intptr_t));
   intptr_t inc = (intptr_t) words;
   intptr_t new_budget = Atomic::add(&_budget, inc);
 
   // Was the budget replenished beyond zero? Then all pacing claims
-  // are satisfied, notify the waiters.
+  // are satisfied, notify the waiters. Avoid taking any locks here,
+  // as it can be called from hot paths and/or while holding other locks.
   if (new_budget >= 0 && (new_budget - inc) < 0) {
-    notify_waiters();
+    _need_notify_waiters.try_set();
   }
 }
 
-inline void ShenandoahPacer::report_progress_internal(size_t words) {
-  assert(ShenandoahPacing, "Only be here when pacing is enabled");
-  STATIC_ASSERT(sizeof(size_t) <= sizeof(intptr_t));
-  Atomic::add(&_progress, (intptr_t)words);
-}
-
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHPACER_INLINE_HPP