On 27.03.2011 19:51, Tom Lane wrote:
Fix plpgsql to release SPI plans when a function or DO block is freed.
This fixes the gripe I made a few months ago about DO blocks getting
slower with repeated use. At least, it fixes it for the case where
the DO block isn't aborted by an error. We could try running
plpgsql_free_function_memory() even during error exit, but that seems
a bit scary since it makes a lot of presumptions about the data
structures being in good shape. It's probably reasonable to assume
that repeated failures of DO blocks isn't a performance-critical case.
I was quite surprised by the way you did this. Instead of adding all
that code to traverse the PLpgSQL_stmt tree (that we'll have to remember
to keep up-to-date), can't we just have a list of cached plans in
PLpgSQL_function? As attached.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index a928e2f..b2139d6 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -2368,12 +2368,9 @@ delete_function(PLpgSQL_function *func)
/* remove function from hash table (might be done already) */
plpgsql_HashTableDelete(func);
- /* release the function's storage if safe and not done already */
- if (func->use_count == 0 && func->fn_cxt)
- {
- MemoryContextDelete(func->fn_cxt);
- func->fn_cxt = NULL;
- }
+ /* release the function's storage if safe */
+ if (func->use_count)
+ plpgsql_free_function_memory(func);
}
/* exported so we can call it from plpgsql_init() */
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 1f4d5ac..e353374 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2919,6 +2919,7 @@ static void
exec_prepare_plan(PLpgSQL_execstate *estate,
PLpgSQL_expr *expr, int cursorOptions)
{
+ MemoryContext oldcxt;
SPIPlanPtr plan;
/*
@@ -2956,6 +2957,11 @@ exec_prepare_plan(PLpgSQL_execstate *estate,
expr->plan = SPI_saveplan(plan);
SPI_freeplan(plan);
exec_simple_check_plan(expr);
+
+ /* Remember this plan so that it can be freed along with the function */
+ oldcxt = MemoryContextSwitchTo(expr->func->fn_cxt);
+ expr->func->cached_plans = lappend(expr->func->cached_plans, expr->plan);
+ MemoryContextSwitchTo(oldcxt);
}
diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index f13e4c3..14c5e08 100644
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -15,6 +15,8 @@
#include "plpgsql.h"
+#include "utils/memutils.h"
+
/* ----------
* Local variables for namespace handling
@@ -264,6 +266,37 @@ plpgsql_stmt_typename(PLpgSQL_stmt *stmt)
}
+/*
+ * Release memory when a PL/pgSQL function is no longer needed
+ */
+void
+plpgsql_free_function_memory(PLpgSQL_function *func)
+{
+ ListCell *lc;
+
+ /* Better not call this on an in-use function */
+ Assert(func->use_count == 0);
+
+ /* Release cached plans belonging to the function */
+ foreach(lc, func->cached_plans)
+ {
+ SPI_freeplan(lfirst(lc));
+ }
+ list_free(func->cached_plans);
+ func->cached_plans = NIL;
+
+ /*
+ * And finally, release all memory except the PLpgSQL_function struct
+ * itself (which has to be kept around because there may be multiple
+ * fn_extra pointers to it).
+ */
+ if (func->fn_cxt)
+ {
+ MemoryContextDelete(func->fn_cxt);
+ func->fn_cxt = NULL;
+ }
+}
+
/**********************************************************************
* Debug functions for analyzing the compiled code
**********************************************************************/
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index 8f7080e..2389849 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -172,6 +172,9 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS)
/* Compile the anonymous code block */
func = plpgsql_compile_inline(codeblock->source_text);
+ /* Mark the function as busy, just pro forma */
+ func->use_count++;
+
/*
* Set up a fake fcinfo with just enough info to satisfy
* plpgsql_exec_function(). In particular note that this sets things up
@@ -185,6 +188,13 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS)
retval = plpgsql_exec_function(func, &fake_fcinfo);
+ /* Function should now have no remaining use-counts ... */
+ func->use_count--;
+ Assert(func->use_count == 0);
+
+ /* ... so we can free subsidiary storage */
+ plpgsql_free_function_memory(func);
+
/*
* Disconnect from SPI manager
*/
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 7015379..9efa982 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -693,6 +693,8 @@ typedef struct PLpgSQL_function
/* these fields change when the function is used */
struct PLpgSQL_execstate *cur_estate;
unsigned long use_count;
+
+ List *cached_plans;
} PLpgSQL_function;
@@ -918,6 +920,7 @@ extern PLpgSQL_nsitem *plpgsql_ns_lookup_label(PLpgSQL_nsitem *ns_cur,
* ----------
*/
extern const char *plpgsql_stmt_typename(PLpgSQL_stmt *stmt);
+extern void plpgsql_free_function_memory(PLpgSQL_function *func);
extern void plpgsql_dumptree(PLpgSQL_function *func);
/* ----------
--
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
