On Thu, 5 Dec 2024 at 13:09, Andres Freund <andres@anarazel.de> wrote:
> On 2024-12-05 11:52:01 +1300, David Rowley wrote:
> > On Thu, 5 Dec 2024 at 03:51, Andres Freund <andres@anarazel.de> wrote:
> > > Possibly stupid idea: Could we instead store the attributes *before* the main
> > > TupleDescData, with increasing "distance" for increased attnos? That way we
> > > wouldn't need to calculate any variable offsets. Of course the price would be
> > > to have some slightly more complicated invocation of pfree(), but that's
> > > comparatively rare.
> >
> > Are you thinking this to make the address calculation cheaper? or so
> > that the hacky code that truncates the TupleDesc would work without
> > crashing still?
>
> Primarily to make the address calculation cheaper.
I mostly got that working, but there were quite several adjustments
needed to fix various things. For example, there are a few places that
are pfreeing a TupleDesc without going through FreeTupleDesc(); namely
index_truncate_tuple() and InsertOneTuple(). There was also a bit more
complexity around the TupleDescs stored in DSA memory as the base
address of those is the start of the FormData_pg_attribute array, so
some offsetting is needed to get the TupleDesc address. None of those
changes are complex themselves, but it was quite painful to find all
those places and I started to worry that there might end up being a
bit more fallout from that method and I basically chickened out. I
agree the address calculation is cheaper, but accessing
FormData_pg_attribute isn't as performance-critical anymore as
CompactAttribute is doing the performance-critical work now.
I've now pushed the 0001 patch and the 0002 patch as one commit. I'm
going to give the buildfarm a bit of time then push the commit to
remove pg_attribute.attcacheoff. I'm planning on letting that settle
overnight then if all is well push the attalignby changes.
David
