On 19.12.2020 10:53, Zhihong Yu wrote:
Hi,
w.r.t. the code in BufferAlloc(), the pointers are compared.
Should we instead compare the tranche Id of the two LWLock ?
Cheers
As far as LWlocks are stored in the array, comparing indexes in this
array (tranche Id) is equivalent to comparing element's pointers.
So I do not see any problem here.
Just as experiment I tried a version of BufferAlloc without double
locking (patch is attached).
I am not absolutely sure that my patch is correct: my main intention was
to estimate influence of this buffer reassignment on performance.
I just run standard pgbench for database with scale 100 and default
shared buffers size (256Mb). So there are should be a lot of page
replacements.
I do not see any noticeable difference:
vanilla: 13087.596845
patch: 13184.442130
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index ad0d1a9..91eb93d 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -70,6 +70,8 @@
#define RELS_BSEARCH_THRESHOLD 20
+#define NO_DOUBLE_BUFFER_LOCK 1
+
typedef struct PrivateRefCountEntry
{
Buffer buffer;
@@ -197,6 +199,7 @@ static PrivateRefCountEntry *NewPrivateRefCountEntry(Buffer buffer);
static PrivateRefCountEntry *GetPrivateRefCountEntry(Buffer buffer, bool do_move);
static inline int32 GetPrivateRefCount(Buffer buffer);
static void ForgetPrivateRefCountEntry(PrivateRefCountEntry *ref);
+static void InvalidateBuffer(BufferDesc *buf);
/*
* Ensure that the PrivateRefCountArray has sufficient space to store one more
@@ -1093,12 +1096,22 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
Assert(BUF_STATE_GET_REFCOUNT(buf_state) == 0);
+#if NO_DOUBLE_BUFFER_LOCK
+ if (buf_state & BM_TAG_VALID)
+ {
+ InvalidateBuffer(buf); /* releases spinlock */
+ continue;
+ }
+#endif
/* Must copy buffer flags while we still hold the spinlock */
oldFlags = buf_state & BUF_FLAG_MASK;
/* Pin the buffer and then release the buffer spinlock */
PinBuffer_Locked(buf);
+#if NO_DOUBLE_BUFFER_LOCK
+ Assert(!(oldFlags & (BM_DIRTY|BM_TAG_VALID)));
+#else
/*
* If the buffer was dirty, try to write it out. There is a race
* condition here, in that someone might dirty it after we released it
@@ -1216,6 +1229,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
}
}
else
+#endif
{
/* if it wasn't valid, we need only the new partition */
LWLockAcquire(newPartitionLock, LW_EXCLUSIVE);
