Add pg_memory_is_all_zeros() in memutils.h

This new function tests if a memory region starting at a given location
for a defined length is made only of zeroes.  This unifies in a single
path the all-zero checks that were happening in a couple of places of
the backend code:
- For pgstats entries of relation, checkpointer and bgwriter, where
some "all_zeroes" variables were previously used with memcpy().
- For all-zero buffer pages in PageIsVerifiedExtended().

This new function uses the same forward scan as the check for all-zero
buffer pages, applying it to the three pgstats paths mentioned above.

Author: Bertrand Drouvot
Reviewed-by: Peter Eisentraut, Heikki Linnakangas, Peter Smith
Discussion: https://postgr.es/m/[email protected]

diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.c
index be6f1f62d292f2b13c6bdc0aebaa90f80d7e4620..5ee1e58cd440abc034446714f7acf12fad3cdac9 100644 (file)
--- a/src/backend/storage/page/bufpage.c
+++ b/src/backend/storage/page/bufpage.c
@@ -89,10 +89,8 @@ PageIsVerifiedExtended(Page page, BlockNumber blkno, int flags)
 {
    PageHeader  p = (PageHeader) page;
    size_t     *pagebytes;
-   int         i;
    bool        checksum_failure = false;
    bool        header_sane = false;
-   bool        all_zeroes = false;
    uint16      checksum = 0;
 
    /*
@@ -126,18 +124,9 @@ PageIsVerifiedExtended(Page page, BlockNumber blkno, int flags)
    }
 
    /* Check all-zeroes case */
-   all_zeroes = true;
    pagebytes = (size_t *) page;
-   for (i = 0; i < (BLCKSZ / sizeof(size_t)); i++)
-   {
-       if (pagebytes[i] != 0)
-       {
-           all_zeroes = false;
-           break;
-       }
-   }
 
-   if (all_zeroes)
+   if (pg_memory_is_all_zeros(pagebytes, (BLCKSZ / sizeof(size_t))))
        return true;
 
    /*

diff --git a/src/backend/utils/activity/pgstat_bgwriter.c b/src/backend/utils/activity/pgstat_bgwriter.c
index 364a7a2024a3199abcae7e0db0d08e98dad682d3..85d53d82f26a6f2e5ed6493684dc2eb369bf88cd 100644 (file)
--- a/src/backend/utils/activity/pgstat_bgwriter.c
+++ b/src/backend/utils/activity/pgstat_bgwriter.c
@@ -17,6 +17,7 @@
 
 #include "postgres.h"
 
+#include "utils/memutils.h"
 #include "utils/pgstat_internal.h"
 
 
@@ -30,7 +31,6 @@ void
 pgstat_report_bgwriter(void)
 {
    PgStatShared_BgWriter *stats_shmem = &pgStatLocal.shmem->bgwriter;
-   static const PgStat_BgWriterStats all_zeroes;
 
    Assert(!pgStatLocal.shmem->is_shutdown);
    pgstat_assert_is_up();
@@ -39,7 +39,8 @@ pgstat_report_bgwriter(void)
     * This function can be called even if nothing at all has happened. In
     * this case, avoid unnecessarily modifying the stats entry.
     */
-   if (memcmp(&PendingBgWriterStats, &all_zeroes, sizeof(all_zeroes)) == 0)
+   if (pg_memory_is_all_zeros(&PendingBgWriterStats,
+                              sizeof(struct PgStat_BgWriterStats)))
        return;
 
    pgstat_begin_changecount_write(&stats_shmem->changecount);

diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index 5a3fb4a9e09add7459049fa198aaea35b22ce024..b2d8eb0d9c3cdb5669e94b6500a88a537f323cc7 100644 (file)
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -17,6 +17,7 @@
 
 #include "postgres.h"
 
+#include "utils/memutils.h"
 #include "utils/pgstat_internal.h"
 
 
@@ -29,8 +30,6 @@ PgStat_CheckpointerStats PendingCheckpointerStats = {0};
 void
 pgstat_report_checkpointer(void)
 {
-   /* We assume this initializes to zeroes */
-   static const PgStat_CheckpointerStats all_zeroes;
    PgStatShared_Checkpointer *stats_shmem = &pgStatLocal.shmem->checkpointer;
 
    Assert(!pgStatLocal.shmem->is_shutdown);
@@ -40,8 +39,8 @@ pgstat_report_checkpointer(void)
     * This function can be called even if nothing at all has happened. In
     * this case, avoid unnecessarily modifying the stats entry.
     */
-   if (memcmp(&PendingCheckpointerStats, &all_zeroes,
-              sizeof(all_zeroes)) == 0)
+   if (pg_memory_is_all_zeros(&PendingCheckpointerStats,
+                              sizeof(struct PgStat_CheckpointerStats)))
        return;
 
    pgstat_begin_changecount_write(&stats_shmem->changecount);

diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c
index 36d3adf731099905da06047079a8a5abbe89ccb8..faba8b64d2344e1742a77a36217d0c9a21695ea3 100644 (file)
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -800,7 +800,6 @@ pgstat_twophase_postabort(TransactionId xid, uint16 info,
 bool
 pgstat_relation_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
 {
-   static const PgStat_TableCounts all_zeroes;
    Oid         dboid;
    PgStat_TableStatus *lstats; /* pending stats entry  */
    PgStatShared_Relation *shtabstats;
@@ -815,11 +814,9 @@ pgstat_relation_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
     * Ignore entries that didn't accumulate any actual counts, such as
     * indexes that were opened by the planner but not used.
     */
-   if (memcmp(&lstats->counts, &all_zeroes,
-              sizeof(PgStat_TableCounts)) == 0)
-   {
+   if (pg_memory_is_all_zeros(&lstats->counts,
+                              sizeof(struct PgStat_TableCounts)))
        return true;
-   }
 
    if (!pgstat_lock_entry(entry_ref, nowait))
        return false;

diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index cd9596ff21930dce7c4eab7ede5ff77f0f78472e..3590c8bad9a45d40d0bf9ab16280d495faa3ddda 100644 (file)
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -189,4 +189,21 @@ extern MemoryContext BumpContextCreate(MemoryContext parent,
 #define SLAB_DEFAULT_BLOCK_SIZE        (8 * 1024)
 #define SLAB_LARGE_BLOCK_SIZE      (8 * 1024 * 1024)
 
+/*
+ * Test if a memory region starting at "ptr" and of size "len" is full of
+ * zeroes.
+ */
+static inline bool
+pg_memory_is_all_zeros(const void *ptr, size_t len)
+{
+   const char *p = (const char *) ptr;
+
+   for (size_t i = 0; i < len; i++)
+   {
+       if (p[i] != 0)
+           return false;
+   }
+   return true;
+}
+
 #endif                         /* MEMUTILS_H */