Thread: 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@samurai.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@samurai.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@postgresql.org so that your > message can get through to the mailing list cleanly -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.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@momjian.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@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.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@momjian.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@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.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@momjian.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@momjian.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