From 4b96a723bfc3a6fe78e05bc3ce454b0b160679d1 Mon Sep 17 00:00:00 2001 From: Alexandre Felipe Date: Wed, 11 Mar 2026 12:08:03 +0000 Subject: [PATCH 4/5] Compact PrivateRefCountEntry The previous implementation used an 8-bytes (64-bit) entry to store a uint32 count and an uint32 lock mode. That is fine when we store the data separate from the key (buffer). But in the simplehash {key, value} are stored together, so each entry is 12-bytes. This is somewhat awkward as we have to either pad the entry to 16-bytes, or the access will be an alternating aligned/misaligned addreses. Lock can assume only 4 values, and 2^30 is a decent limit for the number of pins on a single buffer. So this change is packing the {count[31:2], lock[1:0]} into a single uint32. Incrementing/decrementing the count continue the same, just using 4 instead of 1, lock mode access will require one or two additional bitwise operations. The exact count requires one shift, and is used only for debugging. A special function is provided to check whether count == 1. No bit-shifts are required. --- src/backend/storage/buffer/bufmgr_refcnt.c | 186 +++++++++------------ src/backend/storage/buffer/bufmgr_refcnt.h | 2 +- 2 files changed, 76 insertions(+), 112 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr_refcnt.c b/src/backend/storage/buffer/bufmgr_refcnt.c index dd95d953f6..bf84e9f62e 100644 --- a/src/backend/storage/buffer/bufmgr_refcnt.c +++ b/src/backend/storage/buffer/bufmgr_refcnt.c @@ -47,47 +47,45 @@ #include "bufmgr_refcnt.h" #include "storage/proc.h" -/* Structure definitions - internal to this file */ -typedef struct PrivateRefCountData +/* + * Implementation details - opaque to callers. + * Packed refcount and lockmode in a single uint32: + * Bits 0-1: lockmode (4 values: UNLOCK, SHARE, SHARE_EXCLUSIVE, EXCLUSIVE) + * Bits 2-31: refcount (30 bits, max ~1 billion) + */ +struct PrivateRefCountEntry { - int32 refcount; - BufferLockMode lockmode; -} PrivateRefCountData; + Buffer buffer; + uint32 data; +}; -struct PrivateRefCountEntry +#define PRIVATEREFCOUNT_LOCKMODE_MASK 0x3 +#define ONE_PRIVATE_REFERENCE 4 /* 1 << 2 */ + +#define PrivateRefCountIsZero(d) ((d) < ONE_PRIVATE_REFERENCE) + +struct PrivateRefCountIterator { - Buffer buffer; /* hash key - InvalidBuffer means empty */ - PrivateRefCountData data; + int array_index; + bool in_hash; + void *hash_status; }; -/* - * Define simplehash parameters for the overflow hash table. - * We use buffer == InvalidBuffer as the "empty" marker to avoid needing - * a separate status field. - */ +/* Define simplehash for private refcount overflow hash table */ #define SH_PREFIX refcount #define SH_ELEMENT_TYPE PrivateRefCountEntry #define SH_KEY_TYPE Buffer #define SH_KEY buffer -#define SH_HASH_KEY(tb, key) murmurhash32(key) +#define SH_HASH_KEY(tb, key) ((uint32) murmurhash32(key)) #define SH_EQUAL(tb, a, b) ((a) == (b)) #define SH_SCOPE static inline -#define SH_DEFINE -#define SH_DECLARE -/* Use buffer field as empty marker - no separate status needed */ #define SH_ENTRY_EMPTY(entry) ((entry)->buffer == InvalidBuffer) #define SH_MAKE_EMPTY(entry) ((entry)->buffer = InvalidBuffer) -#define SH_MAKE_IN_USE(entry) ((void)0) /* key assignment handles this */ +#define SH_MAKE_IN_USE(entry) ((void) 0) +#define SH_DECLARE +#define SH_DEFINE #include "lib/simplehash.h" -struct PrivateRefCountIterator -{ - int array_index; - bool in_hash; - refcount_iterator hash_iter; - bool hash_iter_valid; -}; - /* Private refcount array and keys */ #define REFCOUNT_ARRAY_ENTRIES 8 static Buffer PrivateRefCountArrayKeys[REFCOUNT_ARRAY_ENTRIES]; @@ -112,9 +110,9 @@ static int PrivateRefCountEntryLast = -1; static uint32 MaxProportionalPins = 0; /* Forward declarations */ -static void ReservePrivateRefCountEntry(void); +static void ReservePrivateRefCountEntry(Buffer buffer); static PrivateRefCountEntry *NewPrivateRefCountEntry(Buffer buffer); -static pg_noinline PrivateRefCountEntry *GetPrivateRefCountEntrySlow(Buffer buffer, bool do_move); +static pg_noinline PrivateRefCountEntry *GetPrivateRefCountEntrySlow(Buffer buffer); /* * Initialize private refcount tracking for this backend. @@ -133,7 +131,7 @@ InitPrivateRefCount(void) memset(&PrivateRefCountArray, 0, sizeof(PrivateRefCountArray)); memset(&PrivateRefCountArrayKeys, 0, sizeof(PrivateRefCountArrayKeys)); - PrivateRefCountHash = refcount_create(CurrentMemoryContext, 16, NULL); + PrivateRefCountHash = refcount_create(CurrentMemoryContext, 64, NULL); } /* @@ -141,15 +139,15 @@ InitPrivateRefCount(void) * entry. */ static void -ReservePrivateRefCountEntry(void) +ReservePrivateRefCountEntry(Buffer buffer) { /* Already reserved (or freed), nothing to do */ if (ReservedRefCountSlot != -1) return; /* - * First search for a free entry the array, that'll be sufficient in the - * majority of cases. + * First search for a free entry in the array, that'll be sufficient in + * the majority of cases. */ { int i; @@ -159,11 +157,9 @@ ReservePrivateRefCountEntry(void) if (PrivateRefCountArrayKeys[i] == InvalidBuffer) { ReservedRefCountSlot = i; + return; } } - - if (ReservedRefCountSlot != -1) - return; } /* @@ -196,10 +192,7 @@ ReservePrivateRefCountEntry(void) /* clear the now free array slot */ PrivateRefCountArrayKeys[victim_slot] = InvalidBuffer; victim_entry->buffer = InvalidBuffer; - - memset(&victim_entry->data, 0, sizeof(victim_entry->data)); - victim_entry->data.refcount = 0; - victim_entry->data.lockmode = BUFFER_LOCK_UNLOCK; + victim_entry->data = 0; PrivateRefCountOverflowed++; } @@ -222,8 +215,7 @@ NewPrivateRefCountEntry(Buffer buffer) /* and fill it */ PrivateRefCountArrayKeys[ReservedRefCountSlot] = buffer; res->buffer = buffer; - res->data.refcount = 0; - res->data.lockmode = BUFFER_LOCK_UNLOCK; + res->data = 0; /* update cache for the next lookup */ PrivateRefCountEntryLast = ReservedRefCountSlot; @@ -237,10 +229,9 @@ NewPrivateRefCountEntry(Buffer buffer) * Slow-path for GetSharedBufferEntry(). */ static pg_noinline PrivateRefCountEntry * -GetPrivateRefCountEntrySlow(Buffer buffer, bool do_move) +GetPrivateRefCountEntrySlow(Buffer buffer) { PrivateRefCountEntry *res; - int match = -1; int i; /* @@ -250,16 +241,11 @@ GetPrivateRefCountEntrySlow(Buffer buffer, bool do_move) { if (PrivateRefCountArrayKeys[i] == buffer) { - match = i; + PrivateRefCountEntryLast = i; + return &PrivateRefCountArray[i]; } } - if (likely(match != -1)) - { - PrivateRefCountEntryLast = match; - return &PrivateRefCountArray[match]; - } - /* * Only look up the buffer in the hashtable if we've previously overflowed. */ @@ -267,41 +253,11 @@ GetPrivateRefCountEntrySlow(Buffer buffer, bool do_move) return NULL; res = refcount_lookup(PrivateRefCountHash, buffer); - - if (res == NULL) - return NULL; - else if (!do_move) - { - return res; - } - else - { - /* move buffer from hashtable into the free array slot */ - PrivateRefCountEntry *free; - - ReservePrivateRefCountEntry(); - - Assert(ReservedRefCountSlot != -1); - free = &PrivateRefCountArray[ReservedRefCountSlot]; - Assert(free->buffer == InvalidBuffer); - - free->buffer = buffer; - free->data = res->data; - PrivateRefCountArrayKeys[ReservedRefCountSlot] = buffer; - PrivateRefCountEntryLast = ReservedRefCountSlot; - - ReservedRefCountSlot = -1; - - refcount_delete_item(PrivateRefCountHash, res); - Assert(PrivateRefCountOverflowed > 0); - PrivateRefCountOverflowed--; - - return free; - } + return res; } /* - * Return the PrivateRefCountEntry for the passed buffer. + * Return the PrivateRefCount entry for the passed buffer. * Returns NULL if the buffer is not currently pinned. */ static PrivateRefCountEntry * @@ -317,7 +273,7 @@ GetSharedBufferEntry(Buffer buffer) return &PrivateRefCountArray[PrivateRefCountEntryLast]; } - return GetPrivateRefCountEntrySlow(buffer, false); + return GetPrivateRefCountEntrySlow(buffer); } /* @@ -333,9 +289,9 @@ SharedBufferCreateRef(Buffer buffer) Assert(BufferIsValid(buffer)); Assert(!BufferIsLocal(buffer)); - ReservePrivateRefCountEntry(); + ReservePrivateRefCountEntry(buffer); ref = NewPrivateRefCountEntry(buffer); - ref->data.refcount++; + ref->data = ONE_PRIVATE_REFERENCE; return ref; } @@ -348,8 +304,8 @@ static void SharedBufferRefExisting(PrivateRefCountEntry *ref) { Assert(ref != NULL); - Assert(ref->data.refcount > 0); - ref->data.refcount++; + Assert(!PrivateRefCountIsZero(ref->data)); + ref->data += ONE_PRIVATE_REFERENCE; } /* @@ -361,14 +317,14 @@ static bool SharedBufferUnref(PrivateRefCountEntry *ref) { Assert(ref != NULL); - Assert(ref->data.refcount > 0); + Assert(!PrivateRefCountIsZero(ref->data)); - ref->data.refcount--; + ref->data -= ONE_PRIVATE_REFERENCE; - if (ref->data.refcount == 0) + if (PrivateRefCountIsZero(ref->data)) { /* No more references - clean up the entry */ - Assert(ref->data.lockmode == BUFFER_LOCK_UNLOCK); + Assert(SharedBufferGetLockMode(ref) == BUFFER_LOCK_UNLOCK); if (ref >= &PrivateRefCountArray[0] && ref < &PrivateRefCountArray[REFCOUNT_ARRAY_ENTRIES]) @@ -379,7 +335,8 @@ SharedBufferUnref(PrivateRefCountEntry *ref) } else { - refcount_delete_item(PrivateRefCountHash, ref); + /* could make slightly more efficient by using the pointer */ + refcount_delete(PrivateRefCountHash, ref->buffer); Assert(PrivateRefCountOverflowed > 0); PrivateRefCountOverflowed--; } @@ -390,25 +347,16 @@ SharedBufferUnref(PrivateRefCountEntry *ref) return false; } -/* - * Accessors for refcount entry fields. - */ -static int32 -SharedBufferRefCount(PrivateRefCountEntry *ref) -{ - return ref->data.refcount; -} - static BufferLockMode SharedBufferGetLockMode(PrivateRefCountEntry *ref) { - return ref->data.lockmode; + return (BufferLockMode)(ref->data & PRIVATEREFCOUNT_LOCKMODE_MASK); } static void SharedBufferSetLockMode(PrivateRefCountEntry *ref, BufferLockMode mode) { - ref->data.lockmode = mode; + ref->data = (ref->data & ~PRIVATEREFCOUNT_LOCKMODE_MASK) | mode; } /* @@ -417,7 +365,18 @@ SharedBufferSetLockMode(PrivateRefCountEntry *ref, BufferLockMode mode) static bool SharedBufferHasSingleRef(PrivateRefCountEntry *ref) { - return ref != NULL && ref->data.refcount == 1; + return ref != NULL && + ref->data >= ONE_PRIVATE_REFERENCE && + ref->data < (2 * ONE_PRIVATE_REFERENCE); +} + +/* + * Get the refcount for a buffer entry (for debug output only). + */ +static int32 +SharedBufferRefCount(PrivateRefCountEntry *ref) +{ + return (int32)(ref->data >> 2); } /* @@ -488,7 +447,7 @@ AssertBufferLocksPermitCatalogRead(void) if (buf == InvalidBuffer) continue; - AssertNotCatalogBufferLock(buf, res->data.lockmode); + AssertNotCatalogBufferLock(buf, SharedBufferGetLockMode(res)); } FreePrivateRefCountIterator(iter); } @@ -504,7 +463,7 @@ InitPrivateRefCountIterator(void) iter->array_index = 0; iter->in_hash = false; - iter->hash_iter_valid = false; + iter->hash_status = NULL; return iter; } @@ -530,20 +489,23 @@ GetNextPrivateRefCountEntry(PrivateRefCountIterator *iter) iter->in_hash = true; if (PrivateRefCountOverflowed > 0) { - refcount_start_iterate(PrivateRefCountHash, &iter->hash_iter); - iter->hash_iter_valid = true; + refcount_iterator *hiter = palloc(sizeof(refcount_iterator)); + + refcount_start_iterate(PrivateRefCountHash, hiter); + iter->hash_status = hiter; } } - if (iter->hash_iter_valid) + if (iter->hash_status != NULL) { PrivateRefCountEntry *res; - res = refcount_iterate(PrivateRefCountHash, &iter->hash_iter); + res = refcount_iterate(PrivateRefCountHash, (refcount_iterator *) iter->hash_status); if (res != NULL) return res; - iter->hash_iter_valid = false; + pfree(iter->hash_status); + iter->hash_status = NULL; } return NULL; @@ -555,6 +517,8 @@ GetNextPrivateRefCountEntry(PrivateRefCountIterator *iter) void FreePrivateRefCountIterator(PrivateRefCountIterator *iter) { + if (iter->hash_status != NULL) + pfree(iter->hash_status); pfree(iter); } diff --git a/src/backend/storage/buffer/bufmgr_refcnt.h b/src/backend/storage/buffer/bufmgr_refcnt.h index f2a6f45d84..8d31f6ee6f 100644 --- a/src/backend/storage/buffer/bufmgr_refcnt.h +++ b/src/backend/storage/buffer/bufmgr_refcnt.h @@ -39,8 +39,8 @@ static void SharedBufferRefExisting(PrivateRefCountEntry *ref); static bool SharedBufferUnref(PrivateRefCountEntry *ref); static BufferLockMode SharedBufferGetLockMode(PrivateRefCountEntry *ref); static void SharedBufferSetLockMode(PrivateRefCountEntry *ref, BufferLockMode mode); -static int32 SharedBufferRefCount(PrivateRefCountEntry *ref); static bool SharedBufferHasSingleRef(PrivateRefCountEntry *ref); +static int32 SharedBufferRefCount(PrivateRefCountEntry *ref); /* Pin limiting */ extern uint32 GetPinLimit(void); -- 2.34.1