Re: define pg_structiszero(addr, s, r)

2024-11-17 Thread Michael Paquier
On Fri, Nov 15, 2024 at 02:43:32PM +, Bertrand Drouvot wrote: > Thinking about it, actually, "[sizeof(size_t) * 8, inf)" (note the ')' at the > end) > might be the proper notation from a mathematical point of view. v13 relied on a notation I've always used in maths, as infinity cannot be incl

Re: define pg_structiszero(addr, s, r)

2024-11-17 Thread Ranier Vilela
Em sex., 15 de nov. de 2024 às 11:43, Bertrand Drouvot < bertranddrouvot...@gmail.com> escreveu: > Hi, > > On Fri, Nov 15, 2024 at 09:54:33AM -0300, Ranier Vilela wrote: > > There is a tiny typo with V13. > > + /* "len" in the [sizeof(size_t) * 8, inf] range */ > > I think "[sizeof(size_t) * 8, in

Re: define pg_structiszero(addr, s, r)

2024-11-16 Thread Bertrand Drouvot
Hi, On Sat, Nov 16, 2024 at 11:42:54AM -0300, Ranier Vilela wrote: > > Em sex., 15 de nov. de 2024 às 11:43, Bertrand Drouvot < > > bertranddrouvot...@gmail.com> escreveu: > > > >> while that should be: > >> > >> " > >> static inline bool > >> pg_memory_is_all_zeros_simd(const void *p, const void

Re: define pg_structiszero(addr, s, r)

2024-11-16 Thread Ranier Vilela
Em sáb., 16 de nov. de 2024 às 11:40, Ranier Vilela escreveu: > > Em sex., 15 de nov. de 2024 às 11:43, Bertrand Drouvot < > bertranddrouvot...@gmail.com> escreveu: > >> Hi, >> >> On Fri, Nov 15, 2024 at 09:54:33AM -0300, Ranier Vilela wrote: >> > There is a tiny typo with V13. >> > + /* "len" in

Re: define pg_structiszero(addr, s, r)

2024-11-15 Thread Bertrand Drouvot
Hi, On Fri, Nov 15, 2024 at 09:54:33AM -0300, Ranier Vilela wrote: > There is a tiny typo with V13. > + /* "len" in the [sizeof(size_t) * 8, inf] range */ I think "[sizeof(size_t) * 8, inf[ range" is correct. Infinity can not be included into a interval. Thinking about it, actually, "[sizeof(si

Re: define pg_structiszero(addr, s, r)

2024-11-15 Thread Ranier Vilela
Em sex., 15 de nov. de 2024 às 03:46, Bertrand Drouvot < bertranddrouvot...@gmail.com> escreveu: > Hi, > > On Fri, Nov 15, 2024 at 09:30:25AM +0900, Michael Paquier wrote: > > On Thu, Nov 14, 2024 at 12:33:20PM +, Bertrand Drouvot wrote: > > Anyway, as you say, the > > portability of v12 is OK

Re: define pg_structiszero(addr, s, r)

2024-11-14 Thread Michael Paquier
On Fri, Nov 15, 2024 at 06:46:52AM +, Bertrand Drouvot wrote: > Makes sense even if that looks "more difficult" to read. I don't think it's that "bad". > which is adding a "." at the end of single line comments (as the few already > part of this file don't do so). I did so after looking at t

Re: define pg_structiszero(addr, s, r)

2024-11-14 Thread Bertrand Drouvot
Hi, On Fri, Nov 15, 2024 at 09:30:25AM +0900, Michael Paquier wrote: > On Thu, Nov 14, 2024 at 12:33:20PM +, Bertrand Drouvot wrote: > Anyway, as you say, the > portability of v12 is OK even for sizeof(size_t) == 4 because we don't > rely on any hardcoded values, and this patch does what it sh

Re: define pg_structiszero(addr, s, r)

2024-11-14 Thread Michael Paquier
On Thu, Nov 14, 2024 at 12:33:20PM +, Bertrand Drouvot wrote: > Case 2 should be read as "in the 4-31" bytes range on 32-bit system as all > comparisons are done in size_t. I'd suggest to use a -m32 in your gcc switches, when it comes to tests, but you already know that.. Anyway, as you say,

Re: define pg_structiszero(addr, s, r)

2024-11-14 Thread Bertrand Drouvot
Hi, On Thu, Nov 14, 2024 at 09:13:19AM -0300, Ranier Vilela wrote: > Em qui., 14 de nov. de 2024 às 08:58, Bertrand Drouvot < > Maybe I'm doing something wrong. > But I'm testing in 32-bit, with the size set to 63, with v12 and I'm seeing > the SIMD loop execute. Yeah, that's expected and safe as

Re: define pg_structiszero(addr, s, r)

2024-11-14 Thread Ranier Vilela
Em qui., 14 de nov. de 2024 às 08:58, Bertrand Drouvot < bertranddrouvot...@gmail.com> escreveu: > Hi, > > On Thu, Nov 14, 2024 at 08:22:23AM -0300, Ranier Vilela wrote: > > Em qui., 14 de nov. de 2024 ąs 07:09, Bertrand Drouvot < > > bertranddrouvot...@gmail.com> escreveu: > > > > > Hi, > > > > >

Re: define pg_structiszero(addr, s, r)

2024-11-14 Thread Bertrand Drouvot
Hi, On Thu, Nov 14, 2024 at 08:22:23AM -0300, Ranier Vilela wrote: > Em qui., 14 de nov. de 2024 às 07:09, Bertrand Drouvot < > bertranddrouvot...@gmail.com> escreveu: > > > Hi, > > > > On Thu, Nov 14, 2024 at 09:27:06AM +0900, Michael Paquier wrote: > > > Makes sense to me to just do that, with

Re: define pg_structiszero(addr, s, r)

2024-11-14 Thread Ranier Vilela
Em qui., 14 de nov. de 2024 às 07:09, Bertrand Drouvot < bertranddrouvot...@gmail.com> escreveu: > Hi, > > On Thu, Nov 14, 2024 at 09:27:06AM +0900, Michael Paquier wrote: > > Makes sense to me to just do that, with a first < 8 loop, and a second > > for the 8~63 range. > > Thanks for looking at i

Re: define pg_structiszero(addr, s, r)

2024-11-14 Thread Bertrand Drouvot
Hi, On Thu, Nov 14, 2024 at 09:27:06AM +0900, Michael Paquier wrote: > Makes sense to me to just do that, with a first < 8 loop, and a second > for the 8~63 range. Thanks for looking at it! > There is also a "cant'" in the last size_t check. Simple typo. Please find attached v12, with more c

Re: define pg_structiszero(addr, s, r)

2024-11-13 Thread Michael Paquier
On Wed, Nov 13, 2024 at 07:50:50AM +, Bertrand Drouvot wrote: > I did a quick check with clang and it looks like it is not as smart as gcc > for the non inline case. Not as much, still smart enough to skip the > 64B part when dealing with a structure that does not require it. So it's actually

Re: define pg_structiszero(addr, s, r)

2024-11-13 Thread Ranier Vilela
Em qua., 13 de nov. de 2024 às 04:50, Bertrand Drouvot < bertranddrouvot...@gmail.com> escreveu: > Hi, > > On Wed, Nov 13, 2024 at 09:25:37AM +0900, Michael Paquier wrote: > > So that seems worth the addition, especially for > > smaller sizes where this is 6 times faster here. > > So, something li

Re: define pg_structiszero(addr, s, r)

2024-11-12 Thread Bertrand Drouvot
Hi, On Wed, Nov 13, 2024 at 09:25:37AM +0900, Michael Paquier wrote: > So that seems worth the addition, especially for > smaller sizes where this is 6 times faster here. So, something like v12 in pg_memory_is_all_zeros_v12() in allzeros_small.c attached? If so, that gives us: == with BLCKSZ 32

Re: define pg_structiszero(addr, s, r)

2024-11-12 Thread Bertrand Drouvot
Hi, On Tue, Nov 12, 2024 at 01:32:36PM -0300, Ranier Vilela wrote: > It seems to me that it is enough to protect the SIMD loop when the size is > smaller. > > if (len > sizeof(size_t) * 8) > { > for (; p < aligned_end - (sizeof(size_t) * 7); p += sizeof(size_t) * > 8) > { >

Re: define pg_structiszero(addr, s, r)

2024-11-12 Thread Ranier Vilela
Em ter., 12 de nov. de 2024 às 21:33, Michael Paquier escreveu: > On Tue, Nov 12, 2024 at 01:32:36PM -0300, Ranier Vilela wrote: > > See v1_allzeros_small.c attached. > > In your pg_memory_is_all_zeros_v11: > while (((uintptr_t) p & (sizeof(size_t) - 1)) != 0) > { > if (p == end)

Re: define pg_structiszero(addr, s, r)

2024-11-12 Thread Michael Paquier
On Tue, Nov 12, 2024 at 01:32:36PM -0300, Ranier Vilela wrote: > See v1_allzeros_small.c attached. In your pg_memory_is_all_zeros_v11: while (((uintptr_t) p & (sizeof(size_t) - 1)) != 0) { if (p == end) return true; if (*p++ != 0) return false;

Re: define pg_structiszero(addr, s, r)

2024-11-12 Thread Michael Paquier
On Tue, Nov 12, 2024 at 10:56:20AM +, Bertrand Drouvot wrote: > I think that depends of the memory area size. If the size is small enough > then the > byte per byte can be good enough. > > For example, with the allzeros_small.c attached: > > == with BLCKSZ 32 > > $ /usr/local/gcc-14.1.0/bin

Re: define pg_structiszero(addr, s, r)

2024-11-12 Thread Ranier Vilela
Em ter., 12 de nov. de 2024 às 07:56, Bertrand Drouvot < bertranddrouvot...@gmail.com> escreveu: > Hi, > > On Tue, Nov 12, 2024 at 03:56:13PM +0900, Michael Paquier wrote: > > On Tue, Nov 12, 2024 at 06:09:04AM +, Bertrand Drouvot wrote: > > > I think that the 64b len check done in v11 is mand

Re: define pg_structiszero(addr, s, r)

2024-11-12 Thread Bertrand Drouvot
Hi, On Tue, Nov 12, 2024 at 03:56:13PM +0900, Michael Paquier wrote: > On Tue, Nov 12, 2024 at 06:09:04AM +, Bertrand Drouvot wrote: > > I think that the 64b len check done in v11 is mandatory for safety reasons. > > > > The loop above reads 64 bytes at once, so would read beyond the memory a

Re: define pg_structiszero(addr, s, r)

2024-11-11 Thread Michael Paquier
On Tue, Nov 12, 2024 at 06:09:04AM +, Bertrand Drouvot wrote: > I think that the 64b len check done in v11 is mandatory for safety reasons. > > The loop above reads 64 bytes at once, so would read beyond the memory area > bounds > if len < 64: That could cause crash or read invalid data. Sor

Re: define pg_structiszero(addr, s, r)

2024-11-11 Thread Bertrand Drouvot
Hi, On Tue, Nov 12, 2024 at 12:28:53PM +0900, Michael Paquier wrote: > On Mon, Nov 11, 2024 at 05:07:51PM +, Bertrand Drouvot wrote: > > To handle the "what about the len check if the function is not inlined?", I > > can't think about a good approach. > > FWIW, my choice would be to not over-

Re: define pg_structiszero(addr, s, r)

2024-11-11 Thread Michael Paquier
On Mon, Nov 11, 2024 at 05:07:51PM +, Bertrand Drouvot wrote: > To handle the "what about the len check if the function is not inlined?", I > can't think about a good approach. FWIW, my choice would be to not over-engineer things more than what's in v10 posted at [1], hence do something withou

Re: define pg_structiszero(addr, s, r)

2024-11-11 Thread Bertrand Drouvot
Hi, On Mon, Nov 11, 2024 at 06:19:50AM +, Bertrand Drouvot wrote: > Hi, > > On Sat, Nov 09, 2024 at 04:15:04AM +, Bertrand Drouvot wrote: > > Hi, > > > > On Fri, Nov 08, 2024 at 11:18:09PM +1300, David Rowley wrote: > > > I tried with [1] and the > > > latest gcc does not seem to be smar

Re: define pg_structiszero(addr, s, r)

2024-11-10 Thread Bertrand Drouvot
Hi, On Sat, Nov 09, 2024 at 04:15:04AM +, Bertrand Drouvot wrote: > Hi, > > On Fri, Nov 08, 2024 at 11:18:09PM +1300, David Rowley wrote: > > I tried with [1] and the > > latest gcc does not seem to be smart enough to figure this out. I > > tried adding some additional len checks that the co

Re: define pg_structiszero(addr, s, r)

2024-11-08 Thread Bertrand Drouvot
Hi, On Fri, Nov 08, 2024 at 11:18:09PM +1300, David Rowley wrote: > I tried with [1] and the > latest gcc does not seem to be smart enough to figure this out. I > tried adding some additional len checks that the compiler can use as a > cue and won't need to emit code for the checks providing the

Re: define pg_structiszero(addr, s, r)

2024-11-08 Thread Bertrand Drouvot
Hi, On Sat, Nov 09, 2024 at 08:00:35AM +0900, Michael Paquier wrote: > On Fri, Nov 08, 2024 at 11:18:09PM +1300, David Rowley wrote: > > I'm slightly worried due to the current rate we're receiving cleanup > > suggestions that someone might come along and think they'd be doing us > > a favour by s

Re: define pg_structiszero(addr, s, r)

2024-11-08 Thread Michael Paquier
On Fri, Nov 08, 2024 at 11:18:09PM +1300, David Rowley wrote: > I'm slightly worried due to the current rate we're receiving cleanup > suggestions that someone might come along and think they'd be doing us > a favour by submitting a patch to "fixup the inefficient bitwise-ORs > and use boolean-OR".

Re: define pg_structiszero(addr, s, r)

2024-11-08 Thread Bertrand Drouvot
Hi, On Fri, Nov 08, 2024 at 11:18:09PM +1300, David Rowley wrote: > On Fri, 8 Nov 2024 at 18:34, Michael Paquier wrote: > > I've done a round of comment and term cleanup for the whole patch, > > while on it. > > I don't think "intrinsics" is the correct word to use here: > > + * - 8 * sizeof(si

Re: define pg_structiszero(addr, s, r)

2024-11-08 Thread David Rowley
On Fri, 8 Nov 2024 at 18:34, Michael Paquier wrote: > I've done a round of comment and term cleanup for the whole patch, > while on it. I don't think "intrinsics" is the correct word to use here: + * - 8 * sizeof(size_t) comparisons using bitwise OR, to encourage compilers + * to use SIMD intr

Re: define pg_structiszero(addr, s, r)

2024-11-07 Thread Michael Paquier
On Thu, Nov 07, 2024 at 08:41:34AM +, Bertrand Drouvot wrote: > Yeah, fully agree. My initial testing was not "good" enough and so was not > showing > as much improvement as your's and David's ones. > > Please find v8 attached. I've tested that with a couple of structures in an independent m

Re: define pg_structiszero(addr, s, r)

2024-11-07 Thread Bertrand Drouvot
Hi, On Thu, Nov 07, 2024 at 09:45:44AM +0900, Michael Paquier wrote: > On Thu, Nov 07, 2024 at 08:05:10AM +1300, David Rowley wrote: > > That might be quite good for small lengths or for use cases where the > > memory is always or almost always zero. The problem is there's no > > early exit when y

Re: define pg_structiszero(addr, s, r)

2024-11-07 Thread Bertrand Drouvot
Hi, On Thu, Nov 07, 2024 at 09:44:32AM +0900, Michael Paquier wrote: > On Thu, Nov 07, 2024 at 08:10:17AM +1300, David Rowley wrote: > > Did you try with a size where there's a decent remainder, say 124 > > bytes? FWIW, one of the cases has 112 bytes, and I think that is > > aligned memory meaning

Re: define pg_structiszero(addr, s, r)

2024-11-06 Thread Michael Paquier
On Thu, Nov 07, 2024 at 08:05:10AM +1300, David Rowley wrote: > That might be quite good for small lengths or for use cases where the > memory is always or almost always zero. The problem is there's no > early exit when you find the first non-zero which means, for larger > lengths, reading much mor

Re: define pg_structiszero(addr, s, r)

2024-11-06 Thread Michael Paquier
On Thu, Nov 07, 2024 at 08:10:17AM +1300, David Rowley wrote: > Did you try with a size where there's a decent remainder, say 124 > bytes? FWIW, one of the cases has 112 bytes, and I think that is > aligned memory meaning we'll do the first 64 in the SIMD loop and have > to do 48 bytes in the byte-

Re: define pg_structiszero(addr, s, r)

2024-11-06 Thread David Rowley
On Thu, 7 Nov 2024 at 00:40, Bertrand Drouvot wrote: > Do you mean add: > > " > for (; p < aligned_end; p += sizeof(size_t)) > { >if (*(size_t *)p != 0) >return false; > } > " > > just before the last loop? > > If so, I did a few tests and did not see any major improvements. So, I thou

Re: define pg_structiszero(addr, s, r)

2024-11-06 Thread David Rowley
On Thu, 7 Nov 2024 at 07:34, Peter Eisentraut wrote: > Speaking of which, couldn't you just use > > pg_popcount(ptr, len) == 0 That might be quite good for small lengths or for use cases where the memory is always or almost always zero. The problem is there's no early exit when you find the

Re: define pg_structiszero(addr, s, r)

2024-11-06 Thread Peter Eisentraut
On 05.11.24 16:03, Bertrand Drouvot wrote: On Tue, Nov 05, 2024 at 05:08:41PM +1300, David Rowley wrote: On Tue, 5 Nov 2024 at 06:39, Ranier Vilela wrote: I think we can add a small optimization to this last patch [1]. I think if you want to make it faster, you could partially unroll the inn

Re: define pg_structiszero(addr, s, r)

2024-11-06 Thread Bertrand Drouvot
Hi, On Wed, Nov 06, 2024 at 12:16:33PM +1300, David Rowley wrote: > On Wed, 6 Nov 2024 at 04:03, Bertrand Drouvot > wrote: > > Another option could be to use SIMD instructions to check multiple bytes > > is zero in a single operation. Maybe just an idea to keep in mind and > > experiment > > if

Re: define pg_structiszero(addr, s, r)

2024-11-06 Thread Bertrand Drouvot
Hi, On Wed, Nov 06, 2024 at 01:44:58PM +0900, Michael Paquier wrote: > Should the last loop check only 1 byte at a time or should this stuff > include one more step before the last one you wrote to do a couple of > checks with size_t? That may matter for areas small enough (len < > sizeof(size_t)

Re: define pg_structiszero(addr, s, r)

2024-11-05 Thread Michael Paquier
On Wed, Nov 06, 2024 at 03:53:48PM +1300, David Rowley wrote: > I'm not sure if I'm clear on what works for you. The latest patch I > saw did 1 size_t per iteration. Are you saying we should do just > size_t per loop? or we should form the code in a way that allows the > compiler to use SIMD instr

Re: define pg_structiszero(addr, s, r)

2024-11-05 Thread David Rowley
On Wed, 6 Nov 2024 at 13:52, Michael Paquier wrote: > > On Wed, Nov 06, 2024 at 01:38:50PM +1300, David Rowley wrote: > > We could just write it that way and leave it up to the compiler to > > decide whether to use SIMD or not. Going by [1], gcc with -O2 uses > > SIMD instructions from 14.1 and cl

Re: define pg_structiszero(addr, s, r)

2024-11-05 Thread Michael Paquier
On Wed, Nov 06, 2024 at 01:38:50PM +1300, David Rowley wrote: > We could just write it that way and leave it up to the compiler to > decide whether to use SIMD or not. Going by [1], gcc with -O2 uses > SIMD instructions from 14.1 and clang with -O2 does it from version > 8.0.0. gcc 14.1 was release

Re: define pg_structiszero(addr, s, r)

2024-11-05 Thread David Rowley
On Wed, 6 Nov 2024 at 13:20, Michael Paquier wrote: > > On Wed, Nov 06, 2024 at 12:16:33PM +1300, David Rowley wrote: > > On Wed, 6 Nov 2024 at 04:03, Bertrand Drouvot > > wrote: > >> Another option could be to use SIMD instructions to check multiple bytes > >> is zero in a single operation. Mayb

Re: define pg_structiszero(addr, s, r)

2024-11-05 Thread Michael Paquier
On Wed, Nov 06, 2024 at 12:16:33PM +1300, David Rowley wrote: > On Wed, 6 Nov 2024 at 04:03, Bertrand Drouvot > wrote: >> Another option could be to use SIMD instructions to check multiple bytes >> is zero in a single operation. Maybe just an idea to keep in mind and >> experiment >> if we feel t

Re: define pg_structiszero(addr, s, r)

2024-11-05 Thread David Rowley
On Wed, 6 Nov 2024 at 04:03, Bertrand Drouvot wrote: > Another option could be to use SIMD instructions to check multiple bytes > is zero in a single operation. Maybe just an idea to keep in mind and > experiment > if we feel the need later on. Could do. I just wrote it that way to give the comp

Re: define pg_structiszero(addr, s, r)

2024-11-05 Thread Bertrand Drouvot
Hi, On Tue, Nov 05, 2024 at 05:08:41PM +1300, David Rowley wrote: > On Tue, 5 Nov 2024 at 06:39, Ranier Vilela wrote: > > I think we can add a small optimization to this last patch [1]. > > I think if you want to make it faster, you could partially unroll the > inner-most loop, like: > > // siz

Re: define pg_structiszero(addr, s, r)

2024-11-05 Thread Ranier Vilela
Em ter., 5 de nov. de 2024 às 01:12, Michael Paquier escreveu: > On Tue, Nov 05, 2024 at 04:23:34PM +1300, David Rowley wrote: > > I tried your optimisation in the attached allzeros.c and here are my > results: > > > > # My version > > $ gcc allzeros.c -O2 -o allzeros && for i in {1..3}; do ./all

Re: define pg_structiszero(addr, s, r)

2024-11-05 Thread Ranier Vilela
Em ter., 5 de nov. de 2024 às 00:23, David Rowley escreveu: > On Tue, 5 Nov 2024 at 06:39, Ranier Vilela wrote: > > I think we can add a small optimization to this last patch [1]. > > The variable *aligned_end* is only needed in the second loop (for). > > So, only before the for loop do we actua

Re: define pg_structiszero(addr, s, r)

2024-11-04 Thread Bertrand Drouvot
Hi, On Tue, Nov 05, 2024 at 01:31:58PM +0900, Michael Paquier wrote: > On Mon, Nov 04, 2024 at 05:17:54PM +, Bertrand Drouvot wrote: > > Hi, > > > > On Tue, Nov 05, 2024 at 12:24:48AM +1300, David Rowley wrote: > > > On Sat, 2 Nov 2024 at 01:50, Bertrand Drouvot > > > wrote: > > > > > > > >

Re: define pg_structiszero(addr, s, r)

2024-11-04 Thread Bertrand Drouvot
Hi, On Tue, Nov 05, 2024 at 05:08:41PM +1300, David Rowley wrote: > I was happy enough with my patch with Bertrand's comments. I'm not > sure why unsigned chars are better than chars. It doesn't seem to have > any effect on the compiled code. > I think that unsigned chars is better than char fo

Re: define pg_structiszero(addr, s, r)

2024-11-04 Thread Michael Paquier
On Mon, Nov 04, 2024 at 05:17:54PM +, Bertrand Drouvot wrote: > Hi, > > On Tue, Nov 05, 2024 at 12:24:48AM +1300, David Rowley wrote: > > On Sat, 2 Nov 2024 at 01:50, Bertrand Drouvot > > wrote: > > > > > > On Fri, Nov 01, 2024 at 09:47:05PM +1300, David Rowley wrote: > > > > I've attached wh

Re: define pg_structiszero(addr, s, r)

2024-11-04 Thread Michael Paquier
On Tue, Nov 05, 2024 at 04:23:34PM +1300, David Rowley wrote: > I tried your optimisation in the attached allzeros.c and here are my results: > > # My version > $ gcc allzeros.c -O2 -o allzeros && for i in {1..3}; do ./allzeros; done > char: done in 1543600 nanoseconds > size_t: done in 196300 nan

Re: define pg_structiszero(addr, s, r)

2024-11-04 Thread David Rowley
On Tue, 5 Nov 2024 at 06:39, Ranier Vilela wrote: > I think we can add a small optimization to this last patch [1]. I think if you want to make it faster, you could partially unroll the inner-most loop, like: // size_t * 4 for (; p < aligned_end - (sizeof(size_t) * 3); p += sizeof(size_t) * 4) {

Re: define pg_structiszero(addr, s, r)

2024-11-04 Thread David Rowley
On Tue, 5 Nov 2024 at 06:39, Ranier Vilela wrote: > I think we can add a small optimization to this last patch [1]. > The variable *aligned_end* is only needed in the second loop (for). > So, only before the for loop do we actually declare it. > > Result before this change: > check zeros using BER

Re: define pg_structiszero(addr, s, r)

2024-11-04 Thread Ranier Vilela
Hi. Em seg., 4 de nov. de 2024 às 14:18, Bertrand Drouvot < bertranddrouvot...@gmail.com> escreveu: > Hi, > > On Tue, Nov 05, 2024 at 12:24:48AM +1300, David Rowley wrote: > > On Sat, 2 Nov 2024 at 01:50, Bertrand Drouvot > > wrote: > > > > > > On Fri, Nov 01, 2024 at 09:47:05PM +1300, David Row

Re: define pg_structiszero(addr, s, r)

2024-11-04 Thread Bertrand Drouvot
Hi, On Tue, Nov 05, 2024 at 12:24:48AM +1300, David Rowley wrote: > On Sat, 2 Nov 2024 at 01:50, Bertrand Drouvot > wrote: > > > > On Fri, Nov 01, 2024 at 09:47:05PM +1300, David Rowley wrote: > > > I've attached what I thought a more optimal version might look like in > > > case anyone thinks ma

Re: define pg_structiszero(addr, s, r)

2024-11-04 Thread David Rowley
On Sat, 2 Nov 2024 at 01:50, Bertrand Drouvot wrote: > > On Fri, Nov 01, 2024 at 09:47:05PM +1300, David Rowley wrote: > > I've attached what I thought a more optimal version might look like in > > case anyone thinks making it better is a good idea. > > > > Thanks for the proposal! > > I like the

Re: define pg_structiszero(addr, s, r)

2024-11-04 Thread Bertrand Drouvot
Hi, On Fri, Nov 01, 2024 at 12:50:10PM +, Bertrand Drouvot wrote: > Hi, > > On Fri, Nov 01, 2024 at 09:47:05PM +1300, David Rowley wrote: > > On Fri, 1 Nov 2024 at 20:49, Michael Paquier wrote: > > I've attached what I thought a more optimal version might look like in > > case anyone thinks

Re: define pg_structiszero(addr, s, r)

2024-11-01 Thread Japin Li
On Fri, 01 Nov 2024 at 21:47, David Rowley wrote: > On Fri, 1 Nov 2024 at 20:49, Michael Paquier wrote: >> >> On Fri, Nov 01, 2024 at 07:44:22AM +, Bertrand Drouvot wrote: >> > Worth to add a comment as to why pg_memory_is_all_zeros() should not >> > be used here? >> >> I would not add one in

Re: define pg_structiszero(addr, s, r)

2024-11-01 Thread Bertrand Drouvot
Hi, On Fri, Nov 01, 2024 at 09:47:05PM +1300, David Rowley wrote: > On Fri, 1 Nov 2024 at 20:49, Michael Paquier wrote: > > > > On Fri, Nov 01, 2024 at 07:44:22AM +, Bertrand Drouvot wrote: > > > Worth to add a comment as to why pg_memory_is_all_zeros() should not > > > be used here? > > > >

Re: define pg_structiszero(addr, s, r)

2024-11-01 Thread David Rowley
On Fri, 1 Nov 2024 at 20:49, Michael Paquier wrote: > > On Fri, Nov 01, 2024 at 07:44:22AM +, Bertrand Drouvot wrote: > > Worth to add a comment as to why pg_memory_is_all_zeros() should not > > be used here? > > I would not add one in bufpage.c, documenting that where > pg_memory_is_all_zeros

Re: define pg_structiszero(addr, s, r)

2024-11-01 Thread Michael Paquier
On Fri, Nov 01, 2024 at 07:44:22AM +, Bertrand Drouvot wrote: > Thanks! Cool, will fix that in a bit. > Worth to add a comment as to why pg_memory_is_all_zeros() should not > be used here? I would not add one in bufpage.c, documenting that where pg_memory_is_all_zeros() is defined may be mor

Re: define pg_structiszero(addr, s, r)

2024-11-01 Thread Bertrand Drouvot
Hi, On Fri, Nov 01, 2024 at 04:36:45PM +0900, Michael Paquier wrote: > On Fri, Nov 01, 2024 at 08:21:50PM +1300, David Rowley wrote: > > My vote is to just revert this usage of the function. Anything more > > elaborate would need to check pointer alignment before using any types > > larger than ch

Re: define pg_structiszero(addr, s, r)

2024-11-01 Thread Michael Paquier
On Fri, Nov 01, 2024 at 08:21:50PM +1300, David Rowley wrote: > My vote is to just revert this usage of the function. Anything more > elaborate would need to check pointer alignment before using any types > larger than char. The previous code does not need to do that because > the page is going to

Re: define pg_structiszero(addr, s, r)

2024-11-01 Thread David Rowley
On Fri, 1 Nov 2024 at 20:14, Michael Paquier wrote: > 2) On HEAD at 49d6c7d8daba: > .LVL299: >.loc 1 131 16 is_stmt 0 discriminator 1 view .LVU524 >cmpq$8192, %rbx >je .L419 > > 3) With the patch sent at [1]: > .LVL306: >.loc 3 201 23 is_stmt 1 discriminator 1 view .LVU545

Re: define pg_structiszero(addr, s, r)

2024-11-01 Thread David Rowley
On Fri, 1 Nov 2024 at 19:33, Bertrand Drouvot wrote: > Agree, I did a quick test (see [0]) and it looks like it's significantly > slower > using the new inline function. > > We could try to write a more elaborate version of pg_memory_is_all_zeros(), > but > as it looks like there is only one use

Re: define pg_structiszero(addr, s, r)

2024-11-01 Thread David Rowley
On Fri, 1 Nov 2024 at 19:27, Michael Paquier wrote: > 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 itera

Re: define pg_structiszero(addr, s, r)

2024-11-01 Thread Michael Paquier
On Fri, Nov 01, 2024 at 06:33:51AM +, Bertrand Drouvot wrote: > We could try to write a more elaborate version of pg_memory_is_all_zeros(), > but > as it looks like there is only one use case, then it's probably better to not > implement (revert) this change here and "just" add a comment as to

Re: define pg_structiszero(addr, s, r)

2024-10-31 Thread Bertrand Drouvot
Hi, On Fri, Nov 01, 2024 at 05:45:44PM +1300, David Rowley wrote: > On Thu, 31 Oct 2024 at 19:17, Michael Paquier wrote: > > Seems kind of OK seen from here. I am wondering if others more > > comments about the name of the macro, its location, the fact that we > > still have pagebytes in bufpage

Re: define pg_structiszero(addr, s, r)

2024-10-31 Thread Michael Paquier
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, t

Re: define pg_structiszero(addr, s, r)

2024-10-31 Thread David Rowley
On Thu, 31 Oct 2024 at 19:17, Michael Paquier wrote: > Seems kind of OK seen from here. I am wondering if others more > comments about the name of the macro, its location, the fact that we > still have pagebytes in bufpage.c, etc. This change looks incorrect: @@ -126,18 +124,9 @@ PageIsVerified

Re: define pg_structiszero(addr, s, r)

2024-10-30 Thread Michael Paquier
On Thu, Oct 31, 2024 at 05:52:49AM +, Bertrand Drouvot wrote: > That makes sense to me, done that way in the attached. Seems kind of OK seen from here. I am wondering if others more comments about the name of the macro, its location, the fact that we still have pagebytes in bufpage.c, etc. --

Re: define pg_structiszero(addr, s, r)

2024-10-30 Thread Bertrand Drouvot
Hi, On Thu, Oct 31, 2024 at 09:59:35AM +0900, Michael Paquier wrote: > On Thu, Oct 31, 2024 at 07:55:45AM +1100, Peter Smith wrote: > > +/* > > + * 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 void *ptr, si

Re: define pg_structiszero(addr, s, r)

2024-10-30 Thread Michael Paquier
On Thu, Oct 31, 2024 at 07:55:45AM +1100, Peter Smith wrote: > +/* > + * 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 void *ptr, size_t len) > > The function comment should say 'ptr' instead of 'p', right? Yes.

Re: define pg_structiszero(addr, s, r)

2024-10-30 Thread Peter Smith
+/* + * 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 void *ptr, size_t len) The function comment should say 'ptr' instead of 'p', right? == Kind Regards, Peter Smith. Fujitsu Australia

Re: define pg_structiszero(addr, s, r)

2024-10-30 Thread Bertrand Drouvot
Hi, On Wed, Oct 30, 2024 at 09:09:54AM +0100, Peter Eisentraut wrote: > On 29.10.24 14:58, Bertrand Drouvot wrote: > > hi, > > > > On Tue, Oct 29, 2024 at 10:23:37AM +0100, Peter Eisentraut wrote: > > > On 29.10.24 08:54, Bertrand Drouvot wrote: > > > > +static inline bool > > > > +pg_mem_is_all_

Re: define pg_structiszero(addr, s, r)

2024-10-30 Thread Peter Eisentraut
On 29.10.24 14:58, Bertrand Drouvot wrote: hi, On Tue, Oct 29, 2024 at 10:23:37AM +0100, Peter Eisentraut wrote: On 29.10.24 08:54, Bertrand Drouvot wrote: +static inline bool +pg_mem_is_all_zeros(const char *p, size_t len) This should be a void * pointer please. Thanks for looking at it!

Re: define pg_structiszero(addr, s, r)

2024-10-29 Thread Bertrand Drouvot
hi, On Tue, Oct 29, 2024 at 10:23:37AM +0100, Peter Eisentraut wrote: > On 29.10.24 08:54, Bertrand Drouvot wrote: > > +static inline bool > > +pg_mem_is_all_zeros(const char *p, size_t len) > > This should be a void * pointer please. Thanks for looking at it! Yeah better, done in v4 attached.

Re: define pg_structiszero(addr, s, r)

2024-10-29 Thread Bertrand Drouvot
Hi, On Tue, Oct 29, 2024 at 11:39:03AM +0200, Heikki Linnakangas wrote: > On 29/10/2024 09:54, Bertrand Drouvot wrote: > > > 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 Pete

Re: define pg_structiszero(addr, s, r)

2024-10-29 Thread Peter Eisentraut
On 29.10.24 08:54, Bertrand Drouvot wrote: +static inline bool +pg_mem_is_all_zeros(const char *p, size_t len) This should be a void * pointer please.

Re: define pg_structiszero(addr, s, r)

2024-10-29 Thread Heikki Linnakangas
On 29/10/2024 09:54, Bertrand Drouvot wrote: 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 100 times... check zeros using loop -- elapsed=0.041196s ch

Re: define pg_structiszero(addr, s, r)

2024-10-29 Thread Bertrand Drouvot
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: > > > > +#def

Re: define pg_structiszero(addr, s, r)

2024-10-28 Thread Peter Smith
On Thu, Sep 19, 2024 at 4:57 AM Bertrand Drouvot 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

Re: define pg_structiszero(addr, s, r)

2024-10-28 Thread Ranier Vilela
Em seg., 28 de out. de 2024 às 12:08, Tom Lane escreveu: > Ranier Vilela 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 >

Re: define pg_structiszero(addr, s, r)

2024-10-28 Thread Tom Lane
Ranier Vilela 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 det

Re: define pg_structiszero(addr, s, r)

2024-10-28 Thread Ranier Vilela
Em seg., 28 de out. de 2024 às 11:33, Heikki Linnakangas 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: > >>> +#def

Re: define pg_structiszero(addr, s, r)

2024-10-28 Thread Heikki Linnakangas
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

Re: define pg_structiszero(addr, s, r)

2024-09-18 Thread Bertrand Drouvot
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) > >

Re: define pg_structiszero(addr, s, r)

2024-09-18 Thread Peter Eisentraut
On 18.09.24 06:16, Bertrand Drouvot wrote: +#define pg_structiszero(addr, s, r) \ + do

Re: define pg_structiszero(addr, s, r)

2024-09-18 Thread Bertrand Drouvot
aning up all these static > declarations to make the memcpy() calls cheaper. That can also be > useful for anybody doing a custom pgstats kind, fixed or > variable-numbered. Thanks for looking at it! > > #define pg_structiszero(addr, s, r) \ > >

Re: define pg_structiszero(addr, s, r)

2024-09-17 Thread Michael Paquier
custom pgstats kind, fixed or variable-numbered. #define pg_structiszero(addr, s, r) \ Locating that at the top of pgstat_internal.h seems a bit out of order to me. Perhaps it would be better to move it closer to the inline functions? Also, is this the best name

define pg_structiszero(addr, s, r)

2024-09-17 Thread Bertrand Drouvot
ng forward to your feedback, Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com >From 2e9071b54f7fd28027759de871b662a3eaa8e4a3 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot Date: Tue, 10 Sep 2024 01:22:02 + Subject: [P