From f2e8d9de5bd2dae9b16264cb77d92bcc8e5ec8df Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 12 Nov 2025 12:50:52 -0500 Subject: [PATCH v6 06/14] bufmgr: Separate keys for private refcount infrastructure This makes lookups faster, due to allowing auto-vectorized lookups. It is also beneficial for an upcoming patch, independent of auto-vectorization, as the upcoming patch wants to track more information for each pinned buffer, making the existing loop, iterating over an array of PrivateRefCountEntry, more expensive due to increasing its size. Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/storage/buffer/bufmgr.c | 123 ++++++++++++++++++---------- src/tools/pgindent/typedefs.list | 1 + 2 files changed, 79 insertions(+), 45 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index d4235ca7939..972eae1fc7b 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -90,10 +90,18 @@ */ #define BUF_DROP_FULL_SCAN_THRESHOLD (uint64) (NBuffers / 32) -typedef struct PrivateRefCountEntry +typedef struct PrivateRefCountData { - Buffer buffer; + /* + * How many times has the buffer been pinned by this backend. + */ int32 refcount; +} PrivateRefCountData; + +typedef struct PrivateRefCountEntry +{ + Buffer buffer; + PrivateRefCountData data; } PrivateRefCountEntry; /* 64 bytes, about the size of a cache line on common systems */ @@ -212,11 +220,12 @@ static BufferDesc *PinCountWaitBuf = NULL; * memory allocations in NewPrivateRefCountEntry() which can be important * because in some scenarios it's called with a spinlock held... */ +static Buffer PrivateRefCountArrayKeys[REFCOUNT_ARRAY_ENTRIES]; static struct PrivateRefCountEntry PrivateRefCountArray[REFCOUNT_ARRAY_ENTRIES]; static HTAB *PrivateRefCountHash = NULL; static int32 PrivateRefCountOverflowed = 0; static uint32 PrivateRefCountClock = 0; -static PrivateRefCountEntry *ReservedRefCountEntry = NULL; +static int ReservedRefCountSlot = -1; static uint32 MaxProportionalPins; @@ -259,7 +268,7 @@ static void ReservePrivateRefCountEntry(void) { /* Already reserved (or freed), nothing to do */ - if (ReservedRefCountEntry != NULL) + if (ReservedRefCountSlot != -1) return; /* @@ -271,16 +280,19 @@ ReservePrivateRefCountEntry(void) for (i = 0; i < REFCOUNT_ARRAY_ENTRIES; i++) { - PrivateRefCountEntry *res; - - res = &PrivateRefCountArray[i]; - - if (res->buffer == InvalidBuffer) + if (PrivateRefCountArrayKeys[i] == InvalidBuffer) { - ReservedRefCountEntry = res; - return; + ReservedRefCountSlot = i; + + /* + * We could return immediately, but iterating till the end of + * the array allows compiler-autovectorization. + */ } } + + if (ReservedRefCountSlot != -1) + return; } /* @@ -292,27 +304,34 @@ ReservePrivateRefCountEntry(void) * Move entry from the current clock position in the array into the * hashtable. Use that slot. */ + int victim_slot; + PrivateRefCountEntry *victim_entry; PrivateRefCountEntry *hashent; bool found; /* select victim slot */ - ReservedRefCountEntry = - &PrivateRefCountArray[PrivateRefCountClock++ % REFCOUNT_ARRAY_ENTRIES]; + victim_slot = PrivateRefCountClock++ % REFCOUNT_ARRAY_ENTRIES; + victim_entry = &PrivateRefCountArray[victim_slot]; + ReservedRefCountSlot = victim_slot; /* Better be used, otherwise we shouldn't get here. */ - Assert(ReservedRefCountEntry->buffer != InvalidBuffer); + Assert(PrivateRefCountArrayKeys[victim_slot] != InvalidBuffer); + Assert(PrivateRefCountArray[victim_slot].buffer != InvalidBuffer); + Assert(PrivateRefCountArrayKeys[victim_slot] == PrivateRefCountArray[victim_slot].buffer); /* enter victim array entry into hashtable */ hashent = hash_search(PrivateRefCountHash, - &(ReservedRefCountEntry->buffer), + &PrivateRefCountArrayKeys[victim_slot], HASH_ENTER, &found); Assert(!found); - hashent->refcount = ReservedRefCountEntry->refcount; + hashent->data = victim_entry->data; /* clear the now free array slot */ - ReservedRefCountEntry->buffer = InvalidBuffer; - ReservedRefCountEntry->refcount = 0; + PrivateRefCountArrayKeys[victim_slot] = InvalidBuffer; + victim_entry->buffer = InvalidBuffer; + memset(&victim_entry->data, 0, sizeof(victim_entry->data)); + victim_entry->data.refcount = 0; PrivateRefCountOverflowed++; } @@ -327,15 +346,17 @@ NewPrivateRefCountEntry(Buffer buffer) PrivateRefCountEntry *res; /* only allowed to be called when a reservation has been made */ - Assert(ReservedRefCountEntry != NULL); + Assert(ReservedRefCountSlot != -1); /* use up the reserved entry */ - res = ReservedRefCountEntry; - ReservedRefCountEntry = NULL; + res = &PrivateRefCountArray[ReservedRefCountSlot]; /* and fill it */ + PrivateRefCountArrayKeys[ReservedRefCountSlot] = buffer; res->buffer = buffer; - res->refcount = 0; + res->data.refcount = 0; + + ReservedRefCountSlot = -1; return res; } @@ -347,10 +368,11 @@ NewPrivateRefCountEntry(Buffer buffer) * do_move is true, and the entry resides in the hashtable the entry is * optimized for frequent access by moving it to the array. */ -static PrivateRefCountEntry * +static inline PrivateRefCountEntry * GetPrivateRefCountEntry(Buffer buffer, bool do_move) { PrivateRefCountEntry *res; + int match = -1; int i; Assert(BufferIsValid(buffer)); @@ -362,12 +384,16 @@ GetPrivateRefCountEntry(Buffer buffer, bool do_move) */ for (i = 0; i < REFCOUNT_ARRAY_ENTRIES; i++) { - res = &PrivateRefCountArray[i]; - - if (res->buffer == buffer) - return res; + if (PrivateRefCountArrayKeys[i] == buffer) + { + match = i; + /* see ReservePrivateRefCountEntry() for why we don't return */ + } } + if (match != -1) + return &PrivateRefCountArray[match]; + /* * By here we know that the buffer, if already pinned, isn't residing in * the array. @@ -397,14 +423,18 @@ GetPrivateRefCountEntry(Buffer buffer, bool do_move) ReservePrivateRefCountEntry(); /* Use up the reserved slot */ - Assert(ReservedRefCountEntry != NULL); - free = ReservedRefCountEntry; - ReservedRefCountEntry = NULL; + Assert(ReservedRefCountSlot != -1); + free = &PrivateRefCountArray[ReservedRefCountSlot]; + Assert(PrivateRefCountArrayKeys[ReservedRefCountSlot] == free->buffer); Assert(free->buffer == InvalidBuffer); /* and fill it */ free->buffer = buffer; - free->refcount = res->refcount; + free->data = res->data; + PrivateRefCountArrayKeys[ReservedRefCountSlot] = buffer; + + ReservedRefCountSlot = -1; + /* delete from hashtable */ hash_search(PrivateRefCountHash, &buffer, HASH_REMOVE, &found); @@ -437,7 +467,7 @@ GetPrivateRefCount(Buffer buffer) if (ref == NULL) return 0; - return ref->refcount; + return ref->data.refcount; } /* @@ -447,19 +477,21 @@ GetPrivateRefCount(Buffer buffer) static void ForgetPrivateRefCountEntry(PrivateRefCountEntry *ref) { - Assert(ref->refcount == 0); + Assert(ref->data.refcount == 0); if (ref >= &PrivateRefCountArray[0] && ref < &PrivateRefCountArray[REFCOUNT_ARRAY_ENTRIES]) { ref->buffer = InvalidBuffer; + PrivateRefCountArrayKeys[ref - PrivateRefCountArray] = InvalidBuffer; + /* * Mark the just used entry as reserved - in many scenarios that * allows us to avoid ever having to search the array/hash for free * entries. */ - ReservedRefCountEntry = ref; + ReservedRefCountSlot = ref - PrivateRefCountArray; } else { @@ -3073,7 +3105,7 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy, PrivateRefCountEntry *ref; Assert(!BufferIsLocal(b)); - Assert(ReservedRefCountEntry != NULL); + Assert(ReservedRefCountSlot != -1); ref = GetPrivateRefCountEntry(b, true); @@ -3145,8 +3177,8 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy, */ result = (pg_atomic_read_u64(&buf->state) & BM_VALID) != 0; - Assert(ref->refcount > 0); - ref->refcount++; + Assert(ref->data.refcount > 0); + ref->data.refcount++; ResourceOwnerRememberBuffer(CurrentResourceOwner, b); } @@ -3263,9 +3295,9 @@ UnpinBufferNoOwner(BufferDesc *buf) /* not moving as we're likely deleting it soon anyway */ ref = GetPrivateRefCountEntry(b, false); Assert(ref != NULL); - Assert(ref->refcount > 0); - ref->refcount--; - if (ref->refcount == 0) + Assert(ref->data.refcount > 0); + ref->data.refcount--; + if (ref->data.refcount == 0) { uint64 old_buf_state; @@ -3305,7 +3337,7 @@ TrackNewBufferPin(Buffer buf) PrivateRefCountEntry *ref; ref = NewPrivateRefCountEntry(buf); - ref->refcount++; + ref->data.refcount++; ResourceOwnerRememberBuffer(CurrentResourceOwner, buf); @@ -4018,6 +4050,7 @@ InitBufferManagerAccess(void) MaxProportionalPins = NBuffers / (MaxBackends + NUM_AUXILIARY_PROCS); memset(&PrivateRefCountArray, 0, sizeof(PrivateRefCountArray)); + memset(&PrivateRefCountArrayKeys, 0, sizeof(Buffer)); hash_ctl.keysize = sizeof(int32); hash_ctl.entrysize = sizeof(PrivateRefCountEntry); @@ -4067,10 +4100,10 @@ CheckForBufferLeaks(void) /* check the array */ for (i = 0; i < REFCOUNT_ARRAY_ENTRIES; i++) { - res = &PrivateRefCountArray[i]; - - if (res->buffer != InvalidBuffer) + if (PrivateRefCountArrayKeys[i] != InvalidBuffer) { + res = &PrivateRefCountArray[i]; + s = DebugPrintBufferRefcount(res->buffer); elog(WARNING, "buffer refcount leak: %s", s); pfree(s); @@ -5407,7 +5440,7 @@ IncrBufferRefCount(Buffer buffer) ref = GetPrivateRefCountEntry(buffer, true); Assert(ref != NULL); - ref->refcount++; + ref->data.refcount++; } ResourceOwnerRememberBuffer(CurrentResourceOwner, buffer); } diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 5769227e41c..9a89e68c59c 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2327,6 +2327,7 @@ PrintfArgValue PrintfTarget PrinttupAttrInfo PrivTarget +PrivateRefCountData PrivateRefCountEntry ProcArrayStruct ProcLangInfo -- 2.48.1.76.g4e746b1a31.dirty