From 7289cb2565fb331c000c46d42746ec5eb7b1ba99 Mon Sep 17 00:00:00 2001
From: Greg Burd <greg@burd.me>
Date: Mon, 6 Jul 2026 17:25:00 -0400
Subject: [PATCH v4 1/3] Batch the clock sweep to reduce nextVictimBuffer
 atomic contention

StrategyGetBuffer() advances the shared clock hand, nextVictimBuffer, with a
pg_atomic_fetch_add_u32(..., 1) on every tick.  On a multi-socket system the
cache line holding that counter has to travel over the interconnect on each
operation, pushing a sweep tick from ~20ns (same-socket, line warm in L1/L2)
into the ~100-200ns range.  Under eviction pressure with hundreds of backends
in StrategyGetBuffer() concurrently, that single cache line becomes the
dominant cost of the sweep, visible as elevated bus-cycles and cache-misses in
a perf profile.

Have each backend claim a run of consecutive buffer IDs from the shared hand
with a single fetch-add and then iterate through them privately.  The sweep
still advances through the pool in order, each buffer is still visited exactly
once per complete pass, and the meaning of the clock state is unchanged; only
the temporal ordering of visits within a pass changes, which the algorithm
does not depend on.  The contended atomic now fires roughly once per batch
rather than once per buffer.

The batch is one cache line's worth of clock-hand values --
PG_CACHE_LINE_SIZE / sizeof(uint32) -- capped at NBuffers so a claim can never
wrap the pool more than once.  Batching only helps when the counter's cache
line actually bounces between sockets, so it is enabled only on multi-node
NUMA hardware (pg_numa_get_max_node() >= 1); on a single socket, or where
libnuma is unavailable, the batch size stays 1 and the code path is
byte-identical to the stock clock sweep.

Wraparound handling is adjusted: with batching, several backends can each see
a fetch-add return a value past NBuffers within the same pass.  Any such
backend takes buffer_strategy_lock, re-reads the counter, and if it is still
out of range wraps it with a single CAS and increments completePasses.
StrategySyncStart() continues to see a consistent (nextVictimBuffer,
completePasses) pair.

This is the batched-clock-sweep idea from Jim Mlodgenski's pgsql-hackers
thread, adapted to derive the batch size from the platform cache-line size.

Co-authored-by: Jim Mlodgenski <mlodj@amazon.com>
---
 src/backend/storage/buffer/freelist.c | 134 +++++++++++++++++---------
 1 file changed, 91 insertions(+), 43 deletions(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index fdb5bad7910..93fc4dd758b 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -22,6 +22,7 @@
 #include "storage/proc.h"
 #include "storage/shmem.h"
 #include "storage/subsystems.h"
+#include "port/pg_numa.h"
 
 #define INT_ACCESS_ONCE(var)	((int)(*((volatile int *)&(var))))
 
@@ -100,68 +101,92 @@ static BufferDesc *GetBufferFromRing(BufferAccessStrategy strategy,
 static void AddBufferToRing(BufferAccessStrategy strategy,
 							BufferDesc *buf);
 
+/*
+ * Per-backend state for the batched clock sweep.  Each backend claims a run
+ * of consecutive clock-hand values with a single atomic fetch-add and then
+ * iterates through them privately, so the contended nextVictimBuffer cache
+ * line is touched roughly 1/batch as often.  MyBatchPos is the next hand
+ * value to hand out; MyBatchEnd is one past the end of the claimed run.  Both
+ * are absolute (monotonically increasing) hand values; the buffer id is the
+ * value modulo NBuffers.
+ */
+static uint32 MyBatchPos = 0;
+static uint32 MyBatchEnd = 0;
+
+/*
+ * Number of clock-hand values a backend claims per atomic fetch-add,
+ * computed once at startup (see StrategyCtlShmemInit).  When batching is
+ * enabled it is one cache line's worth of hand advance, so concurrent
+ * backends sweep non-overlapping, cache-line-sized runs of the pool; the
+ * global sweep order is preserved (each buffer is still visited exactly once
+ * per pass).  Batching is enabled only on multi-node NUMA hardware; otherwise
+ * this stays 1 and the sweep is byte-identical to the stock clock.
+ */
+static uint32 ClockSweepBatchSize = 1;
+
 /*
  * ClockSweepTick - Helper routine for StrategyGetBuffer()
  *
- * Move the clock hand one buffer ahead of its current position and return the
- * id of the buffer now under the hand.
+ * Return the next buffer to consider for eviction.  Backends claim batches of
+ * consecutive buffer IDs from the shared clock hand, then iterate through
+ * them locally without further atomic operations.  This preserves the global
+ * sweep order while reducing contention on the shared counter.
  */
 static inline uint32
 ClockSweepTick(void)
 {
 	uint32		victim;
 
-	/*
-	 * Atomically move hand ahead one buffer - if there's several processes
-	 * doing this, this can lead to buffers being returned slightly out of
-	 * apparent order.
-	 */
-	victim =
-		pg_atomic_fetch_add_u32(&StrategyControl->nextVictimBuffer, 1);
-
-	if (victim >= NBuffers)
+	if (MyBatchPos >= MyBatchEnd)
 	{
-		uint32		originalVictim = victim;
-
-		/* always wrap what we look up in BufferDescriptors */
-		victim = victim % NBuffers;
-
 		/*
-		 * If we're the one that just caused a wraparound, force
-		 * completePasses to be incremented while holding the spinlock. We
-		 * need the spinlock so StrategySyncStart() can return a consistent
-		 * value consisting of nextVictimBuffer and completePasses.
+		 * Claim a fresh batch from the shared clock hand.  This is the only
+		 * atomic operation per batch, reducing contention by the batch size.
 		 */
-		if (victim == 0)
-		{
-			uint32		expected;
-			uint32		wrapped;
-			bool		success = false;
+		uint32		start;
+		uint32		batch_size = ClockSweepBatchSize;
 
-			expected = originalVictim + 1;
+		start = pg_atomic_fetch_add_u32(&StrategyControl->nextVictimBuffer,
+										batch_size);
 
-			while (!success)
-			{
-				/*
-				 * Acquire the spinlock while increasing completePasses. That
-				 * allows other readers to read nextVictimBuffer and
-				 * completePasses in a consistent manner which is required for
-				 * StrategySyncStart().  In theory delaying the increment
-				 * could lead to an overflow of nextVictimBuffers, but that's
-				 * highly unlikely and wouldn't be particularly harmful.
-				 */
-				SpinLockAcquire(&StrategyControl->buffer_strategy_lock);
+		if (start >= (uint32) NBuffers)
+		{
+			start = start % NBuffers;
 
-				wrapped = expected % NBuffers;
+			/*
+			 * The counter has grown past NBuffers; try to wrap it back.  We
+			 * must hold the spinlock so StrategySyncStart() can read
+			 * nextVictimBuffer and completePasses consistently.
+			 *
+			 * With batching, multiple backends may each land a fetch-add
+			 * that returns a value past NBuffers in the same pass.  After
+			 * acquiring the spinlock we re-read the counter: if another
+			 * backend already wrapped it below NBuffers we are done.
+			 */
+			SpinLockAcquire(&StrategyControl->buffer_strategy_lock);
+			{
+				uint32		current;
+				uint32		wrapped;
 
-				success = pg_atomic_compare_exchange_u32(&StrategyControl->nextVictimBuffer,
-														 &expected, wrapped);
-				if (success)
-					StrategyControl->completePasses++;
-				SpinLockRelease(&StrategyControl->buffer_strategy_lock);
+				current = pg_atomic_read_u32(&StrategyControl->nextVictimBuffer);
+				if (current >= (uint32) NBuffers)
+				{
+					wrapped = current % NBuffers;
+					if (pg_atomic_compare_exchange_u32(&StrategyControl->nextVictimBuffer,
+													   &current, wrapped))
+						StrategyControl->completePasses++;
+				}
 			}
+			SpinLockRelease(&StrategyControl->buffer_strategy_lock);
 		}
+
+		MyBatchPos = start;
+		MyBatchEnd = start + batch_size;
 	}
+
+	victim = MyBatchPos % NBuffers;
+	MyBatchPos++;
+
 	return victim;
 }
 
@@ -408,6 +433,29 @@ StrategyCtlShmemInit(void *arg)
 
 	/* No pending notification */
 	StrategyControl->bgwprocno = -1;
+
+	/*
+	 * Decide whether to batch the clock sweep.
+	 *
+	 * Batching claims a run of consecutive buffer IDs per atomic fetch-add so
+	 * concurrent backends touch the shared nextVictimBuffer cache line ~1/batch
+	 * as often -- a win only when that line actually bounces across sockets,
+	 * i.e. on multi-node NUMA hardware.  On a single socket the atomic is
+	 * already node-local and batching would only make backends skip ahead for
+	 * no benefit, so we fall back to batch size 1 there (byte-identical to the
+	 * stock one-buffer-at-a-time clock sweep).
+	 *
+	 * Batch (> 1) only when libnuma reports more than one node
+	 * (pg_numa_get_max_node() >= 1); pg_numa_init() returns -1 when NUMA is
+	 * unavailable.  (Benchmarking showed the win holds with or without huge
+	 * pages, so huge-page availability is intentionally not part of the gate.)
+	 */
+	if (pg_numa_init() != -1 &&
+		pg_numa_get_max_node() >= 1)
+		ClockSweepBatchSize = Min(PG_CACHE_LINE_SIZE / (uint32) sizeof(uint32),
+								  (uint32) NBuffers);
+	else
+		ClockSweepBatchSize = 1;
 }
 
 
-- 
2.51.2

