Thanks for having a look at this.
On Wed, 25 Feb 2026 at 22:49, Álvaro Herrera <[email protected]> wrote:
> With all these patches applied, I get this from pahole for
> CompactAttribute,
> The 'attnullability' thing stands out. It probably doesn't make much of
> a difference -- considering that it's down to 8 bytes already and I
> doubt anything will change if it's instead 6 -- but given these
> definitions
We could get it to 6 bytes. The alignment is 2, so there's certainly a
memory saving there. I think the primary reason to make it 8 was that
the LEA instruction can be used without any other instructions to
calculate the address of the first CompactAttribute element to use.
LEA can shift the array index to multiply it by 1,2,4 or 8, and
because it can do shift and add concurrently, it can utilise the add
to add the offset of compact_attrs in the TupleDesc struct.
I see gcc doing this when I make attnum a size_t. This is effectively
what I described above:
1c46: 48 8d 74 c2 20 lea rsi,[rdx+rax*8+0x20]
The 0x20 is the offset to the start of compact_attrs in TupleDescData.
rax is attnum. rdx is the tupledesc. Effectively, that's:
rsi = ((char *) tupledesc) + (attnum << 3) + offsetof(TupleDescData,
compact_attrs);
If I make the CompactAttribute 6 bytes, I see gcc producing two LEA
instructions instead of one:
1c80: 48 8d 34 40 lea rsi,[rax+rax*2]
1c89: 49 8d 74 74 20 lea rsi,[r12+rsi*2+0x20]
rax is attnum again. r12 is the tupledesc. Effectively that's:
/* multiply attnum by 3, store in rdi */
rsi = attnum + (attnum << 1);
/* multiply rdi by 2 to make multiply by 6 and at that plus 32 to the
tupledesc's addr. */
rsi = ((char *) tupledesc) + (rsi << 1) + offsetof(TupleDescData,
compact_attrs);
I expect that the extra address calculation costs plus the extra
bitwise-AND for checking attbyval will cost more than it will save.
> Regarding deform_bench, I wonder if we shouldn't start a proper
> benchmarking suite (of which this would the first participant) by
> putting this in src/benchmark/tuple_deform instead (or something like
> that) instead of relegating it to src/test/modules. It doesn't make
> much of a technical difference, but it is a bit of a statement that more
> tools are welcome.
Yeah. Andres was talking about this in [1]. I'd originally not planned
on committing deform_bench at all and only put it in the current
location in the patch as that seemed like the best location for it
that we have today. Andres mentioned just having 1 extension and
adding functions to it. That seems fine to me. I have some other
things that could go in there. If it's just one extension, then
there's likely no need to make a special directory for it. Maybe
src/test/modules is fine. For now, I don't want to focus too much as
I'd rather get the deformation speedups ready and in.
David
[1]
https://postgr.es/m/rbxc2qqhsvzxpukgd36caoa4ydgn5r22fxktxanrkn6nobg7j6%4027b4vogohgu2
