Hi,
In dc6acfd910b8, I added some counters to track and log in
autovacuum/vacuum output the number of pages newly set
all-visible/frozen. Taking another look at the code recently, I
realized the conditions for setting the counters could be simplified
because of what we know to be true about the state of the heap page
and VM at the time we are doing the counting.
Further explanation is in the attached patch. This code is only in 18/master.
- Melanie
From 6cbbdd359ae4de835bbd77369b598885e8a279b2 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Wed, 18 Jun 2025 16:12:15 -0400
Subject: [PATCH v1 02/14] Simplify vacuum VM update logging counters
We can simplify the VM counters added in dc6acfd910b8 to
lazy_vacuum_heap_page() and lazy_scan_new_or_empty().
We won't invoke lazy_vacuum_heap_page() unless there are dead line
pointers, so we know the page can't be all-visible.
In lazy_scan_new_or_empty(), we only update the VM if the page-level
hint PD_ALL_VISIBLE is clear, and the VM bit cannot be set if the page
level bit is clear because a subsequent page update would fail to clear
the visibility map bit.
Simplify the logic for determining which log counters to increment based
on this knowledge.
---
src/backend/access/heap/vacuumlazy.c | 32 +++++++++++-----------------
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 09416450af9..c8da2f835c4 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1900,17 +1900,12 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
VISIBILITYMAP_ALL_FROZEN);
END_CRIT_SECTION();
- /*
- * If the page wasn't already set all-visible and/or all-frozen in
- * the VM, count it as newly set for logging.
- */
- if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0)
- {
- vacrel->vm_new_visible_pages++;
- vacrel->vm_new_visible_frozen_pages++;
- }
- else if ((old_vmbits & VISIBILITYMAP_ALL_FROZEN) == 0)
- vacrel->vm_new_frozen_pages++;
+ /* VM bits cannot have been set if PD_ALL_VISIBLE was clear */
+ Assert((old_vmbits & VISIBILITYMAP_VALID_BITS) == 0);
+ (void) old_vmbits; /* Silence compiler */
+ /* Count the newly all-frozen pages for logging. */
+ vacrel->vm_new_visible_pages++;
+ vacrel->vm_new_visible_frozen_pages++;
}
freespace = PageGetHeapFreeSpace(page);
@@ -2930,20 +2925,17 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
vmbuffer, visibility_cutoff_xid,
flags);
- /*
- * If the page wasn't already set all-visible and/or all-frozen in the
- * VM, count it as newly set for logging.
- */
- if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0)
+ /* We know the page should not have been all-visible */
+ Assert((old_vmbits & VISIBILITYMAP_VALID_BITS) == 0);
+ (void) old_vmbits; /* Silence compiler */
+
+ /* Count the newly set VM page for logging */
+ if ((flags & VISIBILITYMAP_ALL_VISIBLE) != 0)
{
vacrel->vm_new_visible_pages++;
if (all_frozen)
vacrel->vm_new_visible_frozen_pages++;
}
-
- else if ((old_vmbits & VISIBILITYMAP_ALL_FROZEN) == 0 &&
- all_frozen)
- vacrel->vm_new_frozen_pages++;
}
/* Revert to the previous phase information for error traceback */
--
2.34.1
