public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: David Rowley <[email protected]>
Cc: Bertrand Drouvot <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: [email protected]
Subject: Re: define pg_structiszero(addr, s, r)
Date: Fri, 1 Nov 2024 15:27:38 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAApHDvrXzPAr3FxoBuB7b3D-okNoNA2jxLun1rW8Yw5wkbqusw@mail.gmail.com>
References: <ZyCUsIu/[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAHut+Pu9Un427eoPuEetNhk2PZe1-sgSxEuG_2OuTwtxC3cBOw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAApHDvrXzPAr3FxoBuB7b3D-okNoNA2jxLun1rW8Yw5wkbqusw@mail.gmail.com>

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


Attachments:

  [text/x-csrc] allzeros.c (699B, ../[email protected]/2-allzeros.c)
  download | inline:
#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;
}

  [text/x-diff] page-allzeros.patch (729B, ../[email protected]/3-page-allzeros.patch)
  download | inline diff:
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;
 
 	/*


  [application/pgp-signature] signature.asc (833B, ../[email protected]/4-signature.asc)
  download

view thread (36+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: define pg_structiszero(addr, s, r)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox