Avoid taking a new snapshot for an immutable simple expression in plpgsql.

We already used this optimization if the plpgsql function is read-only.
But it seems okay to do it even in a read-write function, if the
expression contains only immutable functions/operators.  There would
only be a change of behavior if an "immutable" called function depends
on seeing database updates made during the current plpgsql function.
That's enough of a violation of the promise of immutability that anyone
who complains won't have much of a case.

The benefits are significant --- for simple cases like

  while i < 10000000
  loop
    i := i + 1;
  end loop;

I see net performance improvements around 45%.  Of course, real-world
cases won't get that much faster, but it ought to be noticeable.
At the very least, this removes much of the performance penalty that
used to exist for forgetting to mark a plpgsql function non-volatile.

Konstantin Knizhnik, reviewed by Pavel Stehule, cosmetic changes by me

Discussion: https://postgr.es/m/ed9da20e-01aa-d04b-d085-e6c16b14b9d7@postgrespro.ru
This commit is contained in:
Tom Lane 2019-11-22 15:02:18 -05:00
parent f67b173771
commit 73b06cf893
2 changed files with 16 additions and 5 deletions

View File

@ -6079,6 +6079,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
LocalTransactionId curlxid = MyProc->lxid;
CachedPlan *cplan;
void *save_setup_arg;
bool need_snapshot;
MemoryContext oldcontext;
/*
@ -6150,12 +6151,19 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
/*
* We have to do some of the things SPI_execute_plan would do, in
* particular advance the snapshot if we are in a non-read-only function.
* Without this, stable functions within the expression would fail to see
* updates made so far by our own function.
* particular push a new snapshot so that stable functions within the
* expression can see updates made so far by our own function. However,
* we can skip doing that (and just invoke the expression with the same
* snapshot passed to our function) in some cases, which is useful because
* it's quite expensive relative to the cost of a simple expression. We
* can skip it if the expression contains no stable or volatile functions;
* immutable functions shouldn't need to see our updates. Also, if this
* is a read-only function, we haven't made any updates so again it's okay
* to skip.
*/
oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate));
if (!estate->readonly_func)
need_snapshot = (expr->expr_simple_mutable && !estate->readonly_func);
if (need_snapshot)
{
CommandCounterIncrement();
PushActiveSnapshot(GetTransactionSnapshot());
@ -6180,7 +6188,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
estate->paramLI->parserSetupArg = save_setup_arg;
if (!estate->readonly_func)
if (need_snapshot)
PopActiveSnapshot();
MemoryContextSwitchTo(oldcontext);
@ -8051,6 +8059,8 @@ exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan)
/* Also stash away the expression result type */
expr->expr_simple_type = exprType((Node *) tle_expr);
expr->expr_simple_typmod = exprTypmod((Node *) tle_expr);
/* We also want to remember if it is immutable or not */
expr->expr_simple_mutable = contain_mutable_functions((Node *) tle_expr);
}
/*

View File

@ -234,6 +234,7 @@ typedef struct PLpgSQL_expr
int expr_simple_generation; /* plancache generation we checked */
Oid expr_simple_type; /* result type Oid, if simple */
int32 expr_simple_typmod; /* result typmod, if simple */
bool expr_simple_mutable; /* true if simple expr is mutable */
/*
* if expr is simple AND prepared in current transaction,