Speed up plpgsql function startup by doing fewer pallocs.

Previously, copy_plpgsql_datum did a separate palloc for each variable
needing instance-local storage.  In simple benchmarks this made for a
noticeable fraction of the total runtime.  Improve it by precalculating
the space needed for all of a function's variables and doing just one
palloc for all of them.

In passing, remove PLPGSQL_DTYPE_EXPR from the list of plpgsql "datum"
types, since in fact it has nothing in common with the others, and there
is noplace that needs to discriminate on the basis of dtype between an
expression and any type of datum.  And add comments clarifying which
datum struct fields are generic and which aren't.

Tom Lane, reviewed by Pavel Stehule

Discussion: https://postgr.es/m/11986.1514407114@sss.pgh.pa.us

4 files changed, 130 insertions, 86 deletions

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 09ecaec6354..97cb7636413 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -2183,12 +2183,29 @@ plpgsql_adddatum(PLpgSQL_datum *new)
 static void
 plpgsql_finish_datums(PLpgSQL_function *function)
 {
+	Size		copiable_size = 0;
 	int			i;
 
 	function->ndatums = plpgsql_nDatums;
 	function->datums = palloc(sizeof(PLpgSQL_datum *) * plpgsql_nDatums);
 	for (i = 0; i < plpgsql_nDatums; i++)
+	{
 		function->datums[i] = plpgsql_Datums[i];
+
+		/* This must agree with copy_plpgsql_datums on what is copiable */
+		switch (function->datums[i]->dtype)
+		{
+			case PLPGSQL_DTYPE_VAR:
+				copiable_size += MAXALIGN(sizeof(PLpgSQL_var));
+				break;
+			case PLPGSQL_DTYPE_REC:
+				copiable_size += MAXALIGN(sizeof(PLpgSQL_rec));
+				break;
+			default:
+				break;
+		}
+	}
+	function->copiable_size = copiable_size;
 }
 
 
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 7612902e8fb..c90024064a0 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -235,7 +235,8 @@ static HTAB *shared_cast_hash = NULL;
 static void coerce_function_result_tuple(PLpgSQL_execstate *estate,
 							 TupleDesc tupdesc);
 static void plpgsql_exec_error_callback(void *arg);
-static PLpgSQL_datum *copy_plpgsql_datum(PLpgSQL_datum *datum);
+static void copy_plpgsql_datums(PLpgSQL_execstate *estate,
+					PLpgSQL_function *func);
 static MemoryContext get_stmt_mcontext(PLpgSQL_execstate *estate);
 static void push_stmt_mcontext(PLpgSQL_execstate *estate);
 static void pop_stmt_mcontext(PLpgSQL_execstate *estate);
@@ -458,8 +459,7 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
 	 * Make local execution copies of all the datums
 	 */
 	estate.err_text = gettext_noop("during initialization of execution state");
-	for (i = 0; i < estate.ndatums; i++)
-		estate.datums[i] = copy_plpgsql_datum(func->datums[i]);
+	copy_plpgsql_datums(&estate, func);
 
 	/*
 	 * Store the actual call argument values into the appropriate variables
@@ -859,8 +859,7 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
 	 * Make local execution copies of all the datums
 	 */
 	estate.err_text = gettext_noop("during initialization of execution state");
-	for (i = 0; i < estate.ndatums; i++)
-		estate.datums[i] = copy_plpgsql_datum(func->datums[i]);
+	copy_plpgsql_datums(&estate, func);
 
 	/*
 	 * Put the OLD and NEW tuples into record variables
@@ -1153,7 +1152,6 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, EventTriggerData *trigdata)
 {
 	PLpgSQL_execstate estate;
 	ErrorContextCallback plerrcontext;
-	int			i;
 	int			rc;
 	PLpgSQL_var *var;
 
@@ -1174,8 +1172,7 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, EventTriggerData *trigdata)
 	 * Make local execution copies of all the datums
 	 */
 	estate.err_text = gettext_noop("during initialization of execution state");
-	for (i = 0; i < estate.ndatums; i++)
-		estate.datums[i] = copy_plpgsql_datum(func->datums[i]);
+	copy_plpgsql_datums(&estate, func);
 
 	/*
 	 * Assign the special tg_ variables
@@ -1290,57 +1287,73 @@ plpgsql_exec_error_callback(void *arg)
  * Support function for initializing local execution variables
  * ----------
  */
-static PLpgSQL_datum *
-copy_plpgsql_datum(PLpgSQL_datum *datum)
+static void
+copy_plpgsql_datums(PLpgSQL_execstate *estate,
+					PLpgSQL_function *func)
 {
-	PLpgSQL_datum *result;
+	int			ndatums = estate->ndatums;
+	PLpgSQL_datum **indatums;
+	PLpgSQL_datum **outdatums;
+	char	   *workspace;
+	char	   *ws_next;
+	int			i;
 
-	switch (datum->dtype)
-	{
-		case PLPGSQL_DTYPE_VAR:
-			{
-				PLpgSQL_var *new = palloc(sizeof(PLpgSQL_var));
+	/* Allocate local datum-pointer array */
+	estate->datums = (PLpgSQL_datum **)
+		palloc(sizeof(PLpgSQL_datum *) * ndatums);
 
-				memcpy(new, datum, sizeof(PLpgSQL_var));
-				/* should be preset to null/non-freeable */
-				Assert(new->isnull);
-				Assert(!new->freeval);
+	/*
+	 * To reduce palloc overhead, we make a single palloc request for all the
+	 * space needed for locally-instantiated datums.
+	 */
+	workspace = palloc(func->copiable_size);
+	ws_next = workspace;
 
-				result = (PLpgSQL_datum *) new;
-			}
-			break;
+	/* Fill datum-pointer array, copying datums into workspace as needed */
+	indatums = func->datums;
+	outdatums = estate->datums;
+	for (i = 0; i < ndatums; i++)
+	{
+		PLpgSQL_datum *indatum = indatums[i];
+		PLpgSQL_datum *outdatum;
 
-		case PLPGSQL_DTYPE_REC:
-			{
-				PLpgSQL_rec *new = palloc(sizeof(PLpgSQL_rec));
+		/* This must agree with plpgsql_finish_datums on what is copiable */
+		switch (indatum->dtype)
+		{
+			case PLPGSQL_DTYPE_VAR:
+				outdatum = (PLpgSQL_datum *) ws_next;
+				memcpy(outdatum, indatum, sizeof(PLpgSQL_var));
+				ws_next += MAXALIGN(sizeof(PLpgSQL_var));
+				break;
 
-				memcpy(new, datum, sizeof(PLpgSQL_rec));
-				/* should be preset to empty */
-				Assert(new->erh == NULL);
+			case PLPGSQL_DTYPE_REC:
+				outdatum = (PLpgSQL_datum *) ws_next;
+				memcpy(outdatum, indatum, sizeof(PLpgSQL_rec));
+				ws_next += MAXALIGN(sizeof(PLpgSQL_rec));
+				break;
 
-				result = (PLpgSQL_datum *) new;
-			}
-			break;
+			case PLPGSQL_DTYPE_ROW:
+			case PLPGSQL_DTYPE_RECFIELD:
+			case PLPGSQL_DTYPE_ARRAYELEM:
 
-		case PLPGSQL_DTYPE_ROW:
-		case PLPGSQL_DTYPE_RECFIELD:
-		case PLPGSQL_DTYPE_ARRAYELEM:
+				/*
+				 * These datum records are read-only at runtime, so no need to
+				 * copy them (well, RECFIELD and ARRAYELEM contain cached
+				 * data, but we'd just as soon centralize the caching anyway).
+				 */
+				outdatum = indatum;
+				break;
 
-			/*
-			 * These datum records are read-only at runtime, so no need to
-			 * copy them (well, RECFIELD and ARRAYELEM contain cached data,
-			 * but we'd just as soon centralize the caching anyway)
-			 */
-			result = datum;
-			break;
+			default:
+				elog(ERROR, "unrecognized dtype: %d", indatum->dtype);
+				outdatum = NULL;	/* keep compiler quiet */
+				break;
+		}
 
-		default:
-			elog(ERROR, "unrecognized dtype: %d", datum->dtype);
-			result = NULL;		/* keep compiler quiet */
-			break;
+		outdatums[i] = outdatum;
 	}
 
-	return result;
+	Assert(ws_next == workspace + func->copiable_size);
 }
 
 /*
@@ -3504,8 +3517,8 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
 
 	estate->found_varno = func->found_varno;
 	estate->ndatums = func->ndatums;
-	estate->datums = palloc(sizeof(PLpgSQL_datum *) * estate->ndatums);
-	/* caller is expected to fill the datums array */
+	estate->datums = NULL;
+	/* the datums array will be filled by copy_plpgsql_datums() */
 	estate->datum_context = CurrentMemoryContext;
 
 	/* initialize our ParamListInfo with appropriate hook functions */
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 97c0d4f98ac..ee943eed935 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -564,7 +564,6 @@ decl_statement	: decl_varname decl_const decl_datatype decl_collate decl_notnull
 
 						curname_def = palloc0(sizeof(PLpgSQL_expr));
 
-						curname_def->dtype = PLPGSQL_DTYPE_EXPR;
 						strcpy(buf, "SELECT ");
 						cp1 = new->refname;
 						cp2 = buf + strlen(buf);
@@ -2697,7 +2696,6 @@ read_sql_construct(int until,
 	}
 
 	expr = palloc0(sizeof(PLpgSQL_expr));
-	expr->dtype			= PLPGSQL_DTYPE_EXPR;
 	expr->query			= pstrdup(ds.data);
 	expr->plan			= NULL;
 	expr->paramnos		= NULL;
@@ -2944,7 +2942,6 @@ make_execsql_stmt(int firsttoken, int location)
 		ds.data[--ds.len] = '\0';
 
 	expr = palloc0(sizeof(PLpgSQL_expr));
-	expr->dtype			= PLPGSQL_DTYPE_EXPR;
 	expr->query			= pstrdup(ds.data);
 	expr->plan			= NULL;
 	expr->paramnos		= NULL;
@@ -3816,7 +3813,6 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 	appendStringInfoChar(&ds, ';');
 
 	expr = palloc0(sizeof(PLpgSQL_expr));
-	expr->dtype			= PLPGSQL_DTYPE_EXPR;
 	expr->query			= pstrdup(ds.data);
 	expr->plan			= NULL;
 	expr->paramnos		= NULL;
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index d4eb67b738a..dadbfb569c4 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -63,8 +63,7 @@ typedef enum PLpgSQL_datum_type
 	PLPGSQL_DTYPE_ROW,
 	PLPGSQL_DTYPE_REC,
 	PLPGSQL_DTYPE_RECFIELD,
-	PLPGSQL_DTYPE_ARRAYELEM,
-	PLPGSQL_DTYPE_EXPR
+	PLPGSQL_DTYPE_ARRAYELEM
 } PLpgSQL_datum_type;
 
 /*
@@ -189,38 +188,10 @@ typedef struct PLpgSQL_type
 } PLpgSQL_type;
 
 /*
- * Generic datum array item
- *
- * PLpgSQL_datum is the common supertype for PLpgSQL_expr, PLpgSQL_var,
- * PLpgSQL_row, PLpgSQL_rec, PLpgSQL_recfield, and PLpgSQL_arrayelem
- */
-typedef struct PLpgSQL_datum
-{
-	PLpgSQL_datum_type dtype;
-	int			dno;
-} PLpgSQL_datum;
-
-/*
- * Scalar or composite variable
- *
- * The variants PLpgSQL_var, PLpgSQL_row, and PLpgSQL_rec share these
- * fields
- */
-typedef struct PLpgSQL_variable
-{
-	PLpgSQL_datum_type dtype;
-	int			dno;
-	char	   *refname;
-	int			lineno;
-} PLpgSQL_variable;
-
-/*
  * SQL Query to plan and execute
  */
 typedef struct PLpgSQL_expr
 {
-	PLpgSQL_datum_type dtype;
-	int			dno;
 	char	   *query;
 	SPIPlanPtr	plan;
 	Bitmapset  *paramnos;		/* all dnos referenced by this query */
@@ -250,6 +221,32 @@ typedef struct PLpgSQL_expr
 } PLpgSQL_expr;
 
 /*
+ * Generic datum array item
+ *
+ * PLpgSQL_datum is the common supertype for PLpgSQL_var, PLpgSQL_row,
+ * PLpgSQL_rec, PLpgSQL_recfield, and PLpgSQL_arrayelem.
+ */
+typedef struct PLpgSQL_datum
+{
+	PLpgSQL_datum_type dtype;
+	int			dno;
+} PLpgSQL_datum;
+
+/*
+ * Scalar or composite variable
+ *
+ * The variants PLpgSQL_var, PLpgSQL_row, and PLpgSQL_rec share these
+ * fields.
+ */
+typedef struct PLpgSQL_variable
+{
+	PLpgSQL_datum_type dtype;
+	int			dno;
+	char	   *refname;
+	int			lineno;
+} PLpgSQL_variable;
+
+/*
  * Scalar variable
  */
 typedef struct PLpgSQL_var
@@ -258,11 +255,18 @@ typedef struct PLpgSQL_var
 	int			dno;
 	char	   *refname;
 	int			lineno;
+	/* end of PLpgSQL_variable fields */
 
+	bool		isconst;
+	bool		notnull;
 	PLpgSQL_type *datatype;
-	int			isconst;
-	int			notnull;
 	PLpgSQL_expr *default_val;
+
+	/*
+	 * Variables declared as CURSOR FOR <query> are mostly like ordinary
+	 * scalar variables of type refcursor, but they have these additional
+	 * properties:
+	 */
 	PLpgSQL_expr *cursor_explicit_expr;
 	int			cursor_explicit_argrow;
 	int			cursor_options;
@@ -286,6 +290,7 @@ typedef struct PLpgSQL_row
 	int			dno;
 	char	   *refname;
 	int			lineno;
+	/* end of PLpgSQL_variable fields */
 
 	/*
 	 * rowtupdesc is only set up if we might need to convert the row into a
@@ -308,6 +313,8 @@ typedef struct PLpgSQL_rec
 	int			dno;
 	char	   *refname;
 	int			lineno;
+	/* end of PLpgSQL_variable fields */
+
 	Oid			rectypeid;		/* declared type of variable */
 	/* RECFIELDs for this record are chained together for easy access */
 	int			firstfield;		/* dno of first RECFIELD, or -1 if none */
@@ -322,6 +329,8 @@ typedef struct PLpgSQL_recfield
 {
 	PLpgSQL_datum_type dtype;
 	int			dno;
+	/* end of PLpgSQL_datum fields */
+
 	char	   *fieldname;		/* name of field */
 	int			recparentno;	/* dno of parent record */
 	int			nextfield;		/* dno of next child, or -1 if none */
@@ -337,6 +346,8 @@ typedef struct PLpgSQL_arrayelem
 {
 	PLpgSQL_datum_type dtype;
 	int			dno;
+	/* end of PLpgSQL_datum fields */
+
 	PLpgSQL_expr *subscript;
 	int			arrayparentno;	/* dno of parent array variable */
 
@@ -884,6 +895,7 @@ typedef struct PLpgSQL_function
 	/* the datums representing the function's local variables */
 	int			ndatums;
 	PLpgSQL_datum **datums;
+	Size		copiable_size;	/* space for locally instantiated datums */
 
 	/* function body parsetree */
 	PLpgSQL_stmt_block *action;
@@ -920,8 +932,14 @@ typedef struct PLpgSQL_execstate
 	ResourceOwner tuple_store_owner;
 	ReturnSetInfo *rsi;
 
-	/* the datums representing the function's local variables */
 	int			found_varno;
+
+	/*
+	 * The datums representing the function's local variables.  Some of these
+	 * are local storage in this execstate, but some just point to the shared
+	 * copy belonging to the PLpgSQL_function, depending on whether or not we
+	 * need any per-execution state for the datum's dtype.
+	 */
 	int			ndatums;
 	PLpgSQL_datum **datums;
 	/* context containing variable values (same as func's SPI_proc context) */