Re: [HACKERS] Arrays of domains - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: [HACKERS] Arrays of domains |
Date | |
Msg-id | 8355.1506705035@sss.pgh.pa.us Whole thread Raw |
In response to | Re: [HACKERS] Arrays of domains (Andrew Dunstan <andrew.dunstan@2ndquadrant.com>) |
Responses |
Re: [HACKERS] Arrays of domains
Re: [HACKERS] Arrays of domains |
List | pgsql-hackers |
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: > On 09/28/2017 05:44 PM, Tom Lane wrote: >> Assuming that that's going to happen for v11, I'm inclined to leave the >> optimization problem until the dust settles around CaseTestExpr. >> Does anyone feel that this can't be committed before that's addressed? > I'm Ok with it as long as we don't forget to revisit this. I decided to go ahead and build a quick optimization for this case, as per the attached patch that applies on top of what we previously discussed. It brings us back to almost par with HEAD: HEAD Patch + 04.patch Q1 5453.235 ms 5440.876 ms 5407.965 ms Q2 9340.670 ms 10191.194 ms 9407.093 ms Q3 19078.457 ms 18967.279 ms 19050.392 ms Q4 48196.338 ms 58547.531 ms 48696.809 ms Unless Andres feels this is too ugly to live, I'm inclined to commit the patch with this addition. If we don't get around to revisiting CaseTestExpr, I think this is OK, and if we do, this will make sure that we consider this case in the revisit. It's probably also worth pointing out that this test case is intentionally chosen to be about the worst possible case for the patch. A less-trivial coercion function would make the overhead proportionally less meaningful. There's also the point that the old code sometimes applies two layers of array coercion rather than one. As an example, coercing int4[] to varchar(10)[] will do that. If I replace "x::int8[]" with "x::varchar(10)[]" in Q2 and Q4 in this test, I get HEAD Patch (+04) Q2 46929.728 ms 20646.003 ms Q4 386200.286 ms 155917.572 ms regards, tom lane diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index e0a8998..c5e97ef 100644 *** a/src/backend/executor/execExprInterp.c --- b/src/backend/executor/execExprInterp.c *************** *** 34,43 **** * * For very simple instructions the overhead of the full interpreter * "startup", as minimal as it is, is noticeable. Therefore ! * ExecReadyInterpretedExpr will choose to implement simple scalar Var ! * and Const expressions using special fast-path routines (ExecJust*). ! * Benchmarking shows anything more complex than those may as well use the ! * "full interpreter". * * Complex or uncommon instructions are not implemented in-line in * ExecInterpExpr(), rather we call out to a helper function appearing later --- 34,41 ---- * * For very simple instructions the overhead of the full interpreter * "startup", as minimal as it is, is noticeable. Therefore ! * ExecReadyInterpretedExpr will choose to implement certain simple ! * opcode patterns using special fast-path routines (ExecJust*). * * Complex or uncommon instructions are not implemented in-line in * ExecInterpExpr(), rather we call out to a helper function appearing later *************** static Datum ExecJustConst(ExprState *st *** 149,154 **** --- 147,153 ---- static Datum ExecJustAssignInnerVar(ExprState *state, ExprContext *econtext, bool *isnull); static Datum ExecJustAssignOuterVar(ExprState *state, ExprContext *econtext, bool *isnull); static Datum ExecJustAssignScanVar(ExprState *state, ExprContext *econtext, bool *isnull); + static Datum ExecJustApplyFuncToCase(ExprState *state, ExprContext *econtext, bool *isnull); /* *************** ExecReadyInterpretedExpr(ExprState *stat *** 184,193 **** /* * Select fast-path evalfuncs for very simple expressions. "Starting up" ! * the full interpreter is a measurable overhead for these. Plain Vars ! * and Const seem to be the only ones where the intrinsic cost is small ! * enough that the overhead of ExecInterpExpr matters. For more complex ! * expressions it's cheaper to use ExecInterpExpr always. */ if (state->steps_len == 3) { --- 183,190 ---- /* * Select fast-path evalfuncs for very simple expressions. "Starting up" ! * the full interpreter is a measurable overhead for these, and these ! * patterns occur often enough to be worth optimizing. */ if (state->steps_len == 3) { *************** ExecReadyInterpretedExpr(ExprState *stat *** 230,235 **** --- 227,239 ---- state->evalfunc = ExecJustAssignScanVar; return; } + else if (step0 == EEOP_CASE_TESTVAL && + step1 == EEOP_FUNCEXPR_STRICT && + state->steps[0].d.casetest.value) + { + state->evalfunc = ExecJustApplyFuncToCase; + return; + } } else if (state->steps_len == 2 && state->steps[0].opcode == EEOP_CONST) *************** ExecJustAssignScanVar(ExprState *state, *** 1811,1816 **** --- 1815,1857 ---- return 0; } + /* Evaluate CASE_TESTVAL and apply a strict function to it */ + static Datum + ExecJustApplyFuncToCase(ExprState *state, ExprContext *econtext, bool *isnull) + { + ExprEvalStep *op = &state->steps[0]; + FunctionCallInfo fcinfo; + bool *argnull; + int argno; + Datum d; + + /* + * XXX with some redesign of the CaseTestExpr mechanism, maybe we could + * get rid of this data shuffling? + */ + *op->resvalue = *op->d.casetest.value; + *op->resnull = *op->d.casetest.isnull; + + op++; + + fcinfo = op->d.func.fcinfo_data; + argnull = fcinfo->argnull; + + /* strict function, so check for NULL args */ + for (argno = 0; argno < op->d.func.nargs; argno++) + { + if (argnull[argno]) + { + *isnull = true; + return (Datum) 0; + } + } + fcinfo->isnull = false; + d = op->d.func.fn_addr(fcinfo); + *isnull = fcinfo->isnull; + return d; + } + /* * Do one-time initialization of interpretation machinery. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
pgsql-hackers by date: