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