v3-0001-Remove-size-increase-in-ExprEvalStep-caused-by-ha.patch

text/plain

Filename: v3-0001-Remove-size-increase-in-ExprEvalStep-caused-by-ha.patch
Type: text/plain
Part: 0
Message: Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size

Patch

Format: format-patch
Series: patch v3-0001
Subject: Remove size increase in ExprEvalStep caused by hashed saops
File+
src/backend/executor/execExpr.c 1 18
src/backend/executor/execExprInterp.c 21 4
src/include/executor/execExpr.h 1 6
From 7a1e71a6b622eff72537ae86187b312d01a5e11d Mon Sep 17 00:00:00 2001
From: David Rowley <drowley@postgresql.org>
Date: Wed, 29 Jun 2022 10:43:41 +1200
Subject: [PATCH v3] Remove size increase in ExprEvalStep caused by hashed
 saops

50e17ad28 increased the size of ExprEvalStep from 64 bytes up to 88 bytes.
Lots of effort was spent during the development of the current expression
evaluation code to make an instance of this struct fit on a single cache
line.

Here we remove the fn_addr field. The values from this field can be
obtained via fcinfo, just with some extra pointer dereferencing.  The
extra indirection does not seem to cause any noticeable slowdowns.

Various other fields have been been moved into the
ScalarArrayOpExprHashTable struct. These fields are only used when the
ScalarArrayOpExprHashTable pointer has already been dereferenced, so no
additional pointer dereferences occur for these.

Author: Andres Freund
Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de
---
 src/backend/executor/execExpr.c       | 19 +------------------
 src/backend/executor/execExprInterp.c | 25 +++++++++++++++++++++----
 src/include/executor/execExpr.h       |  7 +------
 3 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 2831e7978b..2e75f3204d 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -1203,8 +1203,6 @@ ExecInitExprRec(Expr *node, ExprState *state,
 				FmgrInfo   *finfo;
 				FunctionCallInfo fcinfo;
 				AclResult	aclresult;
-				FmgrInfo   *hash_finfo;
-				FunctionCallInfo hash_fcinfo;
 				Oid			cmpfuncid;
 
 				/*
@@ -1262,18 +1260,6 @@ ExecInitExprRec(Expr *node, ExprState *state,
 				 */
 				if (OidIsValid(opexpr->hashfuncid))
 				{
-					hash_finfo = palloc0(sizeof(FmgrInfo));
-					hash_fcinfo = palloc0(SizeForFunctionCallInfo(1));
-					fmgr_info(opexpr->hashfuncid, hash_finfo);
-					fmgr_info_set_expr((Node *) node, hash_finfo);
-					InitFunctionCallInfoData(*hash_fcinfo, hash_finfo,
-											 1, opexpr->inputcollid, NULL,
-											 NULL);
-
-					scratch.d.hashedscalararrayop.hash_finfo = hash_finfo;
-					scratch.d.hashedscalararrayop.hash_fcinfo_data = hash_fcinfo;
-					scratch.d.hashedscalararrayop.hash_fn_addr = hash_finfo->fn_addr;
-
 					/* Evaluate scalar directly into left function argument */
 					ExecInitExprRec(scalararg, state,
 									&fcinfo->args[0].value, &fcinfo->args[0].isnull);
@@ -1292,11 +1278,8 @@ ExecInitExprRec(Expr *node, ExprState *state,
 					scratch.d.hashedscalararrayop.inclause = opexpr->useOr;
 					scratch.d.hashedscalararrayop.finfo = finfo;
 					scratch.d.hashedscalararrayop.fcinfo_data = fcinfo;
-					scratch.d.hashedscalararrayop.fn_addr = finfo->fn_addr;
+					scratch.d.hashedscalararrayop.saop = opexpr;
 
-					scratch.d.hashedscalararrayop.hash_finfo = hash_finfo;
-					scratch.d.hashedscalararrayop.hash_fcinfo_data = hash_fcinfo;
-					scratch.d.hashedscalararrayop.hash_fn_addr = hash_finfo->fn_addr;
 
 					ExprEvalPushStep(state, &scratch);
 				}
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index eaec697bb3..399bdfd861 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -217,6 +217,8 @@ typedef struct ScalarArrayOpExprHashTable
 {
 	saophash_hash *hashtab;		/* underlying hash table */
 	struct ExprEvalStep *op;
+	FunctionCallInfo hash_fcinfo_data;	/* arguments etc */
+	FmgrInfo   *hash_finfo;		/* function's lookup data */
 } ScalarArrayOpExprHashTable;
 
 /* Define parameters for ScalarArrayOpExpr hash table code generation. */
@@ -3474,13 +3476,13 @@ static uint32
 saop_element_hash(struct saophash_hash *tb, Datum key)
 {
 	ScalarArrayOpExprHashTable *elements_tab = (ScalarArrayOpExprHashTable *) tb->private_data;
-	FunctionCallInfo fcinfo = elements_tab->op->d.hashedscalararrayop.hash_fcinfo_data;
+	FunctionCallInfo fcinfo = elements_tab->hash_fcinfo_data;
 	Datum		hash;
 
 	fcinfo->args[0].value = key;
 	fcinfo->args[0].isnull = false;
 
-	hash = elements_tab->op->d.hashedscalararrayop.hash_fn_addr(fcinfo);
+	hash = elements_tab->hash_finfo->fn_addr(fcinfo);
 
 	return DatumGetUInt32(hash);
 }
@@ -3502,7 +3504,7 @@ saop_hash_element_match(struct saophash_hash *tb, Datum key1, Datum key2)
 	fcinfo->args[1].value = key2;
 	fcinfo->args[1].isnull = false;
 
-	result = elements_tab->op->d.hashedscalararrayop.fn_addr(fcinfo);
+	result = elements_tab->op->d.hashedscalararrayop.finfo->fn_addr(fcinfo);
 
 	return DatumGetBool(result);
 }
@@ -3549,6 +3551,7 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco
 	/* Build the hash table on first evaluation */
 	if (elements_tab == NULL)
 	{
+		ScalarArrayOpExpr *saop;
 		int16		typlen;
 		bool		typbyval;
 		char		typalign;
@@ -3560,6 +3563,8 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco
 		MemoryContext oldcontext;
 		ArrayType  *arr;
 
+		saop = op->d.hashedscalararrayop.saop;
+
 		arr = DatumGetArrayTypeP(*op->resvalue);
 		nitems = ArrayGetNItems(ARR_NDIM(arr), ARR_DIMS(arr));
 
@@ -3575,6 +3580,18 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco
 		op->d.hashedscalararrayop.elements_tab = elements_tab;
 		elements_tab->op = op;
 
+		elements_tab->hash_finfo = palloc0(sizeof(FmgrInfo));
+		fmgr_info(saop->hashfuncid, elements_tab->hash_finfo);
+		fmgr_info_set_expr((Node *) saop, elements_tab->hash_finfo);
+
+		elements_tab->hash_fcinfo_data = palloc0(SizeForFunctionCallInfo(1));
+		InitFunctionCallInfoData(*elements_tab->hash_fcinfo_data,
+								 elements_tab->hash_finfo,
+								 1,
+								 saop->inputcollid,
+								 NULL,
+								 NULL);
+
 		/*
 		 * Create the hash table sizing it according to the number of elements
 		 * in the array.  This does assume that the array has no duplicates.
@@ -3669,7 +3686,7 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco
 			fcinfo->args[1].value = (Datum) 0;
 			fcinfo->args[1].isnull = true;
 
-			result = op->d.hashedscalararrayop.fn_addr(fcinfo);
+			result = op->d.hashedscalararrayop.finfo->fn_addr(fcinfo);
 			resultnull = fcinfo->isnull;
 
 			/*
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index e34db8c93c..e5e4d7fc1f 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -582,12 +582,7 @@ typedef struct ExprEvalStep
 			struct ScalarArrayOpExprHashTable *elements_tab;
 			FmgrInfo   *finfo;	/* function's lookup data */
 			FunctionCallInfo fcinfo_data;	/* arguments etc */
-			/* faster to access without additional indirection: */
-			PGFunction	fn_addr;	/* actual call address */
-			FmgrInfo   *hash_finfo; /* function's lookup data */
-			FunctionCallInfo hash_fcinfo_data;	/* arguments etc */
-			/* faster to access without additional indirection: */
-			PGFunction	hash_fn_addr;	/* actual call address */
+			ScalarArrayOpExpr *saop;
 		}			hashedscalararrayop;
 
 		/* for EEOP_XMLEXPR */
-- 
2.34.1