Re: Lookup penalty for VARIADIC patch

From: "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)com>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Lookup penalty for VARIADIC patch
Date: 2008-07-15 22:22:01
Message-ID: 162867790807151522l5f26a11fw92047b1a28155b81@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2008/7/15 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> The proposed variadic-functions patch inserts some none-too-cheap code
> into FuncnameGetCandidates (it's deconstructing the proargmodes column
> to see if the function is variadic or not) which gets executed whether
> or not there are any variadic functions involved. I checked whether
> this would cause a noticeable slowdown in practice, and got a
> discouraging answer:
>
> $ cat functest.sql
> select sin(5), cos(45);
> $ pgbench -c 1 -t 10000 -n -f functest.sql regression
> transaction type: Custom query
> scaling factor: 1
> query mode: simple
> number of clients: 1
> number of transactions per client: 10000
> number of transactions actually processed: 10000/10000
> tps = 927.418555 (including connections establishing)
> tps = 928.953281 (excluding connections establishing)
>
> That's with the patch. CVS HEAD gets
> tps = 1017.901218 (including connections establishing)
> tps = 1019.724948 (excluding connections establishing)
>
> so that code is adding about 10% to the total round-trip execution time
> for the select --- considering all the other overhead involved there,
> that means the actual cost of FuncnameGetCandidates has gone up probably
> by an order of magnitude. And that's for the *best* case, where
> proargmodes is null so SysCacheGetAttr will fall out without returning
> an array to examine. This doesn't seem acceptable to me.
>
> What I'm thinking of doing is adding a column to pg_proc that provides
> the needed info in a trivial-to-get-at format. There are two ways we
> could do it: a bool column that is TRUE if the function is variadic,
> or an oid column that is the variadic array's element type, or zero
> if the function isn't variadic. The second would take more space but
> would avoid having to do a catalog lookup to get the element type in
> the case that the function is indeed variadic. I'm leaning to the
> second way but wanted to know if anyone objected?
>
> Also, it occurs to me that we could buy back a good part of the extra
> space if we allowed pg_proc.probin to be NULL for internal functions.
> Right now it's always "-" in that case, which is useless ...

probin is used in some unofficial pl hacks, so this space its some
times used. I vote for special column that containst variadic element
type

Regards
Pavel Stehule
>
> regards, tom lane
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2008-07-15 22:24:17 Re: Core team statement on replication in PostgreSQL
Previous Message Alvaro Herrera 2008-07-15 22:18:46 Re: Core team statement on replication in PostgreSQL