Re: Make tuple deformation faster

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Victor Yegorov <vyegorov(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Make tuple deformation faster
Date: 2024-12-02 10:24:31
Message-ID: CAApHDvqEuNGsNjH2Jh_qPKnefU=Pu0tpJj7Pm1Wv+PJH0-y4og@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, 30 Nov 2024 at 02:54, Victor Yegorov <vyegorov(at)gmail(dot)com> wrote:
> I've been testing this patch for the last week, I have M3 and i7 based MBP around.

Thanks for having a look at this and running the benchmarks.

> Construct
> sizeof(FormData_pg_attribute) * (src)->natts
> is used in 7 places (in various forms), I thought it might be good
> to use a macro here, say TupleArraySize(natts).

I ended up adjusting the code here so that TupleDescSize() returns the
full size and TupleDescAttrAddress() manually calculates the offset to
start the FormData_pg_attribute array. That allows
TupleDescFullSize() to be deleted. I changed how TupleDescCopy()
works as it used to perform the memcpy in 2 parts. I've changed that
to now perform a single memcpy() and reset the ->attrs field after the
memcpy so that it correctly points to the address for its own
TupleDesc rather than the one from the source.

> In v4-0002-Introduce-CompactAttribute-array-in-TupleDesc.patch
>
> +#define COMPACT_ATTR_IS_PACKABLE(att) \
> +> ((att)->attlen == -1 && att->attispackable)
>
> Seems second att needs parenthesis around it.

Adjusted. Thanks.

> Although I haven't seen 30% speedup, I find this change very good to have.

I think there's a bit more juice to squeeze out still. I started
another thread for a much different approach to increasing the tuple
deform performance over in [1]. The benchmarks I showed over there
show the results with all the v4 patches on this thread plus that
patch, and also another set of results from just the v4 patches from
here. My Apple M2 very much likes the patch from the other thread. I
don't have any decent Intel hardware to test on.

I've attached a v5 set of patches, which I think addresses everything
you mentioned. I've also shuffled the patches around a little to how
I think they should be committed. Here's a summary:

v5-0001: Adds the CompactAttribute struct, includes it in TupleDesc
and adds all the code to populate it. Also includes a very small
number of users of CompactAttribute.

v5-0002: Adjusts dozens of locations to use CompactAttribute struct
instead of the Form_pg_attribute struct. Lots of churn, but not
complex changes. Separated out from v5-0001 so it's easier to see the
important changes 0001 is making.

v5-0003: Change CompactAttribute.attalign char field to attalignby to
uint8 field to optimise alignment calculations and remove branching.

v5-0004: Delete the now unused pg_attribute.attcacheoff column and
Form_pg_attribute field.

v5-0005: This is the patch from [1] rebased atop of this patch set.
I'll pick up the discussion on that thread, but offering a rebased
version here in case you'd like to try that one.

I spent all day today reviewing and fixing up a few missing comments
for the v5 patch series. I'm quite happy with these now. If nobody
else wants to look or test, I plan on pushing these tomorrow (Tuesday
UTC+13). If anyone wants me to delay so they can look, they better let
me know soon.

David

[1] https://postgr.es/m/CAApHDvo9e0XG71WrefYaRv5n4xNPLK4k8LjD0mSR3c9KR2vi2Q@mail.gmail.com

Attachment Content-Type Size
v5-0001-Introduce-CompactAttribute-array-in-TupleDesc.patch application/octet-stream 32.2 KB
v5-0002-Use-CompactAttribute-instead-of-FormData_pg_attri.patch application/octet-stream 26.6 KB
v5-0003-Optimize-alignment-calculations-in-tuple-form-def.patch application/octet-stream 18.4 KB
v5-0004-Remove-pg_attribute.attcacheoff-column.patch application/octet-stream 11.5 KB
v5-0005-Speedup-tuple-deformation-with-additional-functio.patch application/octet-stream 9.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2024-12-02 10:42:20 Re: Interrupts vs signals
Previous Message Peter Eisentraut 2024-12-02 10:10:56 meson missing test dependencies