Split the buffer mapping table into multiple separately lockable

partitions, as per discussion.  Passes functionality checks, but
I don't have any performance data yet.

Author: tglsfdc

contrib/pg_buffercache/pg_buffercache_pages.c

@@ -3,7 +3,7 @@
  * pg_buffercache_pages.c
  *	  display some contents of the buffer cache
  *
- *	  $PostgreSQL: pgsql/contrib/pg_buffercache/pg_buffercache_pages.c,v 1.7 2006/05/30 22:12:13 tgl Exp $
+ *	  $PostgreSQL: pgsql/contrib/pg_buffercache/pg_buffercache_pages.c,v 1.8 2006/07/23 03:07:57 tgl Exp $
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
@@ -74,7 +74,7 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 
 	if (SRF_IS_FIRSTCALL())
 	{
-		uint32		i;
+		int		i;
 		volatile BufferDesc *bufHdr;
 
 		funcctx = SRF_FIRSTCALL_INIT();
@@ -108,7 +108,6 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 		funcctx->max_calls = NBuffers;
 		funcctx->user_fctx = fctx;
 
-
 		/* Allocate NBuffers worth of BufferCachePagesRec records. */
 		fctx->record = (BufferCachePagesRec *) palloc(sizeof(BufferCachePagesRec) * NBuffers);
 
@@ -120,17 +119,21 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 		fctx->values[4] = (char *) palloc(3 * sizeof(uint32) + 1);
 		fctx->values[5] = (char *) palloc(2);
 
-
 		/* Return to original context when allocating transient memory */
 		MemoryContextSwitchTo(oldcontext);
 
-
 		/*
-		 * Lock Buffer map and scan though all the buffers, saving the
-		 * relevant fields in the fctx->record structure.
+		 * To get a consistent picture of the buffer state, we must lock
+		 * all partitions of the buffer map.  Needless to say, this is
+		 * horrible for concurrency...
 		 */
-		LWLockAcquire(BufMappingLock, LW_SHARED);
+		for (i = 0; i < NUM_BUFFER_PARTITIONS; i++)
+			LWLockAcquire(FirstBufMappingLock + i, LW_SHARED);
 
+		/*
+		 * Scan though all the buffers, saving the relevant fields in the
+		 * fctx->record structure.
+		 */
 		for (i = 0, bufHdr = BufferDescriptors; i < NBuffers; i++, bufHdr++)
 		{
 			/* Lock each buffer header before inspecting. */
@@ -157,15 +160,15 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 		}
 
 		/* Release Buffer map. */
-		LWLockRelease(BufMappingLock);
+		for (i = 0; i < NUM_BUFFER_PARTITIONS; i++)
+			LWLockRelease(FirstBufMappingLock + i);
 	}
 
 	funcctx = SRF_PERCALL_SETUP();
 
 	/* Get the saved state */
 	fctx = funcctx->user_fctx;
 
-
 	if (funcctx->call_cntr < funcctx->max_calls)
 	{
 		uint32		i = funcctx->call_cntr;

src/backend/storage/buffer/README

@@ -1,4 +1,4 @@
-$PostgreSQL: pgsql/src/backend/storage/buffer/README,v 1.10 2006/06/08 14:58:33 tgl Exp $
+$PostgreSQL: pgsql/src/backend/storage/buffer/README,v 1.11 2006/07/23 03:07:58 tgl Exp $
 
 Notes about shared buffer access rules
 --------------------------------------
@@ -114,6 +114,14 @@ operation that needs exclusive lock is reading in a page that was not
 in shared buffers already, which will require at least a kernel call
 and usually a wait for I/O, so it will be slow anyway.
 
+* As of PG 8.2, the BufMappingLock has been split into NUM_BUFFER_PARTITIONS
+separate locks, each guarding a portion of the buffer tag space.  This allows
+further reduction of contention in the normal code paths.  The partition
+that a particular buffer tag belongs to is determined from the low-order
+bits of the tag's hash value.  The rules stated above apply to each partition
+independently.  If it is necessary to lock more than one partition at a time,
+they must be locked in partition-number order to avoid risk of deadlock.
+
 * A separate system-wide LWLock, the BufFreelistLock, provides mutual
 exclusion for operations that access the buffer free list or select
 buffers for replacement.  This is always taken in exclusive mode since

src/backend/storage/buffer/buf_table.c

@@ -4,16 +4,18 @@
  *	  routines for mapping BufferTags to buffer indexes.
  *
  * Note: the routines in this file do no locking of their own.	The caller
- * must hold a suitable lock on the BufMappingLock, as specified in the
- * comments.
+ * must hold a suitable lock on the appropriate BufMappingLock, as specified
+ * in the comments.  We can't do the locking inside these functions because
+ * in most cases the caller needs to adjust the buffer header contents
+ * before the lock is released (see notes in README).
  *
  *
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/buffer/buf_table.c,v 1.46 2006/07/14 16:59:19 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/buffer/buf_table.c,v 1.47 2006/07/23 03:07:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -58,29 +60,49 @@ InitBufTable(int size)
 	info.keysize = sizeof(BufferTag);
 	info.entrysize = sizeof(BufferLookupEnt);
 	info.hash = tag_hash;
+	info.num_partitions = NUM_BUFFER_PARTITIONS;
 
 	SharedBufHash = ShmemInitHash("Shared Buffer Lookup Table",
 								  size, size,
 								  &info,
-								  HASH_ELEM | HASH_FUNCTION);
+								  HASH_ELEM | HASH_FUNCTION | HASH_PARTITION);
 
 	if (!SharedBufHash)
 		elog(FATAL, "could not initialize shared buffer hash table");
 }
 
+/*
+ * BufTableHashCode
+ *		Compute the hash code associated with a BufferTag
+ *
+ * This must be passed to the lookup/insert/delete routines along with the
+ * tag.  We do it like this because the callers need to know the hash code
+ * in order to determine which buffer partition to lock, and we don't want
+ * to do the hash computation twice (hash_any is a bit slow).
+ */
+uint32
+BufTableHashCode(BufferTag *tagPtr)
+{
+	return get_hash_value(SharedBufHash, (void *) tagPtr);
+}
+
 /*
  * BufTableLookup
  *		Lookup the given BufferTag; return buffer ID, or -1 if not found
  *
- * Caller must hold at least share lock on BufMappingLock
+ * Caller must hold at least share lock on BufMappingLock for tag's partition
  */
 int
-BufTableLookup(BufferTag *tagPtr)
+BufTableLookup(BufferTag *tagPtr, uint32 hashcode)
 {
 	BufferLookupEnt *result;
 
 	result = (BufferLookupEnt *)
-		hash_search(SharedBufHash, (void *) tagPtr, HASH_FIND, NULL);
+		hash_search_with_hash_value(SharedBufHash,
+									(void *) tagPtr,
+									hashcode,
+									HASH_FIND,
+									NULL);
 
 	if (!result)
 		return -1;
@@ -96,10 +118,10 @@ BufTableLookup(BufferTag *tagPtr)
  * Returns -1 on successful insertion.	If a conflicting entry exists
  * already, returns the buffer ID in that entry.
  *
- * Caller must hold write lock on BufMappingLock
+ * Caller must hold exclusive lock on BufMappingLock for tag's partition
  */
 int
-BufTableInsert(BufferTag *tagPtr, int buf_id)
+BufTableInsert(BufferTag *tagPtr, uint32 hashcode, int buf_id)
 {
 	BufferLookupEnt *result;
 	bool		found;
@@ -108,7 +130,11 @@ BufTableInsert(BufferTag *tagPtr, int buf_id)
 	Assert(tagPtr->blockNum != P_NEW);	/* invalid tag */
 
 	result = (BufferLookupEnt *)
-		hash_search(SharedBufHash, (void *) tagPtr, HASH_ENTER, &found);
+		hash_search_with_hash_value(SharedBufHash,
+									(void *) tagPtr,
+									hashcode,
+									HASH_ENTER,
+									&found);
 
 	if (found)					/* found something already in the table */
 		return result->id;
@@ -122,15 +148,19 @@ BufTableInsert(BufferTag *tagPtr, int buf_id)
  * BufTableDelete
  *		Delete the hashtable entry for given tag (which must exist)
  *
- * Caller must hold write lock on BufMappingLock
+ * Caller must hold exclusive lock on BufMappingLock for tag's partition
  */
 void
-BufTableDelete(BufferTag *tagPtr)
+BufTableDelete(BufferTag *tagPtr, uint32 hashcode)
 {
 	BufferLookupEnt *result;
 
 	result = (BufferLookupEnt *)
-		hash_search(SharedBufHash, (void *) tagPtr, HASH_REMOVE, NULL);
+		hash_search_with_hash_value(SharedBufHash,
+									(void *) tagPtr,
+									hashcode,
+									HASH_REMOVE,
+									NULL);
 
 	if (!result)				/* shouldn't happen */
 		elog(ERROR, "shared buffer hash table corrupted");