Re: More speedups for tuple deformation

On Tue, Jan 27, 2026 at 8:34 PM David Rowley <dgrowleyml(at)gmail(dot)com> 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