On Sat, 30 Nov 2024 at 02:54, Victor Yegorov <vyego...@gmail.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/caaphdvo9e0xg71wrefyarv5n4xnplk4k8ljd0msr3c9kr2v...@mail.gmail.com
v5-0001-Introduce-CompactAttribute-array-in-TupleDesc.patch
Description: Binary data
v5-0002-Use-CompactAttribute-instead-of-FormData_pg_attri.patch
Description: Binary data
v5-0003-Optimize-alignment-calculations-in-tuple-form-def.patch
Description: Binary data
v5-0004-Remove-pg_attribute.attcacheoff-column.patch
Description: Binary data
v5-0005-Speedup-tuple-deformation-with-additional-functio.patch
Description: Binary data
