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

rev 59715 : 8247358: Shenandoah: reconsider free budget slice for marking
Reviewed-by: XXX
rev 59716 : 8247367: Shenandoah: pacer should wait on lock instead of exponential backoff
Reviewed-by: XXX

@@ -189,10 +189,13 @@
   size_t initial = (size_t)(non_taxable_bytes * tax_rate) >> LogHeapWordSize;
   STATIC_ASSERT(sizeof(size_t) <= sizeof(intptr_t));
   Atomic::xchg(&_budget, (intptr_t)initial);
   Atomic::store(&_tax_rate, tax_rate);
   Atomic::inc(&_epoch);
+
+  // Shake up stalled waiters after budget update.
+  notify_waiters();
 }
 
 bool ShenandoahPacer::claim_for_alloc(size_t words, bool force) {
   assert(ShenandoahPacing, "Only be here when pacing is enabled");
 

@@ -229,60 +232,49 @@
 
 void ShenandoahPacer::pace_for_alloc(size_t words) {
   assert(ShenandoahPacing, "Only be here when pacing is enabled");
 
   // Fast path: try to allocate right away
-  if (claim_for_alloc(words, false)) {
+  bool claimed = claim_for_alloc(words, false);
+  if (claimed) {
     return;
   }
 
+  // Forcefully claim the budget: it may go negative at this point, and
+  // GC should replenish for this and subsequent allocations. After this claim,
+  // we would wait a bit until our claim is matched by additional progress,
+  // or the time budget depletes.
+  claimed = claim_for_alloc(words, true);
+  assert(claimed, "Should always succeed");
+
   // Threads that are attaching should not block at all: they are not
   // fully initialized yet. Blocking them would be awkward.
   // This is probably the path that allocates the thread oop itself.
-  // Forcefully claim without waiting.
   if (JavaThread::current()->is_attaching_via_jni()) {
-    claim_for_alloc(words, true);
     return;
   }
 
-  size_t max = ShenandoahPacingMaxDelay;
   double start = os::elapsedTime();
 
-  size_t total = 0;
-  size_t cur = 0;
+  size_t max_ms = ShenandoahPacingMaxDelay;
+  size_t total_ms = 0;
 
   while (true) {
     // We could instead assist GC, but this would suffice for now.
-    // This code should also participate in safepointing.
-    // Perform the exponential backoff, limited by max.
-
-    cur = cur * 2;
-    if (total + cur > max) {
-      cur = (max > total) ? (max - total) : 0;
-    }
-    cur = MAX2<size_t>(1, cur);
-
-    wait(cur);
+    size_t cur_ms = (max_ms > total_ms) ? (max_ms - total_ms) : 1;
+    wait(cur_ms);
 
     double end = os::elapsedTime();
-    total = (size_t)((end - start) * 1000);
-
-    if (total > max) {
-      // Spent local time budget to wait for enough GC progress.
-      // Breaking out and allocating anyway, which may mean we outpace GC,
-      // and start Degenerated GC cycle.
-      _delays.add(total);
-
-      // Forcefully claim the budget: it may go negative at this point, and
-      // GC should replenish for this and subsequent allocations
-      claim_for_alloc(words, true);
-      break;
-    }
+    total_ms = (size_t)((end - start) * 1000);
 
-    if (claim_for_alloc(words, false)) {
-      // Acquired enough permit, nice. Can allocate now.
-      _delays.add(total);
+    if (total_ms > max_ms || Atomic::load(&_budget) >= 0) {
+      // Exiting if either:
+      //  a) Spent local time budget to wait for enough GC progress.
+      //     Breaking out and allocating anyway, which may mean we outpace GC,
+      //     and start Degenerated GC cycle.
+      //  b) The budget had been replenished, which means our claim is satisfied.
+      _delays.add(total_ms);
       break;
     }
   }
 }