Re: SQLFunctionCache and generic plans

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> I did multiple benchmarking, and still looks so the proposed patch doesn't
> help and has significant overhead

Yeah, your fx() test case is clearly worse. For me,

HEAD:

regression=# do $$
begin
for i in 1..1000000 loop
perform fx((random()*100)::int); -- or fx2
end loop;
end;
$$;
DO
Time: 5229.184 ms (00:05.229)

PATCH:

regression=# do $$
begin
for i in 1..1000000 loop
perform fx((random()*100)::int); -- or fx2
end loop;
end;
$$;
DO
Time: 6934.413 ms (00:06.934)

Adding some debug printout shows me that BuildCachedPlan is called to
construct a custom plan on every single execution, which is presumably
because the patch doesn't make any attempt to carry plancache state
across successive executions of the same query. If we were saving that
state it would have soon switched to a generic plan and then won big.

So, even though I thought we could leave that for later, it seems like
maybe we have to have it before we'll have a committable patch.

There might be some residual inefficiency in there though. In the
unpatched code we'd be calling pg_parse_query and pg_plan_query once
per execution. You'd think that would cost more than BuildCachedPlan,
which can skip the raw-parsing part. Even more interesting, the
patch gets slower yet if we use a new-style SQL function:

regression=# create or replace function fx3 (int) returns int immutable
regression-# begin atomic select $1 + $1; end;
CREATE FUNCTION
Time: 0.813 ms
regression=# do $$
begin
for i in 1..1000000 loop
perform fx3((random()*100)::int); -- or fx2
end loop;
end;
$$;
DO
Time: 8007.062 ms (00:08.007)

That makes no sense either, because with a new-style SQL
function we should be skipping parse analysis as well.

But wait: HEAD takes
Time: 6632.709 ms (00:06.633)
to do the same thing. So somehow the new-style SQL function
stuff is very materially slower in this use-case, with or
without this patch. I do not understand why.

Definitely some performance investigation needs to be done here.
Even without cross-query plan caching, I don't see why the
patch isn't better than it is. It ought to be at least
competitive with the unpatched code.

(I've not read the v5 patch yet, so I have no theories.)

regards, tom lane