public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v2] explain: show JIT details in non-text format, even if zero
10+ messages / 7 participants
[nested] [flat]
* [PATCH v2] explain: show JIT details in non-text format, even if zero
@ 2020-10-17 19:10 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Justin Pryzby @ 2020-10-17 19:10 UTC (permalink / raw)
---
src/backend/commands/explain.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 43f9b01e83..89caa76801 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -838,8 +838,9 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, JitInstrumentation *ji)
{
instr_time total_time;
- /* don't print information if no JITing happened */
- if (!ji || ji->created_functions == 0)
+ /* don't print information if JITing wasn't done at planning time */
+ if (!ji || (ji->created_functions == 0 &&
+ es->format == EXPLAIN_FORMAT_TEXT))
return;
/* calculate total time */
--
2.17.0
--9jxsPFA5p3P2qPhR--
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v1] explain: show JIT details in non-text format, even if zero
@ 2020-10-17 19:10 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Justin Pryzby @ 2020-10-17 19:10 UTC (permalink / raw)
---
src/backend/commands/explain.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 41317f1837..7345971507 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -839,7 +839,8 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, JitInstrumentation *ji)
instr_time total_time;
/* don't print information if no JITing happened */
- if (!ji || ji->created_functions == 0)
+ if (!ji || (ji->created_functions == 0 &&
+ es->format == EXPLAIN_FORMAT_TEXT))
return;
/* calculate total time */
--
2.17.0
--veXX9dWIonWZEC6h--
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: define pg_structiszero(addr, s, r)
@ 2024-09-18 08:03 Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Peter Eisentraut @ 2024-09-18 08:03 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; [email protected]
On 18.09.24 06:16, Bertrand Drouvot wrote:
> +#define pg_structiszero(addr, s, r) \
> + do { \
> + /* We assume this initializes to zeroes */ \
> + static const s all_zeroes; \
> + r = (memcmp(addr, &all_zeroes, sizeof(all_zeroes)) == 0); \
> + } while (0)
This assumption is kind of the problem, isn't it? Because, you can't
assume that. And the existing code is arguably kind of wrong. But
moreover, this macro also assumes that the "addr" argument has no random
padding bits.
In the existing code, you can maybe make a local analysis that the code
is working correctly, although I'm not actually sure. But if you are
repackaging this as a general macro under a general-sounding name, then
the requirements should be more stringent.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: define pg_structiszero(addr, s, r)
@ 2024-09-18 18:57 Bertrand Drouvot <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 2 replies; 10+ messages in thread
From: Bertrand Drouvot @ 2024-09-18 18:57 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: [email protected]
Hi,
On Wed, Sep 18, 2024 at 10:03:21AM +0200, Peter Eisentraut wrote:
> On 18.09.24 06:16, Bertrand Drouvot wrote:
> > +#define pg_structiszero(addr, s, r) \
> > + do { \
> > + /* We assume this initializes to zeroes */ \
> > + static const s all_zeroes; \
> > + r = (memcmp(addr, &all_zeroes, sizeof(all_zeroes)) == 0); \
> > + } while (0)
>
Thanks for the feedback.
> This assumption is kind of the problem, isn't it? Because, you can't assume
> that. And the existing code is arguably kind of wrong. But moreover, this
> macro also assumes that the "addr" argument has no random padding bits.
>
> In the existing code, you can maybe make a local analysis that the code is
> working correctly, although I'm not actually sure.
I think it is but will give it a closer look.
> But if you are
> repackaging this as a general macro under a general-sounding name, then the
> requirements should be more stringent.
Agree. That said in v2 ([1]), it has been renamed to pgstat_entry_all_zeros().
I think that I will:
1/ take a closer look regarding the existing assumption
2/ if 1/ outcome is fine, then add more detailed comments around
pgstat_entry_all_zeros() to make sure it's not used outside of the existing
context
Sounds good to you?
[1]: https://www.postgresql.org/message-id/ZuqHLCdZXtEsbyb/%40ip-10-97-1-34.eu-west-3.compute.internal
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: define pg_structiszero(addr, s, r)
@ 2024-10-28 14:32 Heikki Linnakangas <[email protected]>
parent: Bertrand Drouvot <[email protected]>
1 sibling, 2 replies; 10+ messages in thread
From: Heikki Linnakangas @ 2024-10-28 14:32 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; Peter Eisentraut <[email protected]>; +Cc: [email protected]
On 18/09/2024 21:57, Bertrand Drouvot wrote:
> On Wed, Sep 18, 2024 at 10:03:21AM +0200, Peter Eisentraut wrote:
>> On 18.09.24 06:16, Bertrand Drouvot wrote:
>>> +#define pg_structiszero(addr, s, r) \
>>> + do { \
>>> + /* We assume this initializes to zeroes */ \
>>> + static const s all_zeroes; \
>>> + r = (memcmp(addr, &all_zeroes, sizeof(all_zeroes)) == 0); \
>>> + } while (0)
Not new with this patch, but do we guarantee padding bytes to be zeros?
How about this instead:
static inline bool
pg_is_all_zeros(const char *p, size_t len)
{
for (size_t i = 0; i < len; i++)
{
if (p[i] != 0)
return false;
}
return true;
}
Is there's a de facto standard name for that function? I was surprised
that I couldn't find one with a quick google search. That seems like the
kind of small utility function that every C program needs.
How performance sensitive is this? If it's not, then the above seems
like the most straightforward way to do this, which is good. If it is
performance sensitive, it's still good, because the compiler can
optimize that well: https://godbolt.org/z/x9hPWjheq.
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: define pg_structiszero(addr, s, r)
@ 2024-10-28 15:02 Ranier Vilela <[email protected]>
parent: Heikki Linnakangas <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: Ranier Vilela @ 2024-10-28 15:02 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Bertrand Drouvot <[email protected]>; Peter Eisentraut <[email protected]>; [email protected]
Em seg., 28 de out. de 2024 às 11:33, Heikki Linnakangas <[email protected]>
escreveu:
> On 18/09/2024 21:57, Bertrand Drouvot wrote:
> > On Wed, Sep 18, 2024 at 10:03:21AM +0200, Peter Eisentraut wrote:
> >> On 18.09.24 06:16, Bertrand Drouvot wrote:
> >>> +#define pg_structiszero(addr, s, r)
> \
> >>> + do {
> \
> >>> + /* We assume this initializes to zeroes */
> \
> >>> + static const s all_zeroes;
> \
> >>> + r = (memcmp(addr, &all_zeroes, sizeof(all_zeroes)) == 0);
> \
> >>> + } while (0)
>
> Not new with this patch, but do we guarantee padding bytes to be zeros?
>
> How about this instead:
>
> static inline bool
> pg_is_all_zeros(const char *p, size_t len)
> {
> for (size_t i = 0; i < len; i++)
> {
> if (p[i] != 0)
> return false;
> }
> return true;
> }
>
> It seems to me that this way is more optimized.
static inline bool
is_all_zeros(const char *p, size_t len)
{
for (size_t i = len; i >= 0; i--)
{
if (p[i] != 0)
return false;
}
return true;
}
main:
sub rsp, 24
lea rdx, [rsp + 12]
lea rcx, [rsp + 16]
lea rdi, [rip + .L.str]
lea rsi, [rsp + 8]
xor eax, eax
call __isoc99_scanf@PLT
lea rdi, [rip + .L.str.1]
xor esi, esi
xor eax, eax
call printf@PLT
xor eax, eax
add rsp, 24
ret
best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: define pg_structiszero(addr, s, r)
@ 2024-10-28 15:08 Tom Lane <[email protected]>
parent: Ranier Vilela <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Tom Lane @ 2024-10-28 15:08 UTC (permalink / raw)
To: Ranier Vilela <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; Bertrand Drouvot <[email protected]>; Peter Eisentraut <[email protected]>; [email protected]
Ranier Vilela <[email protected]> writes:
> It seems to me that [reversing the loop direction] is more optimized.
That's far from clear: you're ignoring the possibility that memory
access logic is better optimized for forward scanning than reverse
scanning. I'd stick with the forward scan without some extremely
detailed testing.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: define pg_structiszero(addr, s, r)
@ 2024-10-28 16:29 Ranier Vilela <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Ranier Vilela @ 2024-10-28 16:29 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; Bertrand Drouvot <[email protected]>; Peter Eisentraut <[email protected]>; [email protected]
Em seg., 28 de out. de 2024 às 12:08, Tom Lane <[email protected]> escreveu:
> Ranier Vilela <[email protected]> writes:
> > It seems to me that [reversing the loop direction] is more optimized.
>
> That's far from clear: you're ignoring the possibility that memory
> access logic is better optimized for forward scanning than reverse
> scanning. I'd stick with the forward scan without some extremely
> detailed testing.
>
I don't disagree.
After posting, I was wondering if the first possible is not zero, it should
probably be at the beginning and not at the end.
best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: define pg_structiszero(addr, s, r)
@ 2024-10-28 22:36 Peter Smith <[email protected]>
parent: Bertrand Drouvot <[email protected]>
1 sibling, 0 replies; 10+ messages in thread
From: Peter Smith @ 2024-10-28 22:36 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; [email protected]
On Thu, Sep 19, 2024 at 4:57 AM Bertrand Drouvot
<[email protected]> wrote:
>
> Hi,
>
> On Wed, Sep 18, 2024 at 10:03:21AM +0200, Peter Eisentraut wrote:
> > On 18.09.24 06:16, Bertrand Drouvot wrote:
> > > +#define pg_structiszero(addr, s, r) \
> > > + do { \
> > > + /* We assume this initializes to zeroes */ \
> > > + static const s all_zeroes; \
> > > + r = (memcmp(addr, &all_zeroes, sizeof(all_zeroes)) == 0); \
> > > + } while (0)
> >
>
> Thanks for the feedback.
>
> > This assumption is kind of the problem, isn't it? Because, you can't assume
> > that. And the existing code is arguably kind of wrong. But moreover, this
> > macro also assumes that the "addr" argument has no random padding bits.
> >
> > In the existing code, you can maybe make a local analysis that the code is
> > working correctly, although I'm not actually sure.
>
> I think it is but will give it a closer look.
>
> > But if you are
> > repackaging this as a general macro under a general-sounding name, then the
> > requirements should be more stringent.
>
> Agree. That said in v2 ([1]), it has been renamed to pgstat_entry_all_zeros().
>
> I think that I will:
>
> 1/ take a closer look regarding the existing assumption
> 2/ if 1/ outcome is fine, then add more detailed comments around
> pgstat_entry_all_zeros() to make sure it's not used outside of the existing
> context
>
> Sounds good to you?
>
> [1]: https://www.postgresql.org/message-id/ZuqHLCdZXtEsbyb/%40ip-10-97-1-34.eu-west-3.compute.internal
>
> Regards,
>
> --
> Bertrand Drouvot
> PostgreSQL Contributors Team
> RDS Open Source Databases
> Amazon Web Services: https://aws.amazon.com
>
Hi, if this is performance critical then FWIW my understanding is that
memcmp can outperform simple loop checking, and my hacky testing
seemed to confirm that.
See https://godbolt.org/z/GWY1ob9bE
static inline bool
is_all_zeros2(const char *p, size_t len)
{
static size_t nz = 0;
static const char *z = NULL;
if (nz < len)
{
if (z) free((void *)z);
nz = len;
z = (const char *)calloc(1, nz);
}
return memcmp(p, z, len) == 0;
}
~~
Executor x86-64 gcc 14.2 (C, Editor #1)
Program stdout
Iterate 1000000 times...
check zeros using loop -- elapsed=0.041196s
check zeros using memcmp -- elapsed=0.016407s
======
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: define pg_structiszero(addr, s, r)
@ 2024-10-29 07:54 Bertrand Drouvot <[email protected]>
parent: Heikki Linnakangas <[email protected]>
1 sibling, 0 replies; 10+ messages in thread
From: Bertrand Drouvot @ 2024-10-29 07:54 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; [email protected]
Hi,
On Mon, Oct 28, 2024 at 04:32:51PM +0200, Heikki Linnakangas wrote:
> On 18/09/2024 21:57, Bertrand Drouvot wrote:
> > On Wed, Sep 18, 2024 at 10:03:21AM +0200, Peter Eisentraut wrote:
> > > On 18.09.24 06:16, Bertrand Drouvot wrote:
> > > > +#define pg_structiszero(addr, s, r) \
> > > > + do { \
> > > > + /* We assume this initializes to zeroes */ \
> > > > + static const s all_zeroes; \
> > > > + r = (memcmp(addr, &all_zeroes, sizeof(all_zeroes)) == 0); \
> > > > + } while (0)
>
> Not new with this patch
Thanks for looking at it!
> but do we guarantee padding bytes to be zeros?
I can see padding in one of the 3 structs of interest: PgStat_BgWriterStats and
PgStat_CheckpointerStats have no padding but PgStat_TableCounts has:
(gdb) ptype /o struct PgStat_TableCounts
/* offset | size */ type = struct PgStat_TableCounts {
/* 0 | 8 */ PgStat_Counter numscans;
/* 8 | 8 */ PgStat_Counter tuples_returned;
/* 16 | 8 */ PgStat_Counter tuples_fetched;
/* 24 | 8 */ PgStat_Counter tuples_inserted;
/* 32 | 8 */ PgStat_Counter tuples_updated;
/* 40 | 8 */ PgStat_Counter tuples_deleted;
/* 48 | 8 */ PgStat_Counter tuples_hot_updated;
/* 56 | 8 */ PgStat_Counter tuples_newpage_updated;
/* 64 | 1 */ _Bool truncdropped;
/* XXX 7-byte hole */
/* 72 | 8 */ PgStat_Counter delta_live_tuples;
/* 80 | 8 */ PgStat_Counter delta_dead_tuples;
/* 88 | 8 */ PgStat_Counter changed_tuples;
/* 96 | 8 */ PgStat_Counter blocks_fetched;
/* 104 | 8 */ PgStat_Counter blocks_hit;
/* total size (bytes): 112 */
}
According to my testing, I can see that "static const PgStat_TableCounts all_zeroes"
all_zeroes is fully made of zeros (padding included). OTOH I would not bet on the
fact that's guaranteed to be the case 100% of the time.
But even, if that is not the case I don't think that it is a big issue: the
check is in pgstat_relation_flush_cb() to decide if we want to avoid unnecessarily
modifying the stats entry. If padding would contain non zeros then we would
"just" unnecessarily modify the stats entry (adding "zeros" to the shared stats).
> How about this instead:
>
> static inline bool
> pg_is_all_zeros(const char *p, size_t len)
> {
> for (size_t i = 0; i < len; i++)
> {
> if (p[i] != 0)
> return false;
> }
> return true;
> }
>
Yeah, that sounds good to me. It's more generic than the initial proposal that
was taking care of a struct memory area. Also, the same "logic" is already
in PageIsVerifiedExtended().
V3 attached is using the above proposal and also makes the change in
PageIsVerifiedExtended(). Then, the new inline function has been placed in
utils/memutils.h (not sure that's the best place though).
> Is there's a de facto standard name for that function? I was surprised that
> I couldn't find one with a quick google search.
Same here. I was just able to find "memiszero" in [0]. So the naming proposal
in v3 is pg_mem_is_all_zeros().
> That seems like the kind of
> small utility function that every C program needs.
Yeah.
> How performance sensitive is this?
I don't think that's very sensitive. I think it's "cheap" as compared to what lead
to those code paths (stats increments).
> If it's not, then the above seems like
> the most straightforward way to do this, which is good. If it is performance
> sensitive, it's still good, because the compiler can optimize that well:
> https://godbolt.org/z/x9hPWjheq.
Yeah, I also think that's fine. Peter Smith did some testing in [1] comparing
memcmp and simple loop checking (thanks Peter for the testing!):
"
Iterate 1000000 times...
check zeros using loop -- elapsed=0.041196s
check zeros using memcmp -- elapsed=0.016407s
"
So, in this test, the loop is 0.024789s longer means 0.024789s/1000000=24 Nanosecond
slower per comparison (If my math is correct). I don't see this as a red flag and
still go with the loop proposal as this is the one already used in
PageIsVerifiedExtended().
[0]: https://in3.readthedocs.io/en/develop/api-c.html
[1]: https://www.postgresql.org/message-id/CAHut%2BPvHmWiPyqiDRnD7FYsc4QskXpKEZAH3Z8Ahd_GSnRXWrw%40mail.g...
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v3-0001-New-pg_mem_is_all_zeros-p-len-inline-function.patch (5.5K, ../../ZyCUsIu%[email protected]/2-v3-0001-New-pg_mem_is_all_zeros-p-len-inline-function.patch)
download | inline diff:
From aa1b45af21fd363bb4c7b202f7c3aa654048b208 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Tue, 10 Sep 2024 01:22:02 +0000
Subject: [PATCH v3] New pg_mem_is_all_zeros(p, len) inline function
This new function allows to test if a memory region (starting at p) and of
size len is full of zeroes.
---
src/backend/storage/page/bufpage.c | 13 +------------
src/backend/utils/activity/pgstat_bgwriter.c | 5 +++--
src/backend/utils/activity/pgstat_checkpointer.c | 7 +++----
src/backend/utils/activity/pgstat_relation.c | 7 ++-----
src/include/utils/memutils.h | 14 ++++++++++++++
5 files changed, 23 insertions(+), 23 deletions(-)
20.6% src/backend/storage/page/
61.1% src/backend/utils/activity/
18.1% src/include/utils/
diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.c
index be6f1f62d2..306090c778 100644
--- 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_mem_is_all_zeros((const char *) 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 364a7a2024..d9c21cf665 100644
--- 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_mem_is_all_zeros((const char *) &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 bbfc9c7e18..6f3e2a8a45 100644
--- 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_mem_is_all_zeros((const char *) &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 8a3f7d434c..3bbec950b1 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -801,7 +801,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;
@@ -816,11 +815,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_mem_is_all_zeros((const char *) &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 cd9596ff21..2f331514f3 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -189,4 +189,18 @@ 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 p and of size len is full of zeroes.
+ */
+static inline bool
+pg_mem_is_all_zeros(const char *p, size_t len)
+{
+ for (size_t i = 0; i < len; i++)
+ {
+ if (p[i] != 0)
+ return false;
+ }
+ return true;
+}
+
#endif /* MEMUTILS_H */
--
2.34.1
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2024-10-29 07:54 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-10-17 19:10 [PATCH v2] explain: show JIT details in non-text format, even if zero Justin Pryzby <[email protected]>
2020-10-17 19:10 [PATCH v1] explain: show JIT details in non-text format, even if zero Justin Pryzby <[email protected]>
2024-09-18 08:03 Re: define pg_structiszero(addr, s, r) Peter Eisentraut <[email protected]>
2024-09-18 18:57 ` Re: define pg_structiszero(addr, s, r) Bertrand Drouvot <[email protected]>
2024-10-28 14:32 ` Re: define pg_structiszero(addr, s, r) Heikki Linnakangas <[email protected]>
2024-10-28 15:02 ` Re: define pg_structiszero(addr, s, r) Ranier Vilela <[email protected]>
2024-10-28 15:08 ` Re: define pg_structiszero(addr, s, r) Tom Lane <[email protected]>
2024-10-28 16:29 ` Re: define pg_structiszero(addr, s, r) Ranier Vilela <[email protected]>
2024-10-29 07:54 ` Re: define pg_structiszero(addr, s, r) Bertrand Drouvot <[email protected]>
2024-10-28 22:36 ` Re: define pg_structiszero(addr, s, r) Peter Smith <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox