bufmgr: Add Pin/UnpinLocalBuffer()

So far these were open-coded in quite a few places, without a good reason.

Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/20221029025420.eplyow6k7tgu6he3@awork3.anarazel.de

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 2362423b89df09fbeb6ff0f549ec0fdb7f618098..1c3dec487a12874ef57317b3dfa140065f4ade5c 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -636,20 +636,7 @@ ReadRecentBuffer(RelFileLocator rlocator, ForkNumber forkNum, BlockNumber blockN
                /* Is it still valid and holding the right tag? */
                if ((buf_state & BM_VALID) && BufferTagsEqual(&tag, &bufHdr->tag))
                {
-                       /*
-                        * Bump buffer's ref and usage counts. This is equivalent of
-                        * PinBuffer for a shared buffer.
-                        */
-                       if (LocalRefCount[b] == 0)
-                       {
-                               if (BUF_STATE_GET_USAGECOUNT(buf_state) < BM_MAX_USAGE_COUNT)
-                               {
-                                       buf_state += BUF_USAGECOUNT_ONE;
-                                       pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
-                               }
-                       }
-                       LocalRefCount[b]++;
-                       ResourceOwnerRememberBuffer(CurrentResourceOwner, recent_buffer);
+                       PinLocalBuffer(bufHdr, true);
 
                        pgBufferUsage.local_blks_hit++;
 
@@ -1688,8 +1675,7 @@ ReleaseAndReadBuffer(Buffer buffer,
                                BufTagMatchesRelFileLocator(&bufHdr->tag, &relation->rd_locator) &&
                                BufTagGetForkNum(&bufHdr->tag) == forkNum)
                                return buffer;
-                       ResourceOwnerForgetBuffer(CurrentResourceOwner, buffer);
-                       LocalRefCount[-buffer - 1]--;
+                       UnpinLocalBuffer(buffer);
                }
                else
                {
@@ -3982,15 +3968,9 @@ ReleaseBuffer(Buffer buffer)
                elog(ERROR, "bad buffer ID: %d", buffer);
 
        if (BufferIsLocal(buffer))
-       {
-               ResourceOwnerForgetBuffer(CurrentResourceOwner, buffer);
-
-               Assert(LocalRefCount[-buffer - 1] > 0);
-               LocalRefCount[-buffer - 1]--;
-               return;
-       }
-
-       UnpinBuffer(GetBufferDescriptor(buffer - 1));
+               UnpinLocalBuffer(buffer);
+       else
+               UnpinBuffer(GetBufferDescriptor(buffer - 1));
 }
 
 /*

diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index 6f9e7eda57cfab54cb44d232ccf7305f79d16fc9..940b80d165e25025a20a7e1de3201ccb3ad163b1 100644 (file)
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -137,27 +137,8 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum,
                fprintf(stderr, "LB ALLOC (%u,%d,%d) %d\n",
                                smgr->smgr_rlocator.locator.relNumber, forkNum, blockNum, -b - 1);
 #endif
-               buf_state = pg_atomic_read_u32(&bufHdr->state);
 
-               /* this part is equivalent to PinBuffer for a shared buffer */
-               if (LocalRefCount[b] == 0)
-               {
-                       if (BUF_STATE_GET_USAGECOUNT(buf_state) < BM_MAX_USAGE_COUNT)
-                       {
-                               buf_state += BUF_USAGECOUNT_ONE;
-                               pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
-                       }
-               }
-               LocalRefCount[b]++;
-               ResourceOwnerRememberBuffer(CurrentResourceOwner,
-                                                                       BufferDescriptorGetBuffer(bufHdr));
-               if (buf_state & BM_VALID)
-                       *foundPtr = true;
-               else
-               {
-                       /* Previous read attempt must have failed; try again */
-                       *foundPtr = false;
-               }
+               *foundPtr = PinLocalBuffer(bufHdr, true);
                return bufHdr;
        }
 
@@ -194,9 +175,7 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum,
                        else
                        {
                                /* Found a usable buffer */
-                               LocalRefCount[b]++;
-                               ResourceOwnerRememberBuffer(CurrentResourceOwner,
-                                                                                       BufferDescriptorGetBuffer(bufHdr));
+                               PinLocalBuffer(bufHdr, false);
                                break;
                        }
                }
@@ -484,6 +463,48 @@ InitLocalBuffers(void)
        NLocBuffer = nbufs;
 }
 
+/*
+ * XXX: We could have a slightly more efficient version of PinLocalBuffer()
+ * that does not support adjusting the usagecount - but so far it does not
+ * seem worth the trouble.
+ */
+bool
+PinLocalBuffer(BufferDesc *buf_hdr, bool adjust_usagecount)
+{
+       uint32          buf_state;
+       Buffer          buffer = BufferDescriptorGetBuffer(buf_hdr);
+       int                     bufid = -buffer - 1;
+
+       buf_state = pg_atomic_read_u32(&buf_hdr->state);
+
+       if (LocalRefCount[bufid] == 0)
+       {
+               if (adjust_usagecount &&
+                       BUF_STATE_GET_USAGECOUNT(buf_state) < BM_MAX_USAGE_COUNT)
+               {
+                       buf_state += BUF_USAGECOUNT_ONE;
+                       pg_atomic_unlocked_write_u32(&buf_hdr->state, buf_state);
+               }
+       }
+       LocalRefCount[bufid]++;
+       ResourceOwnerRememberBuffer(CurrentResourceOwner,
+                                                               BufferDescriptorGetBuffer(buf_hdr));
+
+       return buf_state & BM_VALID;
+}
+
+void
+UnpinLocalBuffer(Buffer buffer)
+{
+       int                     buffid = -buffer - 1;
+
+       Assert(BufferIsLocal(buffer));
+       Assert(LocalRefCount[buffid] > 0);
+
+       ResourceOwnerForgetBuffer(CurrentResourceOwner, buffer);
+       LocalRefCount[buffid]--;
+}
+
 /*
  * GUC check_hook for temp_buffers
  */

diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h
index 2afb9bb3099b8929831f26003af5b3e512107599..970d00906153151610dc60f4c7efaf7f55ed19e8 100644 (file)
--- a/src/include/storage/buf_internals.h
+++ b/src/include/storage/buf_internals.h
@@ -415,6 +415,8 @@ extern int  BufTableInsert(BufferTag *tagPtr, uint32 hashcode, int buf_id);
 extern void BufTableDelete(BufferTag *tagPtr, uint32 hashcode);
 
 /* localbuf.c */
+extern bool PinLocalBuffer(BufferDesc *buf_hdr, bool adjust_usagecount);
+extern void UnpinLocalBuffer(Buffer buffer);
 extern PrefetchBufferResult PrefetchLocalBuffer(SMgrRelation smgr,
                                                                                                ForkNumber forkNum,
                                                                                                BlockNumber blockNum);