On Fri, Nov 01, 2024 at 05:45:44PM +1300, David Rowley wrote: > I think this should be passing BLCKSZ rather than (BLCKSZ / > sizeof(size_t)), otherwise, it'll just check the first 1 kilobyte is > zero rather than the entire page.
Ugh, Friday brain fart. The attached should be able to fix that, this brings back the movl to its correct path: - movl $1024, %esi + movl $8192, %esi Does that look fine to you? > I didn't test how performance-critical this is, but the header comment > for this function does use the words "cheaply detect". Under gcc -O2 or -O3, the single-byte check or the 8-byte check don't make a difference. Please see the attached (allzeros.txt) for a quick check if you want to check by yourself. With 1M iterations, both average around 3ms for 1M iterations on my laptop (not the fastest thing around). Under -O0, though, the difference is noticeable: - 1-byte check: 3.52s for 1M iterations, averaging one check at 3.52ns. - 8-byte check: 0.46s for 1M iterations, averaging one check at 0.46ns. Even for that, I doubt that this is going to be noticeable in practice, still the difference exists. -- Michael
#include <stdbool.h>
#include <stddef.h>
#include <string.h>
#define BLCKSZ 8192
#define LOOPS 1000000 /* 1M */
static inline bool
allzeros_char(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;
}
static inline bool
allzeros_size_t(const void *ptr, size_t len)
{
const size_t *p = (const size_t *) ptr;
for (size_t i = 0; i < len / sizeof(size_t); i++)
{
if (p[i] != 0)
return false;
}
return true;
}
int main()
{
size_t pagebytes[BLCKSZ] = {0};
for (int i = 0; i < LOOPS; i++)
{
allzeros_char(pagebytes, BLCKSZ);
//allzeros_size_t(pagebytes, BLCKSZ);
}
return 0;
}
diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.c
index 5ee1e58cd4..db5954def2 100644
--- a/src/backend/storage/page/bufpage.c
+++ b/src/backend/storage/page/bufpage.c
@@ -88,7 +88,6 @@ bool
PageIsVerifiedExtended(Page page, BlockNumber blkno, int flags)
{
PageHeader p = (PageHeader) page;
- size_t *pagebytes;
bool checksum_failure = false;
bool header_sane = false;
uint16 checksum = 0;
@@ -124,9 +123,7 @@ PageIsVerifiedExtended(Page page, BlockNumber blkno, int flags)
}
/* Check all-zeroes case */
- pagebytes = (size_t *) page;
-
- if (pg_memory_is_all_zeros(pagebytes, (BLCKSZ / sizeof(size_t))))
+ if (pg_memory_is_all_zeros(page, BLCKSZ))
return true;
/*
signature.asc
Description: PGP signature
