On Fri, 17 Jun 2022 at 11:31, Andres Freund <and...@anarazel.de> wrote:
> hashedscalararrayop/EEOP_HASHED_SCALARARRAYOP is 64 bytes, even though the
> limit is 40 bytes.
> commit 50e17ad281b8d1c1b410c9833955bc80fbad4078
> Author: David Rowley <drow...@postgresql.org>
> Date: 2021-04-08 23:51:22 +1200
>
> Speedup ScalarArrayOpExpr evaluation
I've put together the attached patch which removes 4 fields from the
hashedscalararrayop portion of the struct which, once the JSON part is
fixed, will put sizeof(ExprEvalStep) back down to 64 bytes again.
The attached patch causes some extra pointer dereferencing to perform
a hashed saop step, so I tested the performance on f4fb45d15 (prior to
the JSON patch that pushed the sizeof(ExprEvalStep) up further. I
found:
setup:
create table a (a int);
insert into a select x from generate_series(1000000,2000000) x;
bench.sql
select * from a where a in(1,2,3,4,5,6,7,8,9,10);
f4fb45d15 + reduce_sizeof_hashedsaop_ExprEvalStep.patch
drowley@amd3990x:~$ pgbench -n -f bench.sql -T 60 -M prepared postgres
tps = 44.841851 (without initial connection time)
tps = 44.986472 (without initial connection time)
tps = 44.944315 (without initial connection time)
f4fb45d15
drowley@amd3990x:~$ pgbench -n -f bench.sql -T 60 -M prepared postgres
tps = 44.446127 (without initial connection time)
tps = 44.614134 (without initial connection time)
tps = 44.895011 (without initial connection time)
(Patched is ~0.61% faster here)
So, there appears to be no performance regression due to the extra
indirection. There's maybe even some gains due to the smaller step
size.
David
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 2831e7978b..82bf47ddb9 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -1270,10 +1270,6 @@ ExecInitExprRec(Expr *node, ExprState *state,
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);
@@ -1290,13 +1286,8 @@ ExecInitExprRec(Expr *node, ExprState *state,
/* And perform the operation */
scratch.opcode =
EEOP_HASHED_SCALARARRAYOP;
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.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..c8dc859e1d 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -3480,7 +3480,7 @@ saop_element_hash(struct saophash_hash *tb, Datum key)
fcinfo->args[0].value = key;
fcinfo->args[0].isnull = false;
- hash = elements_tab->op->d.hashedscalararrayop.hash_fn_addr(fcinfo);
+ hash =
elements_tab->op->d.hashedscalararrayop.hash_fcinfo_data->flinfo->fn_addr(fcinfo);
return DatumGetUInt32(hash);
}
@@ -3502,7 +3502,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.fcinfo_data->flinfo->fn_addr(fcinfo);
return DatumGetBool(result);
}
@@ -3526,7 +3526,7 @@ ExecEvalHashedScalarArrayOp(ExprState *state,
ExprEvalStep *op, ExprContext *eco
ScalarArrayOpExprHashTable *elements_tab =
op->d.hashedscalararrayop.elements_tab;
FunctionCallInfo fcinfo = op->d.hashedscalararrayop.fcinfo_data;
bool inclause = op->d.hashedscalararrayop.inclause;
- bool strictfunc = op->d.hashedscalararrayop.finfo->fn_strict;
+ bool strictfunc =
op->d.hashedscalararrayop.fcinfo_data->flinfo->fn_strict;
Datum scalar = fcinfo->args[0].value;
bool scalar_isnull = fcinfo->args[0].isnull;
Datum result;
@@ -3669,7 +3669,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.fcinfo_data->flinfo->fn_addr(fcinfo);
resultnull = fcinfo->isnull;
/*
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index e34db8c93c..4bb6d34275 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -580,14 +580,8 @@ typedef struct ExprEvalStep
bool has_nulls;
bool inclause; /* true for IN and
false for NOT IN */
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
*/
} hashedscalararrayop;
/* for EEOP_XMLEXPR */
