Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On Thu, Aug 25, 2022 at 10:38:41AM +0200, Alvaro Herrera wrote: > It changes code. Any bugfix in the surrounding code would have to fix a > conflict. That is nonzero effort. Is it a huge risk? No, it is very > small risk and a very small cost to fix such a conflict; but my claim is > that this

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 2022-Aug-24, Peter Eisentraut wrote: > I don't follow how this is a backpatching hazard. It changes code. Any bugfix in the surrounding code would have to fix a conflict. That is nonzero effort. Is it a huge risk? No, it is very small risk and a very small cost to fix such a conflict; but

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qua., 24 de ago. de 2022 às 16:41, Robert Haas escreveu: > On Wed, Aug 24, 2022 at 3:20 PM Ranier Vilela wrote: > > But, these same arguments apply to Designated Initializers [1]. > > > > like: > > struct foo a = { > >.i = 0, > >.b = 0, > > }; > > > > That is slowly being introduced a

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On Wed, Aug 24, 2022 at 3:20 PM Ranier Vilela wrote: > But, these same arguments apply to Designated Initializers [1]. > > like: > struct foo a = { >.i = 0, >.b = 0, > }; > > That is slowly being introduced and IMHO brings the same problems with > padding bits. Yep. I don't find that an

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qua., 24 de ago. de 2022 às 16:00, Tom Lane escreveu: > Peter Eisentraut writes: > > On 24.08.22 16:30, Alvaro Herrera wrote: > >> If you do this, you're creating a potential backpatching hazard. This > >> is OK if we get something in return, so a question to ask is whether > >> there is any

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On Wed, Aug 24, 2022 at 3:00 PM Tom Lane wrote: > Call me a trogdolyte, but I don't follow how it's an improvement. > It looks to me like an entirely random change that doesn't get rid > of assumptions about what the bits are, it just replaces one set of > assumptions with a different set. Moreov

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Peter Eisentraut writes: > On 24.08.22 16:30, Alvaro Herrera wrote: >> If you do this, you're creating a potential backpatching hazard. This >> is OK if we get something in return, so a question to ask is whether >> there is any benefit in doing it. > I don't follow how this is a backpatching ha

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 24.08.22 16:30, Alvaro Herrera wrote: On 2022-Aug-19, David Zhang wrote: Should these obviously possible replacement of the standard library function "memset" be considered as well? For example, something like the attached one which is focusing on the pageinspect extension only. If you do t

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 2022-Aug-19, David Zhang wrote: > Should these obviously possible replacement of the standard library function > "memset" be considered as well? For example, something like the attached one > which is focusing on the pageinspect extension only. If you do this, you're creating a potential backp

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em sex., 19 de ago. de 2022 às 19:27, David Zhang escreveu: > Hi Ranier, > Hi David, > > Following the comment in commit 9fd45870c1436b477264c0c82eb195df52bc0919, > > (The same could be done with appropriate memset() calls, but this > patch is part of an effort to phase out MemSet(), so

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Hi Ranier, Following the comment in commit 9fd45870c1436b477264c0c82eb195df52bc0919,     (The same could be done with appropriate memset() calls, but this     patch is part of an effort to phase out MemSet(), so it doesn't touch     memset() calls.) Should these obviously possible replacement o

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qui., 11 de ago. de 2022 às 09:23, Julien Rouhaud escreveu: > On Thu, Aug 11, 2022 at 08:51:53AM -0300, Ranier Vilela wrote: > > Em qui., 11 de ago. de 2022 às 08:48, Alvaro Herrera < > > alvhe...@alvh.no-ip.org> escreveu: > > > > > On 2022-Aug-11, Ranier Vilela wrote: > > > > > > > According

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On Thu, Aug 11, 2022 at 08:51:53AM -0300, Ranier Vilela wrote: > Em qui., 11 de ago. de 2022 às 08:48, Alvaro Herrera < > alvhe...@alvh.no-ip.org> escreveu: > > > On 2022-Aug-11, Ranier Vilela wrote: > > > > > According to: > > > https://interrupt.memfault.com/blog/c-struct-padding-initialization

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qui., 11 de ago. de 2022 às 08:48, Alvaro Herrera < alvhe...@alvh.no-ip.org> escreveu: > On 2022-Aug-11, Ranier Vilela wrote: > > > According to: > > https://interrupt.memfault.com/blog/c-struct-padding-initialization > > Did you actually read it? > Yes, today. > > > https://interrupt.memfaul

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 2022-Aug-11, Ranier Vilela wrote: > According to: > https://interrupt.memfault.com/blog/c-struct-padding-initialization Did you actually read it? https://interrupt.memfault.com/blog/c-struct-padding-initialization#structure-zero-initialization : This looks great! However, it’s not obvious (f

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qui., 11 de ago. de 2022 às 07:38, Peter Eisentraut < peter.eisentr...@enterprisedb.com> escreveu: > On 01.08.22 19:08, Ranier Vilela wrote: > > Like how > > > https://github.com/postgres/postgres/commit/9fd45870c1436b477264c0c82eb195df52bc0919 > > < > https://github.com/postgres/postgres/commi

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 01.08.22 19:08, Ranier Vilela wrote: Like how https://github.com/postgres/postgres/commit/9fd45870c1436b477264c0c82eb195df52bc0919 New attempt to remove more MemSet calls, that are safe. Attached v3 patch

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em ter., 2 de ago. de 2022 às 10:17, mahendrakar s < mahendrakarfo...@gmail.com> escreveu: > Hi Ranier, > > I'm pretty late to thread but would like to know about your claim in the > thread: > `All compilers currently have memset optimized.` > What did I mean, modern compilers. I know one case of

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Hi Ranier, I'm pretty late to thread but would like to know about your claim in the thread: `All compilers currently have memset optimized.` I know one case of optimization where variable is not used after the memset. Are the cases for which the optimization is done consistent across all the compi

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em seg., 1 de ago. de 2022 às 22:19, Peter Smith escreveu: > On Tue, Aug 2, 2022 at 3:09 AM Ranier Vilela wrote: > > > > Em sáb., 16 de jul. de 2022 às 16:54, Ranier Vilela > escreveu: > >> > >> > >> > >> Em sáb, 16 de jul de 2022 2:58 AM, Peter Eisentraut < > peter.eisentr...@enterprisedb.com>

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On Tue, Aug 2, 2022 at 3:09 AM Ranier Vilela wrote: > > Em sáb., 16 de jul. de 2022 às 16:54, Ranier Vilela > escreveu: >> >> >> >> Em sáb, 16 de jul de 2022 2:58 AM, Peter Eisentraut >> escreveu: >>> >>> On 11.07.22 21:06, Ranier Vilela wrote: >>> > Em qui., 7 de jul. de 2022 às 14:01, Ranier

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em sáb., 16 de jul. de 2022 às 16:54, Ranier Vilela escreveu: > > > Em sáb, 16 de jul de 2022 2:58 AM, Peter Eisentraut < > peter.eisentr...@enterprisedb.com> escreveu: > >> On 11.07.22 21:06, Ranier Vilela wrote: >> > Em qui., 7 de jul. de 2022 às 14:01, Ranier Vilela > >

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em sáb, 16 de jul de 2022 2:58 AM, Peter Eisentraut < peter.eisentr...@enterprisedb.com> escreveu: > On 11.07.22 21:06, Ranier Vilela wrote: > > Em qui., 7 de jul. de 2022 às 14:01, Ranier Vilela > > escreveu: > > > > Attached the v1 of your patch. > > I think

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 11.07.22 21:06, Ranier Vilela wrote: Em qui., 7 de jul. de 2022 às 14:01, Ranier Vilela > escreveu: Attached the v1 of your patch. I think that all is safe to switch MemSet by {0}. Here the rebased patch v2, against latest head. I have committed my patch

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qui., 7 de jul. de 2022 às 14:01, Ranier Vilela escreveu: > Attached the v1 of your patch. > I think that all is safe to switch MemSet by {0}. > Here the rebased patch v2, against latest head. regards, Ranier Vilela v2-0001-WIP-Replace-MemSet-calls-with-struct-initialization.patch Descripti

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 07.07.22 13:16, Alvaro Herrera wrote: On 2022-Jul-07, Peter Eisentraut wrote: diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 4445a86aee..79b23fa7d7 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebacku

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qui., 7 de jul. de 2022 às 08:00, Peter Eisentraut < peter.eisentr...@enterprisedb.com> escreveu: >diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c >index 41b31c5c6f..803d169f57 100644 >--- a/src/backend/access/transam/twophase.c >+++ b/src/backend/acces

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qui., 7 de jul. de 2022 às 08:00, Peter Eisentraut < peter.eisentr...@enterprisedb.com> escreveu: > On 18.05.22 15:52, Peter Eisentraut wrote: > > On 18.05.22 01:18, Justin Pryzby wrote: > >> Take the first one as an example. It says: > >> > >> GenericCosts costs; > >> MemSet

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 01.07.22 17:58, Ranier Vilela wrote: Em qui., 30 de jun. de 2022 às 19:37, Peter Eisentraut > escreveu: I have also committed a patch that gets rid of MemSet() calls where the value is a constant not-0, because that just falls back to memset(

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 2022-Jul-07, Peter Eisentraut wrote: > diff --git a/src/bin/pg_basebackup/pg_basebackup.c > b/src/bin/pg_basebackup/pg_basebackup.c > index 4445a86aee..79b23fa7d7 100644 > --- a/src/bin/pg_basebackup/pg_basebackup.c > +++ b/src/bin/pg_basebackup/pg_basebackup.c > @@ -1952,7 +1948,6 @@ BaseBac

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 18.05.22 15:52, Peter Eisentraut wrote: On 18.05.22 01:18, Justin Pryzby wrote: Take the first one as an example.  It says: GenericCosts costs; MemSet(&costs, 0, sizeof(costs)); You sent a patch to change it to sizeof(GenericCosts). But it's not a pointer, so they are the

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qui., 30 de jun. de 2022 às 19:37, Peter Eisentraut < peter.eisentr...@enterprisedb.com> escreveu: > I have also committed a patch that gets rid of MemSet() calls where the > value is a constant not-0, because that just falls back to memset() anyway. > Peter there are some missing paths in this

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qui., 30 de jun. de 2022 às 19:37, Peter Eisentraut < peter.eisentr...@enterprisedb.com> escreveu: > On 19.05.22 18:09, Ranier Vilela wrote: > > Taking it a step further. > > Created a new patch into commitfest, targeting 16 version. > > https://commitfest.postgresql.org/38/3645/ > >

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 19.05.22 18:09, Ranier Vilela wrote: Taking it a step further. Created a new patch into commitfest, targeting 16 version. https://commitfest.postgresql.org/38/3645/ I have committed your 001 patch, which was clearly a (harmless) mistake. I have

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On Thu, 19 May 2022 at 11:20, Tom Lane wrote: > The thing that makes this a bit more difficult than it might be is > the special cases we have for known-aligned and so on targets, which > are particularly critical for palloc0 and makeNode etc. So there's > more than one case to look into. But I'

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qua., 18 de mai. de 2022 às 19:57, David Rowley escreveu: > On Thu, 19 May 2022 at 02:08, Ranier Vilela wrote: > > That would initialize the content at compilation and not at runtime, > correct? > > Your mental model of compilation and run-time might be flawed here. > Here's no such thing as

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qua., 18 de mai. de 2022 às 20:20, Tom Lane escreveu: > zeroing > relatively small, known-aligned node structs is THE use case. > Currently, especially on 64-bit Windows, MemSet can break alignment. regards, Ranier Vilela

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qua., 18 de mai. de 2022 às 19:57, David Rowley escreveu: > On Thu, 19 May 2022 at 02:08, Ranier Vilela wrote: > > That would initialize the content at compilation and not at runtime, > correct? > > Your mental model of compilation and run-time might be flawed here. > Here's no such thing as

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

David Rowley writes: > I've been wondering for a while if we really need to have the MemSet() > macro. I see it was added in 8cb415449 (1997). I think compilers have > evolved quite a bit in the past 25 years, so it could be time to > revisit that. Yeah, I've thought for awhile that technology h

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On Thu, 19 May 2022 at 02:08, Ranier Vilela wrote: > That would initialize the content at compilation and not at runtime, correct? Your mental model of compilation and run-time might be flawed here. Here's no such thing as zeroing memory at compile time. There's only emitting instructions that pe

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qua., 18 de mai. de 2022 às 10:52, Peter Eisentraut < peter.eisentr...@enterprisedb.com> escreveu: > On 18.05.22 01:18, Justin Pryzby wrote: > > Take the first one as an example. It says: > > > > GenericCosts costs; > > MemSet(&costs, 0, sizeof(costs)); > > > > You sent a pat

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On 18.05.22 01:18, Justin Pryzby wrote: Take the first one as an example. It says: GenericCosts costs; MemSet(&costs, 0, sizeof(costs)); You sent a patch to change it to sizeof(GenericCosts). But it's not a pointer, so they are the same. This instance can more easily be wr

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em qua., 18 de mai. de 2022 às 05:54, Alvaro Herrera < alvhe...@alvh.no-ip.org> escreveu: > This one caught my attention: > > diff --git a/contrib/pgcrypto/crypt-blowfish.c > b/contrib/pgcrypto/crypt-blowfish.c > index a663852ccf..63fcef562d 100644 > --- a/contrib/pgcrypto/crypt-blowfish.c > +++ b

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

This one caught my attention: diff --git a/contrib/pgcrypto/crypt-blowfish.c b/contrib/pgcrypto/crypt-blowfish.c index a663852ccf..63fcef562d 100644 --- a/contrib/pgcrypto/crypt-blowfish.c +++ b/contrib/pgcrypto/crypt-blowfish.c @@ -750,7 +750,7 @@ _crypt_blowfish_rn(const char *key, const char *

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em ter., 17 de mai. de 2022 às 20:18, Justin Pryzby escreveu: > On Tue, May 17, 2022 at 07:52:30PM -0300, Ranier Vilela wrote: > > I found, I believe, a serious problem of incorrect usage of the memset > api. > > Historically, people have relied on using memset or MemSet, using the > > variable n

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Ranier Vilela writes: > I found, I believe, a serious problem of incorrect usage of the memset api. > Historically, people have relied on using memset or MemSet, using the > variable name as an argument for the sizeof. > While it works correctly, for arrays, when it comes to pointers to > structur

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On Tue, May 17, 2022 at 07:52:30PM -0300, Ranier Vilela wrote: > I found, I believe, a serious problem of incorrect usage of the memset api. > Historically, people have relied on using memset or MemSet, using the > variable name as an argument for the sizeof. > While it works correctly, for arrays,

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em ter., 17 de mai. de 2022 às 10:33, Ranier Vilela escreveu: > Em seg., 16 de mai. de 2022 às 20:26, David Rowley > escreveu: > >> On Sun, 15 May 2022 at 09:47, Ranier Vilela wrote: >> > At function load_relcache_init_file, there is an unnecessary function >> call, >> > to initialize pgstat_in

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em seg., 16 de mai. de 2022 às 20:26, David Rowley escreveu: > On Sun, 15 May 2022 at 09:47, Ranier Vilela wrote: > > At function load_relcache_init_file, there is an unnecessary function > call, > > to initialize pgstat_info pointer to NULL. > > > > MemSet(&rel->pgstat_info, 0, sizeof(rel->pgst

Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

On Sun, 15 May 2022 at 09:47, Ranier Vilela wrote: > At function load_relcache_init_file, there is an unnecessary function call, > to initialize pgstat_info pointer to NULL. > > MemSet(&rel->pgstat_info, 0, sizeof(rel->pgstat_info)); What seems to have happened here is the field was changed to be