Re: Make tuple deformation faster - Mailing list pgsql-hackers
From | David Rowley |
---|---|
Subject | Re: Make tuple deformation faster |
Date | |
Msg-id | CAApHDvot1CKPmd6GF8sgreQ2T5DZjvxM91uqoSeEB9MKf4cWKw@mail.gmail.com Whole thread Raw |
In response to | Re: Make tuple deformation faster (Matthias van de Meent <boekewurm+postgres@gmail.com>) |
Responses |
Re: Make tuple deformation faster
|
List | pgsql-hackers |
On Mon, 1 Jul 2024 at 22:07, Matthias van de Meent <boekewurm+postgres@gmail.com> wrote: > Cool, that's similar to, but even better than, my patch from 2021 over at [0]. I'll have a read of that. Thanks for pointing it out. > One thing I'm slightly concerned about is that this allocates another > 8 bytes for each attribute in the tuple descriptor. While that's not a > lot when compared with the ->attrs array, it's still quite a lot when > we might not care at all about this data; e.g. in temporary tuple > descriptors during execution, in intermediate planner nodes. I've not done it in the patch, but one way to get some of that back is to ditch pg_attribute.attcacheoff. There's no need for it after this patch. That's only 4 out of 8 bytes, however. I think in most cases due to FormData_pg_attribute being so huge, the aset.c power-of-2 roundup behaviour will be unlikely to cross a power-of-2 boundary. The following demonstrates which column counts that actually makes a difference with: select c as n_cols,old_bytes, new_bytes from (select c,24+104*c as old_bytes, 32+100*c+8*c as new_bytes from generate_series(1,1024) c) where position('1' in old_bytes::bit(32)::text) != position('1' in new_bytes::bit(32)::text); That returns just 46 column counts out of 1024 where we cross a power of 2 boundaries with the patched code that we didn't cross in master. Of course, larger pallocs will result in a malloc() directly, so perhaps that's not a good measure. At least for smaller column counts it should be mainly the same amount of memory used. There are only 6 rows in there for column counts below 100. I think if we were worried about memory there are likely 100 other things we could do to reclaim some. It would only take some shuffling of fields in RelationData. I count 50 bytes of holes in that struct out of the 488 bytes. There are probably a few that could be moved without upsetting the struct-field-order-lords too much. > Did you test for performance gains (or losses) with an out-of-line > TupleDescDeformAttr array? One benefit from this would be that we > could reuse the deform array for suffix truncated TupleDescs, reuse of > which currently would require temporarily updating TupleDesc->natts > with a smaller value; but with out-of-line ->attrs and ->deform_attrs, > we could reuse these arrays between TupleDescs if one is shorter than > the other, but has otherwise fully matching attributes. I know that > btree split code would benefit from this, as it wouldn't have to > construct a full new TupleDesc when it creates a suffix-truncated > tuple during page splits. No, but it sounds easy to test as patch 0002 moves that out of line and does nothing else. > > 0004: Adjusts the attalign to change it from char to uint8. See below. > > > > The 0004 patch changes the TupleDescDeformAttr.attalign to a uint8 > > rather than a char containing 'c', 's', 'i' or 'd'. This allows much > > more simple code in the att_align_nominal() macro. What's in master is > > quite a complex expression to evaluate every time we deform a column > > as it much translate: 'c' -> 1, 's' -> 2, 'i' -> 4, 'd' -> 8. If we > > just store that numeric value in the struct that macro can become a > > simple TYPEALIGN() so the operation becomes simple bit masking rather > > than a poorly branch predictable series of compare and jump. > > +1, that's something I'd missed in my patches, and is probably the > largest contributor to the speedup. I think so too and I did consider if we should try and do that to pg_attribute, renaming the column to attalignby. I started but didn't finish a patch for that. > > I'll stick this in the July CF. It would be good to get some feedback > > on the idea and feedback on whether more work on this is worthwhile. > > Do you plan to remove the ->attcacheoff catalog field from the > FormData_pg_attribute, now that (with your patch) it isn't used > anymore as a placeholder field for fast (de)forming of tuples? Yes, I plan to do that once I get more confidence I'm on to a winner here. Thanks for having a look at this. David
pgsql-hackers by date: