From 4b96a723bfc3a6fe78e05bc3ce454b0b160679d1 Mon Sep 17 00:00:00 2001
From: Alexandre Felipe <o.alexandre.felipe@gmail.com>
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

