From 8d2c1df4f4c54d0a73fcabaf5e25bc55ce7fb5fa Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Sun, 15 Mar 2026 10:42:34 -0400 Subject: [PATCH] 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 Discussion: https://postgr.es/m/CAAKRu_a%2BhO4PCptyaPR7AMZd7FjcHfOFKKJT8ouU3KedMud0tQ%40mail.gmail.com --- src/backend/access/heap/heapam.c | 17 ++++++++------ src/backend/access/heap/heapam_xlog.c | 34 ++++++++++++++------------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 8f1c11a9350..b6a9366000c 100644 --- 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 6d39a5fff7c..1da774c1536 100644 --- 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); }