Idea for minor tstore optimization

Started by Neil Conwayalmost 18 years ago10 messages
#1Neil Conway
neilc@samurai.com

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

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Neil Conway (#1)
Re: Idea for minor tstore optimization

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

#3Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#2)
Re: Idea for minor tstore optimization

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. +

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#3)
Re: Idea for minor tstore optimization

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

#5Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#4)
Re: Idea for minor tstore optimization

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. +

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#5)
Re: Idea for minor tstore optimization

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

#7Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#6)
Re: Idea for minor tstore optimization

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. +

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#7)
Re: Idea for minor tstore optimization

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

#9Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#8)
Re: Idea for minor tstore optimization

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. +

#10Neil Conway
neilc@samurai.com
In reply to: Tom Lane (#6)
Re: Idea for minor tstore optimization

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