On 28.12.2012 23:53, Peter Eisentraut wrote:
On 12/27/12 1:07 AM, Pavel Stehule wrote:
Hello
I rechecked performance of dynamic SQL and it is significantly slower
in 9.2 than 9.1
-- 9.1
postgres=# create or replace function test() returns void as $$ begin
for i in 1..1000000 loop execute 'select 1'; end loop; end $$ language
plpgsql;
I think this is the same as the case discussed at
<CAD4+=qWnGU0qi+iq=eph6egpuunscysgdtgkazizevrggjo...@mail.gmail.com>.
Yeah, probably so.
As it happens, I just spent a lot of time today narrowing down yet
another report of a regression in 9.2, when running DBT-2:
http://archives.postgresql.org/pgsql-performance/2012-11/msg00007.php.
It looks like that is also caused by the plancache changes. DBT-2
implements the transactions using C functions, which use SPI_execute()
to run all the queries.
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.
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.
- Heikki
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index e3de7f2..0b047d0 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -75,7 +75,7 @@ PrepareQuery(PrepareStmt *stmt, const char *queryString)
* to see the unmodified raw parse tree.
*/
plansource = CreateCachedPlan(stmt->query, queryString,
- CreateCommandTag(stmt->query));
+ CreateCommandTag(stmt->query), false);
/* Transform list of TypeNames to array of type OIDs */
nargs = list_length(stmt->argtypes);
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 5a11c6f..d7522f1 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -49,7 +49,7 @@ static Portal SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
ParamListInfo paramLI, bool read_only);
static void _SPI_prepare_plan(const char *src, SPIPlanPtr plan,
- ParamListInfo boundParams);
+ ParamListInfo boundParams, bool oneshot);
static int _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
Snapshot snapshot, Snapshot crosscheck_snapshot,
@@ -354,7 +354,7 @@ SPI_execute(const char *src, bool read_only, long tcount)
plan.magic = _SPI_PLAN_MAGIC;
plan.cursor_options = 0;
- _SPI_prepare_plan(src, &plan, NULL);
+ _SPI_prepare_plan(src, &plan, NULL, true);
res = _SPI_execute_plan(&plan, NULL,
InvalidSnapshot, InvalidSnapshot,
@@ -505,7 +505,7 @@ SPI_execute_with_args(const char *src,
paramLI = _SPI_convert_params(nargs, argtypes,
Values, Nulls);
- _SPI_prepare_plan(src, &plan, paramLI);
+ _SPI_prepare_plan(src, &plan, paramLI, true);
res = _SPI_execute_plan(&plan, paramLI,
InvalidSnapshot, InvalidSnapshot,
@@ -546,7 +546,7 @@ SPI_prepare_cursor(const char *src, int nargs, Oid *argtypes,
plan.parserSetup = NULL;
plan.parserSetupArg = NULL;
- _SPI_prepare_plan(src, &plan, NULL);
+ _SPI_prepare_plan(src, &plan, NULL, false);
/* copy plan to procedure context */
result = _SPI_make_plan_non_temp(&plan);
@@ -583,7 +583,7 @@ SPI_prepare_params(const char *src,
plan.parserSetup = parserSetup;
plan.parserSetupArg = parserSetupArg;
- _SPI_prepare_plan(src, &plan, NULL);
+ _SPI_prepare_plan(src, &plan, NULL, false);
/* copy plan to procedure context */
result = _SPI_make_plan_non_temp(&plan);
@@ -1082,7 +1082,7 @@ SPI_cursor_open_with_args(const char *name,
paramLI = _SPI_convert_params(nargs, argtypes,
Values, Nulls);
- _SPI_prepare_plan(src, &plan, paramLI);
+ _SPI_prepare_plan(src, &plan, paramLI, true);
/* We needn't copy the plan; SPI_cursor_open_internal will do so */
@@ -1656,7 +1656,8 @@ spi_printtup(TupleTableSlot *slot, DestReceiver *self)
* parsing is also left in CurrentMemoryContext.
*/
static void
-_SPI_prepare_plan(const char *src, SPIPlanPtr plan, ParamListInfo boundParams)
+_SPI_prepare_plan(const char *src, SPIPlanPtr plan, ParamListInfo boundParams,
+ bool oneshot)
{
List *raw_parsetree_list;
List *plancache_list;
@@ -1688,6 +1689,8 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr plan, ParamListInfo boundParams)
Node *parsetree = (Node *) lfirst(list_item);
List *stmt_list;
CachedPlanSource *plansource;
+ MemoryContext querytree_ctx;
+ MemoryContext oldctx;
/*
* Create the CachedPlanSource before we do parse analysis, since it
@@ -1695,12 +1698,25 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr plan, ParamListInfo boundParams)
*/
plansource = CreateCachedPlan(parsetree,
src,
- CreateCommandTag(parsetree));
+ CreateCommandTag(parsetree),
+ oneshot);
/*
* Parameter datatypes are driven by parserSetup hook if provided,
* otherwise we use the fixed parameter list.
*/
+ oldctx = CurrentMemoryContext;
+ if (oneshot)
+ {
+ querytree_ctx = AllocSetContextCreate(CurrentMemoryContext,
+ "Short-lived querytree context",
+ ALLOCSET_SMALL_MINSIZE,
+ ALLOCSET_SMALL_INITSIZE,
+ ALLOCSET_DEFAULT_MAXSIZE);
+ MemoryContextSwitchTo(querytree_ctx);
+ }
+ else
+ querytree_ctx = NULL;
if (plan->parserSetup != NULL)
{
Assert(plan->nargs == 0);
@@ -1717,10 +1733,13 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr plan, ParamListInfo boundParams)
plan->nargs);
}
+ if (oneshot)
+ MemoryContextSwitchTo(oldctx);
+
/* Finish filling in the CachedPlanSource */
CompleteCachedPlan(plansource,
stmt_list,
- NULL,
+ querytree_ctx,
plan->argtypes,
plan->nargs,
plan->parserSetup,
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index f1633f9..1eae238 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1260,7 +1260,7 @@ exec_parse_message(const char *query_string, /* string to execute */
* Create the CachedPlanSource before we do parse analysis, since it
* needs to see the unmodified raw parse tree.
*/
- psrc = CreateCachedPlan(raw_parse_tree, query_string, commandTag);
+ psrc = CreateCachedPlan(raw_parse_tree, query_string, commandTag, false);
/*
* Set up a snapshot if parse analysis will need one.
@@ -1312,7 +1312,7 @@ exec_parse_message(const char *query_string, /* string to execute */
/* Empty input string. This is legal. */
raw_parse_tree = NULL;
commandTag = NULL;
- psrc = CreateCachedPlan(raw_parse_tree, query_string, commandTag);
+ psrc = CreateCachedPlan(raw_parse_tree, query_string, commandTag, false);
querytree_list = NIL;
}
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index c42765c..4e5ff2b 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -134,7 +134,8 @@ InitPlanCache(void)
CachedPlanSource *
CreateCachedPlan(Node *raw_parse_tree,
const char *query_string,
- const char *commandTag)
+ const char *commandTag,
+ bool oneshot)
{
CachedPlanSource *plansource;
MemoryContext source_context;
@@ -183,6 +184,7 @@ CreateCachedPlan(Node *raw_parse_tree,
plansource->is_complete = false;
plansource->is_saved = false;
plansource->is_valid = false;
+ plansource->is_oneshot = oneshot;
plansource->generation = 0;
plansource->next_saved = NULL;
plansource->generic_cost = -1;
@@ -339,6 +341,9 @@ SaveCachedPlan(CachedPlanSource *plansource)
Assert(plansource->is_complete);
Assert(!plansource->is_saved);
+ if (plansource->is_oneshot)
+ elog(ERROR, "one-shot cached plans cannot be reused");
+
/*
* In typical use, this function would be called before generating any
* plans from the CachedPlanSource. If there is a generic plan, moving it
@@ -741,7 +746,27 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
* scribbled on by the planner, make one.
*/
if (qlist == NIL)
- qlist = (List *) copyObject(plansource->query_list);
+ {
+ /*
+ * If the caller said this is a one-shot query, we feel free to
+ * scribble on the query-tree list directly
+ */
+ if (plansource->is_oneshot)
+ qlist = plansource->query_list;
+ else
+ qlist = (List *) copyObject(plansource->query_list);
+ }
+
+ /*
+ * Make a dedicated memory context for the CachedPlan and its subsidiary
+ * data. It's probably not going to be large, but just in case, use the
+ * default maxsize parameter. It's transient for the moment.
+ */
+ plan_context = AllocSetContextCreate(CurrentMemoryContext,
+ "CachedPlan",
+ ALLOCSET_SMALL_MINSIZE,
+ ALLOCSET_SMALL_INITSIZE,
+ ALLOCSET_DEFAULT_MAXSIZE);
/*
* Restore the search_path that was in use when the plan was made. See
@@ -780,6 +805,16 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
/*
* Generate the plan.
*/
+
+ /*
+ * If this is a one-shot query, do the planning directly in the
+ * CachedPlan's context. It's going to be littered by the planning
+ * process, but we don't need to copy the result to the CachedPlan's
+ * context. That's a good tradeoff for a single-shot query, as the
+ * CachedPLan won't live long.
+ */
+ if (plansource->is_oneshot)
+ MemoryContextSwitchTo(plan_context);
plist = pg_plan_queries(qlist, plansource->cursor_options, boundParams);
/* Clean up SPI state */
@@ -792,23 +827,14 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
/* Now we can restore current search path */
PopOverrideSearchPath();
- /*
- * Make a dedicated memory context for the CachedPlan and its subsidiary
- * data. It's probably not going to be large, but just in case, use the
- * default maxsize parameter. It's transient for the moment.
- */
- plan_context = AllocSetContextCreate(CurrentMemoryContext,
- "CachedPlan",
- ALLOCSET_SMALL_MINSIZE,
- ALLOCSET_SMALL_INITSIZE,
- ALLOCSET_DEFAULT_MAXSIZE);
+ oldcxt = MemoryContextSwitchTo(plan_context);
/*
- * Copy plan into the new context.
+ * Copy plan into the new context, if we didn't create it there to begin
+ * with.
*/
- oldcxt = MemoryContextSwitchTo(plan_context);
-
- plist = (List *) copyObject(plist);
+ if (!plansource->is_oneshot)
+ plist = (List *) copyObject(plist);
/*
* Create and fill the CachedPlan struct within the new context.
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 413e846..b6eed19 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -91,6 +91,7 @@ typedef struct CachedPlanSource
bool is_complete; /* has CompleteCachedPlan been done? */
bool is_saved; /* has CachedPlanSource been "saved"? */
bool is_valid; /* is the query_list currently valid? */
+ bool is_oneshot;
int generation; /* increments each time we create a plan */
/* If CachedPlanSource has been saved, it is a member of a global list */
struct CachedPlanSource *next_saved; /* list link, if so */
@@ -128,7 +129,8 @@ extern void ResetPlanCache(void);
extern CachedPlanSource *CreateCachedPlan(Node *raw_parse_tree,
const char *query_string,
- const char *commandTag);
+ const char *commandTag,
+ bool is_oneshot);
extern void CompleteCachedPlan(CachedPlanSource *plansource,
List *querytree_list,
MemoryContext querytree_context,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
