Idea for minor tstore optimization

I notice that several of the call sites of tuplestore_puttuple() start
with arrays of datums and nulls, call heap_form_tuple(), and then switch
into the tstore's context and call tuplestore_puttuple(), which
deep-copies the HeapTuple into the tstore. ISTM it would be faster and
simpler to provide a tuplestore_putvalues(), which just takes the datum
+ nulls arrays and avoids the additional copy.

-Neil

Neil Conway <neilc(at)samurai(dot)com> writes:
> I notice that several of the call sites of tuplestore_puttuple() start
> with arrays of datums and nulls, call heap_form_tuple(), and then switch
> into the tstore's context and call tuplestore_puttuple(), which
> deep-copies the HeapTuple into the tstore. ISTM it would be faster and
> simpler to provide a tuplestore_putvalues(), which just takes the datum
> + nulls arrays and avoids the additional copy.

Seems reasonable. Check whether tuplesort should offer the same, while
you are at it.

regards, tom lane

Added to TODO:

* Avoid tuple some tuple copying in sort routines

http://archives.postgresql.org/pgsql-hackers/2008-02/msg01206.php

---------------------------------------------------------------------------

Tom Lane wrote:
> Neil Conway <neilc(at)samurai(dot)com> writes:
> > I notice that several of the call sites of tuplestore_puttuple() start
> > with arrays of datums and nulls, call heap_form_tuple(), and then switch
> > into the tstore's context and call tuplestore_puttuple(), which
> > deep-copies the HeapTuple into the tstore. ISTM it would be faster and
> > simpler to provide a tuplestore_putvalues(), which just takes the datum
> > + nulls arrays and avoids the additional copy.
>
> Seems reasonable. Check whether tuplesort should offer the same, while
> you are at it.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Added to TODO:
> * Avoid tuple some tuple copying in sort routines
> http://archives.postgresql.org/pgsql-hackers/2008-02/msg01206.php

Actually ... isn't this done already?

http://archives.postgresql.org/pgsql-patches/2008-02/msg00176.php

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Added to TODO:
> > * Avoid tuple some tuple copying in sort routines
> > http://archives.postgresql.org/pgsql-hackers/2008-02/msg01206.php
>
> Actually ... isn't this done already?
>
> http://archives.postgresql.org/pgsql-patches/2008-02/msg00176.php

Yea, removed because I thought you just did it.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> Actually ... isn't this done already?
>> http://archives.postgresql.org/pgsql-patches/2008-02/msg00176.php

> Yea, removed because I thought you just did it.

Oh, wait, that's just a -patches entry; it doesn't look like Neil
ever committed it. Neil, how come?

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Tom Lane wrote:
> >> Actually ... isn't this done already?
> >> http://archives.postgresql.org/pgsql-patches/2008-02/msg00176.php
>
> > Yea, removed because I thought you just did it.
>
> Oh, wait, that's just a -patches entry; it doesn't look like Neil
> ever committed it. Neil, how come?

I thought this was Neil's commit that you just did:

http://archives.postgresql.org/pgsql-committers/2008-03/msg00439.php

but I see now this was another patch queue patch. I have re-added the
TODO item and included your URL.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> Oh, wait, that's just a -patches entry; it doesn't look like Neil
>> ever committed it. Neil, how come?

> I thought this was Neil's commit that you just did:

No, the one I just put in was the one you have listed under "Avoid
needless copy in nodeMaterial". That should be removed, but the
tstore optimization thread is still live.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Tom Lane wrote:
> >> Oh, wait, that's just a -patches entry; it doesn't look like Neil
> >> ever committed it. Neil, how come?
>
> > I thought this was Neil's commit that you just did:
>
> No, the one I just put in was the one you have listed under "Avoid
> needless copy in nodeMaterial". That should be removed, but the
> tstore optimization thread is still live.

I am thinking I need a todo queue separate from the patches queue,
except I often can't figure out which is which until I am done.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Sat, 2008-03-22 at 21:24 -0400, Tom Lane wrote:
> Oh, wait, that's just a -patches entry; it doesn't look like Neil
> ever committed it. Neil, how come?

Sorry, slipped through the cracks -- I've now committed the patch.

-Neil