On 5/21/21 6:43 PM, Andres Freund wrote:
Hi,
> ...
>
Attached are the flame graphs for all three cases. The change in master is
pretty clearly visible, but I don't see any clear difference between old and
patched code :-(
I'm pretty sure it's the additional WAL records?
Not sure. If I understand what you suggested elsewhere in the thread, it
should be fine to modify heap_insert to pass the page recptr to
visibilitymap_set, roughly per the attached patch.
I'm not sure it's correct, but it does eliminate the Heap2/VISIBILITY
records for me (when applied on top of your patch). Funnily enough it
does make it a wee bit slower:
patch #1: 56941.505
patch #2: 58099.788
I wonder if this might be due to -fno-omit-frame-pointer, though, as
without it I get these timings:
0c7d3bb99: 25540.417
master: 31868.236
patch #1: 26566.199
patch #2: 26487.943
So without the frame pointers there's no slowdown, but there's no clear
improvement after removal of the WAL records either :-(
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 6ac07f2fda..2ec5158866 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2069,6 +2069,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
bool all_visible_cleared = false;
bool all_frozen_set = false;
uint8 vmstatus = 0;
+ XLogRecPtr recptr = InvalidXLogRecPtr;
/* Cheap, simplistic check that the tuple matches the rel's rowtype. */
Assert(HeapTupleHeaderGetNatts(tup->t_data) <=
@@ -2179,7 +2180,6 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
{
xl_heap_insert xlrec;
xl_heap_header xlhdr;
- XLogRecPtr recptr;
Page page = BufferGetPage(buffer);
uint8 info = XLOG_HEAP_INSERT;
int bufflags = 0;
@@ -2275,7 +2275,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
* visibility rules.
*/
visibilitymap_set(relation, BufferGetBlockNumber(buffer), buffer,
- InvalidXLogRecPtr, vmbuffer,
+ recptr, vmbuffer,
InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN);
}
diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
index e198df65d8..c00544525f 100644
--- a/src/backend/access/heap/visibilitymap.c
+++ b/src/backend/access/heap/visibilitymap.c
@@ -266,6 +266,10 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf,
if (!BufferIsValid(vmBuf) || BufferGetBlockNumber(vmBuf) != mapBlock)
elog(ERROR, "wrong VM buffer passed to visibilitymap_set");
+ /* bail out if we already got a valid LSN */
+ if (recptr != InvalidXLogRecPtr)
+ return;
+
page = BufferGetPage(vmBuf);
map = (uint8 *) PageGetContents(page);
LockBuffer(vmBuf, BUFFER_LOCK_EXCLUSIVE);
