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;
 
 	/*

Attachment: signature.asc
Description: PGP signature

Reply via email to