Avoid BufferGetPage() calls in heap_update()

BufferGetPage() isn't cheap and heap_update() calls it multiple times
when it could just save the page from a single call. Do that.
While we are at it, make separate variables for old and new page in
heap_xlog_update(). It's confusing to reuse "page" for both pages.

Author: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/CAAKRu_a%2BhO4PCptyaPR7AMZd7FjcHfOFKKJT8ouU3KedMud0tQ%40mail.gmail.com

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 8f1c11a93500de4bb545cb286d111d2280495857..b6a9366000c97da9dc51f922e4fed66c2689713f 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3328,7 +3328,8 @@ heap_update(Relation relation, const ItemPointerData *otid, HeapTuple newtup,
        HeapTuple       heaptup;
        HeapTuple       old_key_tuple = NULL;
        bool            old_key_copied = false;
-       Page            page;
+       Page            page,
+                               newpage;
        BlockNumber block;
        MultiXactStatus mxact_status;
        Buffer          buffer,
@@ -4054,6 +4055,8 @@ l2:
                heaptup = newtup;
        }
 
+       newpage = BufferGetPage(newbuf);
+
        /*
         * We're about to do the actual update -- check for conflict first, to
         * avoid possibly having to roll back work we've just done.
@@ -4168,17 +4171,17 @@ l2:
        oldtup.t_data->t_ctid = heaptup->t_self;
 
        /* clear PD_ALL_VISIBLE flags, reset all visibilitymap bits */
-       if (PageIsAllVisible(BufferGetPage(buffer)))
+       if (PageIsAllVisible(page))
        {
                all_visible_cleared = true;
-               PageClearAllVisible(BufferGetPage(buffer));
+               PageClearAllVisible(page);
                visibilitymap_clear(relation, BufferGetBlockNumber(buffer),
                                                        vmbuffer, VISIBILITYMAP_VALID_BITS);
        }
-       if (newbuf != buffer && PageIsAllVisible(BufferGetPage(newbuf)))
+       if (newbuf != buffer && PageIsAllVisible(newpage))
        {
                all_visible_cleared_new = true;
-               PageClearAllVisible(BufferGetPage(newbuf));
+               PageClearAllVisible(newpage);
                visibilitymap_clear(relation, BufferGetBlockNumber(newbuf),
                                                        vmbuffer_new, VISIBILITYMAP_VALID_BITS);
        }
@@ -4209,9 +4212,9 @@ l2:
                                                                 all_visible_cleared_new);
                if (newbuf != buffer)
                {
-                       PageSetLSN(BufferGetPage(newbuf), recptr);
+                       PageSetLSN(newpage, recptr);
                }
-               PageSetLSN(BufferGetPage(buffer), recptr);
+               PageSetLSN(page, recptr);
        }
 
        END_CRIT_SECTION();

diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c
index 6d39a5fff7cf603dcc2bd3924a5c27a2de5be868..1da774c153618ddd5be67a66098d7a71c744be7a 100644 (file)
--- a/src/backend/access/heap/heapam_xlog.c
+++ b/src/backend/access/heap/heapam_xlog.c
@@ -822,7 +822,8 @@ heap_xlog_update(XLogReaderState *record, bool hot_update)
        ItemPointerData newtid;
        Buffer          obuffer,
                                nbuffer;
-       Page            page;
+       Page            opage,
+                               npage;
        OffsetNumber offnum;
        ItemId          lp;
        HeapTupleData oldtup;
@@ -886,15 +887,15 @@ heap_xlog_update(XLogReaderState *record, bool hot_update)
                                                                          &obuffer);
        if (oldaction == BLK_NEEDS_REDO)
        {
-               page = BufferGetPage(obuffer);
+               opage = BufferGetPage(obuffer);
                offnum = xlrec->old_offnum;
-               if (offnum < 1 || offnum > PageGetMaxOffsetNumber(page))
+               if (offnum < 1 || offnum > PageGetMaxOffsetNumber(opage))
                        elog(PANIC, "offnum out of range");
-               lp = PageGetItemId(page, offnum);
+               lp = PageGetItemId(opage, offnum);
                if (!ItemIdIsNormal(lp))
                        elog(PANIC, "invalid lp");
 
-               htup = (HeapTupleHeader) PageGetItem(page, lp);
+               htup = (HeapTupleHeader) PageGetItem(opage, lp);
 
                oldtup.t_data = htup;
                oldtup.t_len = ItemIdGetLength(lp);
@@ -913,12 +914,12 @@ heap_xlog_update(XLogReaderState *record, bool hot_update)
                htup->t_ctid = newtid;
 
                /* Mark the page as a candidate for pruning */
-               PageSetPrunable(page, XLogRecGetXid(record));
+               PageSetPrunable(opage, XLogRecGetXid(record));
 
                if (xlrec->flags & XLH_UPDATE_OLD_ALL_VISIBLE_CLEARED)
-                       PageClearAllVisible(page);
+                       PageClearAllVisible(opage);
 
-               PageSetLSN(page, lsn);
+               PageSetLSN(opage, lsn);
                MarkBufferDirty(obuffer);
        }
 
@@ -933,8 +934,8 @@ heap_xlog_update(XLogReaderState *record, bool hot_update)
        else if (XLogRecGetInfo(record) & XLOG_HEAP_INIT_PAGE)
        {
                nbuffer = XLogInitBufferForRedo(record, 0);
-               page = BufferGetPage(nbuffer);
-               PageInit(page, BufferGetPageSize(nbuffer), 0);
+               npage = BufferGetPage(nbuffer);
+               PageInit(npage, BufferGetPageSize(nbuffer), 0);
                newaction = BLK_NEEDS_REDO;
        }
        else
@@ -966,10 +967,10 @@ heap_xlog_update(XLogReaderState *record, bool hot_update)
                recdata = XLogRecGetBlockData(record, 0, &datalen);
                recdata_end = recdata + datalen;
 
-               page = BufferGetPage(nbuffer);
+               npage = BufferGetPage(nbuffer);
 
                offnum = xlrec->new_offnum;
-               if (PageGetMaxOffsetNumber(page) + 1 < offnum)
+               if (PageGetMaxOffsetNumber(npage) + 1 < offnum)
                        elog(PANIC, "invalid max offset number");
 
                if (xlrec->flags & XLH_UPDATE_PREFIX_FROM_OLD)
@@ -1046,16 +1047,17 @@ heap_xlog_update(XLogReaderState *record, bool hot_update)
                /* Make sure there is no forward chain link in t_ctid */
                htup->t_ctid = newtid;
 
-               offnum = PageAddItem(page, htup, newlen, offnum, true, true);
+               offnum = PageAddItem(npage, htup, newlen, offnum, true, true);
                if (offnum == InvalidOffsetNumber)
                        elog(PANIC, "failed to add tuple");
 
                if (xlrec->flags & XLH_UPDATE_NEW_ALL_VISIBLE_CLEARED)
-                       PageClearAllVisible(page);
+                       PageClearAllVisible(npage);
 
-               freespace = PageGetHeapFreeSpace(page); /* needed to update FSM below */
+               /* needed to update FSM below */
+               freespace = PageGetHeapFreeSpace(npage);
 
-               PageSetLSN(page, lsn);
+               PageSetLSN(npage, lsn);
                MarkBufferDirty(nbuffer);
        }