Skip to content

Commit 7cf9612

Browse files
committed
perf(qwp): wake quiescence waiters only when one is parked
serviceRing's per-pass finally called lock.notifyAll() unconditionally - with the default 1 ms poll that is ~1000 wakeups/sec per registered ring on the production worker, paid for a barrier (awaitRingQuiescence) that production never takes: all three production constructions own their manager, so close() goes through manager.close()+isWorkerReaped() and never parks on the lock. - new quiescenceWaiters count, incremented/decremented around the awaitRingQuiescence wait loop under the same lock as the worker's check - no lost-wakeup window, and the timed wait remains a fallback - the per-pass finally notifies only when quiescenceWaiters > 0; in steady state it never fires - notifyAll (not notify) retained when a waiter exists: with a shared manager, distinct waiters may await different rings
1 parent d3b6147 commit 7cf9612

1 file changed

Lines changed: 32 additions & 12 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentManager.java

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,14 @@ public final class SegmentManager implements QuietCloseable {
109109
// {@link #lock}; cleared with lock.notifyAll() so awaitRingQuiescence can
110110
// block until an in-flight pass for a just-deregistered ring finishes.
111111
private RingEntry inService;
112+
// Number of threads currently parked in awaitRingQuiescence()'s wait
113+
// loop. Guarded by {@link #lock}. The worker's per-pass finally block
114+
// only calls lock.notifyAll() when this is > 0, so the steady-state
115+
// production path (no quiescence barrier in flight) never notifies.
116+
// Both sides mutate/check under the same lock, so there is no lost
117+
// wakeup: a waiter that increments after the worker's check re-reads
118+
// inService before waiting and sees the pass already finished.
119+
private int quiescenceWaiters;
112120
private long lastDiskFullLogNs;
113121
private volatile boolean running;
114122
// pathScratch free-exactly-once coordination between a timed-out close()
@@ -284,18 +292,23 @@ public boolean awaitRingQuiescence(SegmentRing ring) {
284292
boolean interrupted = Thread.interrupted();
285293
try {
286294
synchronized (lock) {
287-
while (inService != null && inService.ring == ring) {
288-
long remainingNanos = deadlineNanos - System.nanoTime();
289-
if (remainingNanos <= 0) {
290-
return false;
291-
}
292-
try {
293-
// Round up so a sub-millisecond remainder still waits
294-
// instead of spinning through wait(0) == wait-forever.
295-
lock.wait(Math.max(1L, remainingNanos / 1_000_000L));
296-
} catch (InterruptedException ignored) {
297-
interrupted = true;
295+
quiescenceWaiters++;
296+
try {
297+
while (inService != null && inService.ring == ring) {
298+
long remainingNanos = deadlineNanos - System.nanoTime();
299+
if (remainingNanos <= 0) {
300+
return false;
301+
}
302+
try {
303+
// Round up so a sub-millisecond remainder still waits
304+
// instead of spinning through wait(0) == wait-forever.
305+
lock.wait(Math.max(1L, remainingNanos / 1_000_000L));
306+
} catch (InterruptedException ignored) {
307+
interrupted = true;
308+
}
298309
}
310+
} finally {
311+
quiescenceWaiters--;
299312
}
300313
}
301314
return true;
@@ -553,7 +566,14 @@ private void serviceRing(RingEntry e) {
553566
} finally {
554567
synchronized (lock) {
555568
inService = null;
556-
lock.notifyAll();
569+
// Wake quiescence barriers only when one is actually parked.
570+
// In production no engine ever waits here (owned-manager
571+
// close joins the worker thread instead), so the per-tick
572+
// pass must not pay an unconditional notifyAll. notifyAll,
573+
// not notify: distinct waiters may await different rings.
574+
if (quiescenceWaiters > 0) {
575+
lock.notifyAll();
576+
}
557577
}
558578
}
559579
}

0 commit comments

Comments
 (0)