From be1923e29a0ca53adae8853f7ca7b4534fa58258 Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Fri, 20 Dec 2024 18:38:52 +0100
Subject: [PATCH v0 2/2] Buftable: Reduce size of buffer table entries by 60%

By using the stored id as identifier of which BufferDesc we point to
and thus which BufferTag we need to compare against, we can remove
that BufferTag from the BufferLookupEnt.

On 32-bit systems, that reduces the per-element size from 32 bytes to 12
bytes, a 62% reduction. On 64-bit systems we additionally fit the now
4-byte entry data into the empty space of the hash table element, thus
reducing the per-element size down to 16 bytes from 40 bytes; a 60%
reduction.
---
 src/include/storage/buf_internals.h    |   2 +-
 src/backend/storage/buffer/buf_table.c | 112 ++++++++++++++++++++++---
 src/backend/storage/buffer/bufmgr.c    |   4 +-
 3 files changed, 105 insertions(+), 13 deletions(-)

diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h
index 1a65342177d..db752bf8368 100644
--- a/src/include/storage/buf_internals.h
+++ b/src/include/storage/buf_internals.h
@@ -439,7 +439,7 @@ extern void InitBufTable(int size);
 extern uint32 BufTableHashCode(BufferTag *tagPtr);
 extern int	BufTableLookup(BufferTag *tagPtr, uint32 hashcode);
 extern int	BufTableInsert(BufferTag *tagPtr, uint32 hashcode, int buf_id);
-extern void BufTableDelete(BufferTag *tagPtr, uint32 hashcode);
+extern void BufTableDelete(int buffer, uint32 hashcode);
 
 /* localbuf.c */
 extern bool PinLocalBuffer(BufferDesc *buf_hdr, bool adjust_usagecount);
diff --git a/src/backend/storage/buffer/buf_table.c b/src/backend/storage/buffer/buf_table.c
index a50955d5286..0cda2652807 100644
--- a/src/backend/storage/buffer/buf_table.c
+++ b/src/backend/storage/buffer/buf_table.c
@@ -22,16 +22,90 @@
 #include "postgres.h"
 
 #include "storage/buf_internals.h"
+#include "common/hashfn.h"
 
 /* entry for buffer lookup hashtable */
 typedef struct
 {
-	BufferTag	key;			/* Tag of a disk page */
-	int			id;				/* Associated buffer ID */
+	int			id;				/* Buffer offset into BufferDescriptors */
 } BufferLookupEnt;
 
+/*
+ * We don't store Buffer in BufTable, but offsets into BufferDescriptors, so
+ * we must use our own InvalidBuffer equivalent.
+ */
+#define SurrogateBuffer (-1)
+
 static HTAB *SharedBufHash;
 
+static BufferTag *MyBackendBufLookup;
+
+static inline BufferTag *
+BufTableIdToBufTag(int id)
+{
+	if (id == SurrogateBuffer)
+	{
+		Assert(PointerIsValid(MyBackendBufLookup));
+		return MyBackendBufLookup;
+	}
+	else
+	{
+		BufferDesc *buf = GetBufferDescriptor(id);
+		/* can't be in the buffer table if the tag ain't valid */
+		Assert(pg_atomic_read_u32(&buf->state) & BM_TAG_VALID);
+		return &buf->tag;
+	}
+}
+
+static uint32
+BufTableHashValue(const void *key, Size keysize)
+{
+	int id = *(int *) key;
+
+	/*
+	 * Key hashes are stored, and are provided during search/insert/delete
+	 * operations, so the only opportunity for this to happen is the hash
+	 * table doing hash operations (which we don't want to happen). Therefore,
+	 * assert we don't get called, but correctly handle other operations.
+	 */
+	Assert(false);
+
+	if (id == SurrogateBuffer)
+	{
+		if (MyBackendBufLookup == NULL)
+		{
+			elog(ERROR, "Surrogate buffer ID should have a lookup buffer tag set aside");
+		}
+
+		return tag_hash(MyBackendBufLookup, sizeof(BufferTag));
+	}
+
+	return tag_hash(BufTableIdToBufTag(id), keysize);
+}
+
+static int
+BufTableHashCompare(const void *key1, const void *key2, Size keysize)
+{
+	int id1 = *(int *) key1;
+	int id2 = *(int *) key2;
+
+	/*
+	 * Hash table searches always provide existing entries as argument 1;
+	 * and we can't have a missing ID as argument.
+	 */
+	Assert(id1 != SurrogateBuffer);
+
+	if (id1 == id2)
+		return 0;
+
+	/* we're looking for exactly this buffer ID */
+	if (id2 != SurrogateBuffer)
+		return false;
+
+	return memcmp(BufTableIdToBufTag(id1),
+				  BufTableIdToBufTag(id2),
+				  sizeof(BufferTag));
+}
 
 /*
  * Estimate space needed for mapping hashtable
@@ -40,7 +114,9 @@ static HTAB *SharedBufHash;
 Size
 BufTableShmemSize(int size)
 {
-	return hash_estimate_size(size, sizeof(BufferLookupEnt));
+	return hash_estimate_size_aligned(size,
+									  sizeof(BufferLookupEnt),
+									  ALIGNOF_INT);
 }
 
 /*
@@ -55,14 +131,18 @@ InitBufTable(int size)
 	/* assume no locking is needed yet */
 
 	/* BufferTag maps to Buffer */
-	info.keysize = sizeof(BufferTag);
+	info.keysize = sizeof(BufferLookupEnt);
 	info.entrysize = sizeof(BufferLookupEnt);
+	info.entryalign = ALIGNOF_INT;
 	info.num_partitions = NUM_BUFFER_PARTITIONS;
+	info.hash = BufTableHashValue;
+	info.match = BufTableHashCompare;
 
 	SharedBufHash = ShmemInitHash("Shared Buffer Lookup Table",
 								  size, size,
 								  &info,
-								  HASH_ELEM | HASH_BLOBS | HASH_PARTITION);
+								  HASH_ELEM | HASH_PARTITION | HASH_FUNCTION
+								  | HASH_COMPARE | HASH_ALIGN);
 }
 
 /*
@@ -77,7 +157,7 @@ InitBufTable(int size)
 uint32
 BufTableHashCode(BufferTag *tagPtr)
 {
-	return get_hash_value(SharedBufHash, tagPtr);
+	return tag_hash(tagPtr, sizeof(BufferTag));
 }
 
 /*
@@ -89,15 +169,19 @@ BufTableHashCode(BufferTag *tagPtr)
 int
 BufTableLookup(BufferTag *tagPtr, uint32 hashcode)
 {
+	const int	surrogateid = SurrogateBuffer;
 	BufferLookupEnt *result;
+	MyBackendBufLookup = tagPtr;
 
 	result = (BufferLookupEnt *)
 		hash_search_with_hash_value(SharedBufHash,
-									tagPtr,
+									&surrogateid,
 									hashcode,
 									HASH_FIND,
 									NULL);
 
+	MyBackendBufLookup = NULL;
+
 	if (!result)
 		return -1;
 
@@ -117,21 +201,29 @@ BufTableLookup(BufferTag *tagPtr, uint32 hashcode)
 int
 BufTableInsert(BufferTag *tagPtr, uint32 hashcode, int buf_id)
 {
+	const int	surrogateid = SurrogateBuffer;
 	BufferLookupEnt *result;
 	bool		found;
 
 	Assert(buf_id >= 0);		/* -1 is reserved for not-in-table */
 	Assert(tagPtr->blockNum != P_NEW);	/* invalid tag */
 
+	MyBackendBufLookup = tagPtr;
+
 	result = (BufferLookupEnt *)
 		hash_search_with_hash_value(SharedBufHash,
-									tagPtr,
+									&surrogateid,
 									hashcode,
 									HASH_ENTER,
 									&found);
 
+	MyBackendBufLookup = NULL;
+
 	if (found)					/* found something already in the table */
+	{
+		Assert(result->id != SurrogateBuffer);
 		return result->id;
+	}
 
 	result->id = buf_id;
 
@@ -145,13 +237,13 @@ BufTableInsert(BufferTag *tagPtr, uint32 hashcode, int buf_id)
  * Caller must hold exclusive lock on BufMappingLock for tag's partition
  */
 void
-BufTableDelete(BufferTag *tagPtr, uint32 hashcode)
+BufTableDelete(int buffer, uint32 hashcode)
 {
 	BufferLookupEnt *result;
 
 	result = (BufferLookupEnt *)
 		hash_search_with_hash_value(SharedBufHash,
-									tagPtr,
+									&buffer,
 									hashcode,
 									HASH_REMOVE,
 									NULL);
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 5008641baff..25398467a2b 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1856,7 +1856,7 @@ retry:
 	 * Remove the buffer from the lookup hashtable, if it was in there.
 	 */
 	if (oldFlags & BM_TAG_VALID)
-		BufTableDelete(&oldTag, oldHash);
+		BufTableDelete(buf->buf_id, oldHash);
 
 	/*
 	 * Done with mapping lock.
@@ -1935,7 +1935,7 @@ InvalidateVictimBuffer(BufferDesc *buf_hdr)
 	Assert(BUF_STATE_GET_REFCOUNT(buf_state) > 0);
 
 	/* finally delete buffer from the buffer mapping table */
-	BufTableDelete(&tag, hash);
+	BufTableDelete(buf_hdr->buf_id, hash);
 
 	LWLockRelease(partition_lock);
 
-- 
2.45.2

