public inbox for [email protected]
help / color / mirror / Atom feedFrom: Junwang Zhao <[email protected]>
To: David Rowley <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: John Naylor <[email protected]>
Cc: Chao Li <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Subject: Re: More speedups for tuple deformation
Date: Sun, 15 Mar 2026 21:50:04 +0800
Message-ID: <CAEG8a3LHoaXuzD8PoHqBTixqbu=zn54o1DbHb1TjtXMMbEbiOQ@mail.gmail.com> (raw)
In-Reply-To: <CAApHDvqp_DGNkz3Rr29hPuSYdai5c_32mWSv2inrW2ufsE+qZQ@mail.gmail.com>
References: <CAApHDvpbntG7V3_EsZ+w-V=jU-y8rFmv9RB1EDJm4sxKno-4UA@mail.gmail.com>
<e7sto7tk5dk5hfyvoocaddnxcngemcmfvbuh23l32w5cssaizy@znuphjqug7qe>
<CAApHDvpuEbhvH1ViCZRz5vks+_bGbEnPoEdZYAZXK76_isb_+Q@mail.gmail.com>
<v6z545yozjtywghn5glujemu72z4i4ynadsc2xks4ejotdg7yl@4rry7ixwr4us>
<CANWCAZabO1oj+khF+YNVpmkTQwRRyNJesbsBhRFL5emZJh3tow@mail.gmail.com>
<lzgoxzbh2gel5w362revuwaecrsbjr44kjdzrewuejugcodkeq@ixymojwnylsy>
<CAApHDvodSVBj3ypOYbYUCJX+NWL=VZs63RNBQ_FxB_F+6QXF-A@mail.gmail.com>
<rbxc2qqhsvzxpukgd36caoa4ydgn5r22fxktxanrkn6nobg7j6@27b4vogohgu2>
<CAApHDvpWQn8sXDYpSNNpieJW-UTG4Nf4TVjT8ew64L073hz-Fw@mail.gmail.com>
<mq6ddpgctt42srolsvo5kph2s6shfg62meb7i5fbg6n3s73zju@2n7gviiyga3h>
<uhqul2ryci4tyg5ylddjrmf4kybzwb7m5z7rmurhhjp37vrn5f@zgxil7egr62n>
<CAApHDvq21qQigiM6z2YgadFusQC_pfEYP8D=oQCrwJ_kKzcqDg@mail.gmail.com>
<CAApHDvpdB1t7LCgH8=KOKC6VBb2rsEbaas0FiXo5awsRgCsDxQ@mail.gmail.com>
<CAEG8a3KeKcZxJsH9nL+D1JzC4Ekx51ps7-1ZGWkwdXbPS5jTXw@mail.gmail.com>
<CAApHDvqp_DGNkz3Rr29hPuSYdai5c_32mWSv2inrW2ufsE+qZQ@mail.gmail.com>
On Fri, Mar 13, 2026 at 8:19 PM David Rowley <[email protected]> wrote:
>
> Thanks for having a look.
>
>
> On Sun, 8 Mar 2026 at 05:36, Junwang Zhao <[email protected]> wrote:
> > I have some comments on v12-0004.
> >
> > 1.
> >
> > + off += cattr->attlen;
> > + firstNonCachedOffsetAttr = i + 1;
> > + }
> > +
> > + tupdesc->firstNonCachedOffsetAttr = firstNonCachedOffsetAttr;
> > + tupdesc->firstNonGuaranteedAttr = firstNonGuaranteedAttr;
> > +}
> >
> > The firstNonCachedOffsetAttr seems to be the first variable width
> > attribute, but it seems that the offset of this attribute can be cached,
> > for example, in a table defined as (int, text), the offset of
> > firstNonCachedOffsetAttr should be 4, is that correct?
>
> Yes.
>
> > If TupleDescFinalize records the offset firstNonCachedOffsetAttr,
> > it might save one iterator of the deforming loop. For example,
> > add something like the following after the above mentioned code.
> >
> > if (firstNonCachedOffsetAttr < tupdesc->natts)
> > {
> > cattr = TupleDescCompactAttr(tupdesc, firstNonCachedOffsetAttr);
> > cattr->attcacheoff = off;
> > }
>
> The problem is that short varlenas have 1 byte alignment and normal
> varlenas have 4 byte alignment. It might be possible to do something
> if the previous column is 4-byte aligned and has a length of 4 or 8,
> since that means the offset must also be 1-byte aligned. The main
> reason I don't want to do this is that the only positive is that
> *maybe* 1 extra column can be deformed with a fixed offset. The
> drawback is that the following code *has* to use
> att_addlength_pointer(), *regardless* instead of "off += attlen;".
> This means more deforming code and more complexity in
> TupleDescFinalize(). I'd rather not do this.
That explains, thanks.
>
> > 2.
> >
> > in slot_deform_heap_tuple, there are multiple statements setting
> > firstNonCacheOffsetAttr,
> >
> > + firstNonCacheOffsetAttr = tupleDesc->firstNonCachedOffsetAttr;
> >
> > + /* We can only use any cached offsets until the first NULL attr */
> > + firstNonCacheOffsetAttr = Min(firstNonCacheOffsetAttr,
> > + firstNullAttr);
> >
> > + /* We can only fetch as many attributes as the tuple has. */
> > + firstNonCacheOffsetAttr = Min(firstNonCacheOffsetAttr, natts);
> >
> > Based on the logic, it seems the second one could be moved
> > to the third position, and the third one could then be safely
> > removed?
>
> Yeah. Well spotted. I've done that in the attached.
>
> I've also modified the 0006 patch to add a new deform_bench_select()
> function which allows the benchmark to call the new selective deform
> function. See the attached graphs comparing master to v13-0001-0005
> and master to v13-0001-0006. It's good to see that there's still quite
> a large speedup even from the tests that don't have an attcacheoff for
> the column being deformed. Tests 1 and 5 do have a attcacheoff for the
> column deformed, so they're a good bit faster again. To get the
> 0001-0006 results, I used the deform_test_run.sh script from [1] and
> modified it to call deform_bench_select() instead of deform_bench().
>
> I also noticed that when building with older gcc versions, I was
> getting warnings about attlen and 'off' not being initialised. I ended
> up switching back to the do/while loops to fix that rather than adding
> needless initialisation, which would add overhead. 1 loop is
> guaranteed, and the older compiler is not clever enough to work that
> out.
>
> David
>
> [1] https://postgr.es/m/CAApHDvo1i-ycAcWnK3L7ZASTuM8mW46kvRqMaUHD46HSuJmx7A@mail.gmail.com
--
Regards
Junwang Zhao
view thread (31+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: More speedups for tuple deformation
In-Reply-To: <CAEG8a3LHoaXuzD8PoHqBTixqbu=zn54o1DbHb1TjtXMMbEbiOQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox