From cb6b3b22f2a1b56ee9bdda8fd605ab2c956555b3 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 6 Mar 2026 16:46:01 -0500 Subject: [PATCH v37 14/15] 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. --- 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 abc6fe904fb..dbdf6521c42 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -3339,7 +3339,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, @@ -4065,6 +4066,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. @@ -4179,17 +4182,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); } @@ -4220,9 +4223,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 a83f6b03d69..1302bb13e18 100644 --- a/src/backend/access/heap/heapam_xlog.c +++ b/src/backend/access/heap/heapam_xlog.c @@ -685,7 +685,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; @@ -749,15 +750,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); @@ -776,12 +777,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); } @@ -796,8 +797,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 @@ -829,10 +830,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) @@ -909,16 +910,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); } -- 2.43.0