From b8d59c33e980c58bc7f79694541f4b9db1608ae7 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Sun, 11 Oct 2026 17:30:05 +0000 Subject: [PATCH v20261011 04/12] clock-sweep: track one second chance per buffer, not a 0..5 count A buffer carries a 0..5 usage counter: a pin increments it up to five, and each visit by the clock hand decrements it, so a buffer becomes a candidate for eviction only once the hand has passed it as many times as it has been touched. Replace that counter with a single flag, BM_SECOND_CHANCE: a pin sets it, a visit clears it, and the sweep reclaims a buffer on the visit that finds it already clear. This is a change to the sweep's bookkeeping, not to the sweep. The hand is untouched -- ClockSweepTick(), nextVictimBuffer and completePasses are exactly as before -- and a buffer accessed between two visits is still passed over. What changes is how much credit one access can buy: one revolution of grace instead of up to five. The counter's range was always a trade-off, and the header said so: a large value approximates LRU, but "it can take as many as BM_MAX_USAGE_COUNT+1 complete cycles of the clock-sweep hand to find a free buffer". That cost is the problem. The extra resolution only distinguishes buffers by how many times they were touched *between passes of the hand*, which on a large pool is a long and load-dependent interval; what it reliably buys is up to five revolutions of scanning before a hot buffer can be reclaimed. One bit keeps the property that matters -- a recently used buffer survives a visit -- and makes the work needed to find a victim independent of how hot the pool has been. Admission policy is deliberately unchanged here: a demand-loaded page arrives with its chance already granted, just as it previously arrived with a count of 1. A later commit makes admission probationary. The field itself is gone rather than narrowed. BUF_USAGECOUNT_BITS, _SHIFT, _MASK and _ONE, BUF_STATE_GET_USAGECOUNT and BM_MAX_USAGE_COUNT are all removed, and the state is now a buffer flag in the bit position the counter used to occupy (flag 6, free since b0f4ff3c9). That reclaims four bits of the buffer state word and lets the existing flag machinery carry it: the two victim-reuse paths in InvalidateBuffer() and InvalidateVictimBuffer() already cleared BUF_FLAG_MASK and BUF_USAGECOUNT_MASK in the same UnlockBufHdrExt() call, so the second half of those masks simply goes away. It also retires the note on UnlockBufHdrExt() explaining that flags machinery could not carry the usagecount because the value needed capping -- with one bit there is nothing to cap. pg_buffercache keeps its user-visible shape. The usagecount column and pg_buffercache_usage_counts() are documented API; they keep their names and their types, and now report 0 or 1. The extension carries its own bound for the number of rows the latter returns instead of borrowing a backend constant. --- contrib/pg_buffercache/pg_buffercache_pages.c | 20 ++++--- doc/src/sgml/pgbuffercache.sgml | 8 ++- src/backend/storage/buffer/bufmgr.c | 27 +++++---- src/backend/storage/buffer/freelist.c | 34 +++++++---- src/backend/storage/buffer/localbuf.c | 18 +++--- src/include/storage/buf_internals.h | 57 +++++++++---------- 6 files changed, 89 insertions(+), 75 deletions(-) diff --git a/contrib/pg_buffercache/pg_buffercache_pages.c b/contrib/pg_buffercache/pg_buffercache_pages.c index 46b6b85a2e3..9592556a406 100644 --- a/contrib/pg_buffercache/pg_buffercache_pages.c +++ b/contrib/pg_buffercache/pg_buffercache_pages.c @@ -23,6 +23,12 @@ #define NUM_BUFFERCACHE_PAGES_ELEM 9 #define NUM_BUFFERCACHE_SUMMARY_ELEM 5 #define NUM_BUFFERCACHE_USAGE_COUNTS_ELEM 4 + +/* + * Highest value the usagecount columns can report. The clock sweep tracks one + * second chance per buffer, so the reported count is 0 or 1. + */ +#define PGBUFFERCACHE_MAX_USAGE_COUNT 1 #define NUM_BUFFERCACHE_EVICT_ELEM 2 #define NUM_BUFFERCACHE_EVICT_RELATION_ELEM 3 #define NUM_BUFFERCACHE_EVICT_ALL_ELEM 3 @@ -163,7 +169,7 @@ pg_buffercache_pages(PG_FUNCTION_ARGS) reldatabase = bufHdr->tag.dbOid; forknum = BufTagGetForkNum(&bufHdr->tag); blocknum = bufHdr->tag.blockNum; - usagecount = BUF_STATE_GET_USAGECOUNT(buf_state); + usagecount = (BUF_STATE_HAS_SECOND_CHANCE(buf_state) ? 1 : 0); pinning_backends = BUF_STATE_GET_REFCOUNT(buf_state); if (buf_state & BM_DIRTY) @@ -607,7 +613,7 @@ pg_buffercache_summary(PG_FUNCTION_ARGS) if (buf_state & BM_VALID) { buffers_used++; - usagecount_total += BUF_STATE_GET_USAGECOUNT(buf_state); + usagecount_total += (BUF_STATE_HAS_SECOND_CHANCE(buf_state) ? 1 : 0); if (buf_state & BM_DIRTY) buffers_dirty++; @@ -641,9 +647,9 @@ Datum pg_buffercache_usage_counts(PG_FUNCTION_ARGS) { ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; - int usage_counts[BM_MAX_USAGE_COUNT + 1] = {0}; - int dirty[BM_MAX_USAGE_COUNT + 1] = {0}; - int pinned[BM_MAX_USAGE_COUNT + 1] = {0}; + int usage_counts[PGBUFFERCACHE_MAX_USAGE_COUNT + 1] = {0}; + int dirty[PGBUFFERCACHE_MAX_USAGE_COUNT + 1] = {0}; + int pinned[PGBUFFERCACHE_MAX_USAGE_COUNT + 1] = {0}; Datum values[NUM_BUFFERCACHE_USAGE_COUNTS_ELEM]; bool nulls[NUM_BUFFERCACHE_USAGE_COUNTS_ELEM] = {0}; @@ -657,7 +663,7 @@ pg_buffercache_usage_counts(PG_FUNCTION_ARGS) CHECK_FOR_INTERRUPTS(); - usage_count = BUF_STATE_GET_USAGECOUNT(buf_state); + usage_count = (BUF_STATE_HAS_SECOND_CHANCE(buf_state) ? 1 : 0); usage_counts[usage_count]++; if (buf_state & BM_DIRTY) @@ -667,7 +673,7 @@ pg_buffercache_usage_counts(PG_FUNCTION_ARGS) pinned[usage_count]++; } - for (int i = 0; i < BM_MAX_USAGE_COUNT + 1; i++) + for (int i = 0; i < PGBUFFERCACHE_MAX_USAGE_COUNT + 1; i++) { values[0] = Int32GetDatum(i); values[1] = Int32GetDatum(usage_counts[i]); diff --git a/doc/src/sgml/pgbuffercache.sgml b/doc/src/sgml/pgbuffercache.sgml index 1e9aee10275..f751b55e754 100644 --- a/doc/src/sgml/pgbuffercache.sgml +++ b/doc/src/sgml/pgbuffercache.sgml @@ -99,7 +99,9 @@ The pg_buffercache_usage_counts() function returns a set of records, each row describing the number of buffers with a given usage - count. + count. The clock sweep tracks one access per buffer, so there are two rows: + usage count 0 for buffers that are candidates for replacement, and 1 for + those that will be passed over once more. @@ -240,7 +242,9 @@ usagecount smallint - Clock-sweep access count + Clock-sweep access count: 1 if the buffer has been accessed since the + clock hand last passed it, and so will be passed over once more, or 0 + if it is a candidate for replacement diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index f81c7732e68..ea3220e43db 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -2335,7 +2335,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, * checkpoints, except for their "init" forks, which need to be treated * just like permanent relations. */ - set_bits |= BM_TAG_VALID | BUF_USAGECOUNT_ONE; + set_bits |= BM_TAG_VALID | BM_SECOND_CHANCE; if (relpersistence == RELPERSISTENCE_PERMANENT || forkNum == INIT_FORKNUM) set_bits |= BM_PERMANENT; @@ -2445,7 +2445,7 @@ retry: UnlockBufHdrExt(buf, buf_state, 0, - BUF_FLAG_MASK | BUF_USAGECOUNT_MASK, + BUF_FLAG_MASK, 0); /* @@ -2528,7 +2528,7 @@ InvalidateVictimBuffer(BufferDesc *buf_hdr) ClearBufferTag(&buf_hdr->tag); UnlockBufHdrExt(buf_hdr, buf_state, 0, - BUF_FLAG_MASK | BUF_USAGECOUNT_MASK, + BUF_FLAG_MASK, 0); Assert(BUF_STATE_GET_REFCOUNT(buf_state) > 0); @@ -3004,7 +3004,7 @@ ExtendBufferedRelShared(BufferManagerRelation bmr, victim_buf_hdr->tag = tag; - set_bits |= BM_TAG_VALID | BUF_USAGECOUNT_ONE; + set_bits |= BM_TAG_VALID | BM_SECOND_CHANCE; if (bmr.relpersistence == RELPERSISTENCE_PERMANENT || fork == INIT_FORKNUM) set_bits |= BM_PERMANENT; @@ -3336,18 +3336,21 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy, if (strategy == NULL) { - /* Default case: increase usagecount unless already max. */ - if (BUF_STATE_GET_USAGECOUNT(buf_state) < BM_MAX_USAGE_COUNT) - buf_state += BUF_USAGECOUNT_ONE; + /* Default case: grant the buffer its second chance. */ + buf_state |= BM_SECOND_CHANCE; } else { /* - * Ring buffers shouldn't evict others from pool. Thus we - * don't make usagecount more than 1. + * Ring buffers: a page is pinned once per cycle through the + * ring by the scan that owns it, and those pins must not keep + * it resident -- otherwise a sequential scan would protect its + * own pages and evict the working set. With a single bit + * there is nothing to cap: leave the bit alone and let the + * ring recycle the page, while an access from outside the ring + * (which takes the default path above) grants the chance and + * so removes the page from the ring's reach. */ - if (BUF_STATE_GET_USAGECOUNT(buf_state) == 0) - buf_state += BUF_USAGECOUNT_ONE; } if (pg_atomic_compare_exchange_u64(&buf->state, &old_buf_state, @@ -4174,7 +4177,7 @@ SyncOneBuffer(int buf_id, bool skip_recently_used, WritebackContext *wb_context) buf_state = LockBufHdr(bufHdr); if (BUF_STATE_GET_REFCOUNT(buf_state) == 0 && - BUF_STATE_GET_USAGECOUNT(buf_state) == 0) + !BUF_STATE_HAS_SECOND_CHANCE(buf_state)) { result |= BUF_REUSABLE; } diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c index 53ef5239e8d..c58ed0c79eb 100644 --- a/src/backend/storage/buffer/freelist.c +++ b/src/backend/storage/buffer/freelist.c @@ -264,11 +264,8 @@ StrategyGetBuffer(BufferAccessStrategy strategy, uint64 *buf_state, bool *from_r local_buf_state = old_buf_state; /* - * If the buffer is pinned or has a nonzero usage_count, we cannot - * use it; decrement the usage_count (unless pinned) and keep - * scanning. + * If the buffer is pinned we cannot use it; keep scanning. */ - if (BUF_STATE_GET_REFCOUNT(local_buf_state) != 0) { if (--trycounter == 0) @@ -292,9 +289,17 @@ StrategyGetBuffer(BufferAccessStrategy strategy, uint64 *buf_state, bool *from_r continue; } - if (BUF_STATE_GET_USAGECOUNT(local_buf_state) != 0) + if (BUF_STATE_HAS_SECOND_CHANCE(local_buf_state)) { - local_buf_state -= BUF_USAGECOUNT_ONE; + /* + * The buffer has been used since the hand last passed it. + * Spend its second chance and keep scanning: it becomes a + * candidate only if it is still untouched when the hand comes + * back around, which gives it a full revolution of grace. + * Spending a chance is progress toward finding a victim, so + * reset trycounter. + */ + local_buf_state &= ~BM_SECOND_CHANCE; if (pg_atomic_compare_exchange_u64(&buf->state, &old_buf_state, local_buf_state)) @@ -664,14 +669,19 @@ GetBufferFromRing(BufferAccessStrategy strategy, uint64 *buf_state) /* * If the buffer is pinned we cannot use it under any circumstances. * - * If usage_count is 0 or 1 then the buffer is fair game (we expect 1, - * since our own previous usage of the ring element would have left it - * there, but it might've been decremented by clock-sweep since then). - * A higher usage_count indicates someone else has touched the buffer, - * so we shouldn't re-use it. + * The buffer is fair game only if nothing has touched it since we + * last cycled past this ring slot. If it still has its second + * chance, an access from outside the ring has claimed it for the + * working set and we must not recycle it. + * + * This replaces the stock "usage_count > 1 means somebody else + * touched it" test. That needed a count only because a pin taken + * from inside the ring was indistinguishable from one taken outside + * it; now the ring does not grant the bit at all, so its own pins + * leave no trace and a single bit is enough. */ if (BUF_STATE_GET_REFCOUNT(local_buf_state) != 0 - || BUF_STATE_GET_USAGECOUNT(local_buf_state) > 1) + || BUF_STATE_HAS_SECOND_CHANCE(local_buf_state)) break; /* See equivalent code in PinBuffer() */ diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c index 4870c8e13d0..9c6035a62d4 100644 --- a/src/backend/storage/buffer/localbuf.c +++ b/src/backend/storage/buffer/localbuf.c @@ -166,8 +166,8 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum, bufHdr->tag = newTag; buf_state = pg_atomic_read_u64(&bufHdr->state); - buf_state &= ~(BUF_FLAG_MASK | BUF_USAGECOUNT_MASK); - buf_state |= BM_TAG_VALID | BUF_USAGECOUNT_ONE; + buf_state &= ~BUF_FLAG_MASK; + buf_state |= BM_TAG_VALID | BM_SECOND_CHANCE; pg_atomic_unlocked_write_u64(&bufHdr->state, buf_state); *foundPtr = false; @@ -248,9 +248,9 @@ GetLocalVictimBuffer(void) { uint64 buf_state = pg_atomic_read_u64(&bufHdr->state); - if (BUF_STATE_GET_USAGECOUNT(buf_state) > 0) + if (BUF_STATE_HAS_SECOND_CHANCE(buf_state)) { - buf_state -= BUF_USAGECOUNT_ONE; + buf_state &= ~BM_SECOND_CHANCE; pg_atomic_unlocked_write_u64(&bufHdr->state, buf_state); trycounter = NLocBuffer; } @@ -454,7 +454,7 @@ ExtendBufferedRelLocal(BufferManagerRelation bmr, victim_buf_hdr->tag = tag; - buf_state |= BM_TAG_VALID | BUF_USAGECOUNT_ONE; + buf_state |= BM_TAG_VALID | BM_SECOND_CHANCE; pg_atomic_unlocked_write_u64(&victim_buf_hdr->state, buf_state); @@ -670,7 +670,6 @@ InvalidateLocalBuffer(BufferDesc *bufHdr, bool check_unreferenced) /* Mark buffer invalid */ ClearBufferTag(&bufHdr->tag); buf_state &= ~BUF_FLAG_MASK; - buf_state &= ~BUF_USAGECOUNT_MASK; pg_atomic_unlocked_write_u64(&bufHdr->state, buf_state); } @@ -838,11 +837,8 @@ PinLocalBuffer(BufferDesc *buf_hdr, bool adjust_usagecount) { NLocalPinnedBuffers++; buf_state += BUF_REFCOUNT_ONE; - if (adjust_usagecount && - BUF_STATE_GET_USAGECOUNT(buf_state) < BM_MAX_USAGE_COUNT) - { - buf_state += BUF_USAGECOUNT_ONE; - } + if (adjust_usagecount) + buf_state |= BM_SECOND_CHANCE; pg_atomic_unlocked_write_u64(&buf_hdr->state, buf_state); /* diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index 87d7d5124da..ba640eb1001 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -35,7 +35,6 @@ * * State of the buffer itself (in order): * - 18 bits refcount - * - 4 bits usage count * - 12 bits of flags * - 18 bits share-lock count * - 1 bit share-exclusive locked @@ -47,11 +46,10 @@ * The definition of buffer state components is below. */ #define BUF_REFCOUNT_BITS 18 -#define BUF_USAGECOUNT_BITS 4 #define BUF_FLAG_BITS 12 #define BUF_LOCK_BITS (18+2) -StaticAssertDecl(BUF_REFCOUNT_BITS + BUF_USAGECOUNT_BITS + BUF_FLAG_BITS + BUF_LOCK_BITS <= 64, +StaticAssertDecl(BUF_REFCOUNT_BITS + BUF_FLAG_BITS + BUF_LOCK_BITS <= 64, "parts of buffer state space need to be <= 64"); /* refcount related definitions */ @@ -59,17 +57,9 @@ StaticAssertDecl(BUF_REFCOUNT_BITS + BUF_USAGECOUNT_BITS + BUF_FLAG_BITS + BUF_L #define BUF_REFCOUNT_MASK \ ((UINT64CONST(1) << BUF_REFCOUNT_BITS) - 1) -/* usage count related definitions */ -#define BUF_USAGECOUNT_SHIFT \ - BUF_REFCOUNT_BITS -#define BUF_USAGECOUNT_MASK \ - (((UINT64CONST(1) << BUF_USAGECOUNT_BITS) - 1) << (BUF_USAGECOUNT_SHIFT)) -#define BUF_USAGECOUNT_ONE \ - (UINT64CONST(1) << BUF_REFCOUNT_BITS) - /* flags related definitions */ #define BUF_FLAG_SHIFT \ - (BUF_REFCOUNT_BITS + BUF_USAGECOUNT_BITS) + BUF_REFCOUNT_BITS #define BUF_FLAG_MASK \ (((UINT64CONST(1) << BUF_FLAG_BITS) - 1) << BUF_FLAG_SHIFT) @@ -86,11 +76,19 @@ StaticAssertDecl(BUF_REFCOUNT_BITS + BUF_USAGECOUNT_BITS + BUF_FLAG_BITS + BUF_L ((((uint64) MAX_BACKENDS) << BM_LOCK_SHIFT) | BM_LOCK_VAL_SHARE_EXCLUSIVE | BM_LOCK_VAL_EXCLUSIVE) -/* Get refcount and usagecount from buffer state */ +/* Get refcount from buffer state */ #define BUF_STATE_GET_REFCOUNT(state) \ ((uint32)((state) & BUF_REFCOUNT_MASK)) -#define BUF_STATE_GET_USAGECOUNT(state) \ - ((uint32)(((state) & BUF_USAGECOUNT_MASK) >> BUF_USAGECOUNT_SHIFT)) + +/* + * Does this buffer still have its second chance? + * + * The clock sweep reclaims a buffer only on a visit that finds the bit + * already clear, so a buffer that is accessed between two visits is always + * passed over once. See BM_SECOND_CHANCE. + */ +#define BUF_STATE_HAS_SECOND_CHANCE(state) \ + (((state) & BM_SECOND_CHANCE) != 0) /* * Flags for buffer descriptors @@ -114,7 +112,19 @@ StaticAssertDecl(BUF_REFCOUNT_BITS + BUF_USAGECOUNT_BITS + BUF_FLAG_BITS + BUF_L #define BM_IO_IN_PROGRESS BUF_DEFINE_FLAG( 4) /* previous I/O failed */ #define BM_IO_ERROR BUF_DEFINE_FLAG( 5) -/* flag bit 6 is not used anymore */ +/* + * Set when a buffer is pinned, cleared by the clock sweep as the hand passes. + * The sweep reclaims a buffer only if it finds this bit already clear, so any + * buffer touched since the hand last passed it survives one more revolution: + * the classic second chance. + * + * This occupies the bit position of the 0..5 usage counter it replaces. The + * counter let a buffer accumulate up to five chances, which cost a full + * revolution each to spend; one chance is enough to protect a working set and + * makes the work the sweep must do to find a victim independent of how hot the + * pool has been. + */ +#define BM_SECOND_CHANCE BUF_DEFINE_FLAG( 6) /* have waiter for sole pin */ #define BM_PIN_COUNT_WAITER BUF_DEFINE_FLAG( 7) /* must write for checkpoint */ @@ -133,18 +143,6 @@ StaticAssertDecl(MAX_BACKENDS_BITS <= (BUF_LOCK_BITS - 2), "MAX_BACKENDS_BITS needs to be <= BUF_LOCK_BITS - 2"); -/* - * The maximum allowed value of usage_count represents a tradeoff between - * accuracy and speed of the clock-sweep buffer management algorithm. A - * large value (comparable to NBuffers) would approximate LRU semantics. - * But it can take as many as BM_MAX_USAGE_COUNT+1 complete cycles of the - * clock-sweep hand to find a free buffer, so in practice we don't want the - * value to be very large. - */ -#define BM_MAX_USAGE_COUNT 5 - -StaticAssertDecl(BM_MAX_USAGE_COUNT < (UINT64CONST(1) << BUF_USAGECOUNT_BITS), - "BM_MAX_USAGE_COUNT doesn't fit in BUF_USAGECOUNT_BITS bits"); /* * Buffer tag identifies which disk block the buffer contains. @@ -477,9 +475,6 @@ UnlockBufHdr(BufferDesc *desc) * unset_bits, adding the flags in set_bits, and changing the refcount by * refcount_change. If a flag is both cleared and added, it will end up being * set. - * - * Note that this approach would not trivially work for usagecount, since we - * need to cap the usagecount at BM_MAX_USAGE_COUNT. */ static inline uint64 UnlockBufHdrExt(BufferDesc *desc, uint64 old_buf_state, -- 2.56.0