Tuesday, July 15, 2008

[HACKERS] Lookup penalty for VARIADIC patch

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

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

No comments: