Peter Eisentraut <peter_e@gmx.net> writes:
> On 12/28/12 5:11 PM, Heikki Linnakangas wrote:
>> It looks like the regression is caused by extra copying of the parse
>> tree and plan trees. Node-copy-related functions like AllocSetAlloc and
>> _copy* are high in the profile, They are also high in the 9.1 profile,
>> but even more so in 9.2.
Hm ... those 9.2 changes were supposed to *improve* performance, and I
believe they did so for code paths where the plan cache is actually
doing something useful. In this path, it's basically useless.
>> I hacked together a quick&dirty patch to reduce the copying of
>> single-shot plans, and was able to buy back much of the regression I was
>> seeing on DBT-2. Patch attached. But of course, DBT-2 really should be
>> preparing the queries once with SPI_prepare, and reusing them thereafter.
> I was recently profiling an application that uses a fair amount of
> PL/pgSQL with dynamic queries and also noticed AllocSetAlloc high in the
> profile. I was getting suspicious now and compared 9.1 and 9.2
> performance: 9.2 is consistently about 3% slower. Your patch doesn't
> seem to have a measurable effect, but it might be if I ran the test for
> longer.
I'm inclined to think that Heikki's patch doesn't go far enough, if we
want to optimize behavior in this case. What we really want to happen
is that parsing, planning, and execution all happen in the caller's
memory context, with no copying of parse or plan trees at all - and we
could do without overhead such as dependency extraction and invalidation
checking, too. This would make SPI_execute a lot more comparable to the
behavior of exec_simple_query().
So basically plancache.c has got no useful functionality to offer here.
But to avoid having multiple drastically different code paths in spi.c,
it would be nice if we had some sort of "shell" API that would provide
the illusion of using a CachedPlan without any of the overhead that
comes along with actually being able to save or reuse the plan.
Heikki's "oneshot" concept is moving in that direction, but not far
enough IMO. I'm thinking we don't want it to create any new memory
contexts at all, just palloc a CachedPlan object directly in the
caller's memory context and link to the caller-supplied trees.
I'll take a whack at that ...
regards, tom lane
