public inbox for [email protected]  
help / color / mirror / Atom feed
From: John Naylor <[email protected]>
To: David Rowley <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Chao Li <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Subject: Re: More speedups for tuple deformation
Date: Sat, 31 Jan 2026 09:47:49 +0700
Message-ID: <CANWCAZYUQ-TXp5Tq6QYWktcK0ub9s73dYiprQpm341UcNOds=A@mail.gmail.com> (raw)
In-Reply-To: <CAApHDvo1i-ycAcWnK3L7ZASTuM8mW46kvRqMaUHD46HSuJmx7A@mail.gmail.com>
References: <CAApHDvpoFjaj3+w_jD5uPnGazaw41A71tVJokLDJg2zfcigpMQ@mail.gmail.com>
	<CAApHDvrF6DG7=xD8JGo2HoQKN0LRFNF0ysVt6cKSNPiqbdQOSA@mail.gmail.com>
	<CAApHDvoh3Q413szd-zsUTCpQPWNdpUYvx-fvsB8DP8zOja+ckg@mail.gmail.com>
	<[email protected]>
	<CAApHDvqhbJU_-yF3Hbf4VhX33qXtpeYv3MsvMPDMcDwGGLr9ZQ@mail.gmail.com>
	<rbskhk7scqbxqnaw4o6nh6na2ffcclg3cxn4d4cn5jfr2z7vv3@kadtz65meesb>
	<CAApHDvpDxDFatUskuOfuM7A3VESrx8U7MtYnU_HiB0QLAg94zg@mail.gmail.com>
	<pmik622adey6fnddivkt4uvkulvnc6rasmq3tcbrzeglx4hsn7@f3x6e2eph3w5>
	<rvlc7pb6zn4kydqovcqh72lf2qfcgs3qkj2seq7tcpvxyqwtqt@nrvv6lpehwwa>
	<CAApHDvo1i-ycAcWnK3L7ZASTuM8mW46kvRqMaUHD46HSuJmx7A@mail.gmail.com>

On Tue, Jan 27, 2026 at 8:34 PM David Rowley <[email protected]> wrote:
> I've also included a slightly revised patch. I made a small change to
> the first_null_attr() to get rid of the masking of higher attnums and
> also now making use of __builtin_ctz to find the first NULL attnum in
> the byte. For compilers that don't support that, I've included a
> pg_rightmost_*zero*_pos table. I didn't want to use the pg_bitutils
> table for the rightmost *one* pos as it meant having to special-case
> what happens when using index 255, as that would return 0, and I want
> 8. I'll make the MSVC version use _BitScanForward() in the next patch.

I don't get why we'd need to special-case 255 in only one place.

+ /* Process all bytes up to just before the byte for the natts index */
+ for (bytenum = 0; bytenum < lastByte; bytenum++)
+ {
+   /* break if there's any NULL attrs (a 0 bit) */
+   if (bits[bytenum] != 0xFF)
+   break;
+ }
+
+ res = bytenum << 3;
+
+#ifdef HAVE__BUILTIN_CTZ
+   res += __builtin_ctz(~bits[bytenum]);
+#else
+   res += pg_rightmost_zero_pos[bits[bytenum]];
+#endif

If bits[bytenum] is 255, then __builtin_ctz(0) is undefined. The top
of the function says

+ * We expect that 'bits' contains at least one 0 bit somewhere in the mask,
+ * not necessarily < natts.

...in which case it should be well defined everywhere. Am I missing
something? If we need to handle the 255 case, this should work:

pg_rightmost_one_pos32(~((uint32) bits[bytenum]))

--
John Naylor
Amazon Web Services






view thread (19+ 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]
  Subject: Re: More speedups for tuple deformation
  In-Reply-To: <CANWCAZYUQ-TXp5Tq6QYWktcK0ub9s73dYiprQpm341UcNOds=A@mail.gmail.com>

* 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