Re: Changing SQL Inlining Behaviour (or...?) - Mailing list pgsql-hackers

> On Jan 19, 2019, at 12:25 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Adam Brightwell <adam.brightwell@crunchydata.com> writes:
>> I've been working with Paul to create and test a patch (attached) that
>> addresses Solution #2. This patch essentially modifies the inlining
>> logic to compare the cost of the function with the total cost of the
>> parameters. The goal as stated above, is that for these kinds of
>> functions, they would be assigned relatively high cost to trigger the
>> inlining case.
>
> I played around with this a bit.  I think we could make the rule simpler
> and more useful by expressing it as "if the total incremental cost from
> multiple evaluation of parameters exceeds the inlinable function's
> nominal cost, then don't inline".  The existing rule is "no individual
> param expression can cost more than 10*cpu_operator_cost, if the
> function uses it more than once --- never mind how many times exactly".
> With the default cost (100) for a SQL language function, this'd become
> "the total incremental cost can't be more than 100*cpu_operator_cost".
> So that's more or less in the same ballpark, though it will make some
> different decisions in cases near the boundary, and probably will choose
> to inline in more cases than before.  But it's simpler to compute and
> more defensible as a criterion, IMO.

Is there a rule of thumb we can use in costing our wrappers to ensure that they always inline?

P

> inlining-solution-3a.patch, attached, does it like that, keeping the
> existing rule that we flat out won't inline if there are any subplans
> in multiply-evaluated parameters.  That seems kind of ugly, though,
> so I also prepared inlining-solution-3b.patch which eliminates the
> cruft around subplans in this logic by moving the cruft into
> cost_qual_eval instead: it has to make an arbitrary assumption about
> the cost of a SubLink.
>
> I'm not really sure which of these I like better.  The arbitrary cost
> assumption in cost_qual_eval is not great, and it'd be really bad if
> anyone ever tried to use costs for pre-SS_process_sublinks expressions
> for something more important than the case at hand.  But on the other
> hand it seems nice to get rid of the arbitrary inlining choice.
>
> Thoughts?
>
> The other thing we need to consider is whether we need any documentation
> adjustments.  I believe that right now, the rules for inlining SQL
> functions are not documented anywhere but the code, and maybe it's not
> this patch's job to change that.
>
>             regards, tom lane
>
> diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
> index f0ef102..c99304d 100644
> *** a/src/backend/optimizer/util/clauses.c
> --- b/src/backend/optimizer/util/clauses.c
> *************** evaluate_function(Oid funcid, Oid result
> *** 4645,4651 ****
>   * unreasonably expensive to evaluate.  The expensiveness check not only
>   * prevents us from doing multiple evaluations of an expensive parameter
>   * at runtime, but is a safety value to limit growth of an expression due
> !  * to repeated inlining.
>   *
>   * We must also beware of changing the volatility or strictness status of
>   * functions by inlining them.
> --- 4645,4653 ----
>   * unreasonably expensive to evaluate.  The expensiveness check not only
>   * prevents us from doing multiple evaluations of an expensive parameter
>   * at runtime, but is a safety value to limit growth of an expression due
> !  * to repeated inlining.  (Currently, the expensiveness check is expressed
> !  * as a limit on the total additional cost of extra parameter evaluations,
> !  * rather than of any one parameter.)
>   *
>   * We must also beware of changing the volatility or strictness status of
>   * functions by inlining them.
> *************** inline_function(Oid funcid, Oid result_t
> *** 4682,4687 ****
> --- 4684,4690 ----
>      Query       *querytree;
>      Node       *newexpr;
>      int           *usecounts;
> +     Cost        delta_param_cost;
>      ListCell   *arg;
>      int            i;
>
> *************** inline_function(Oid funcid, Oid result_t
> *** 4876,4882 ****
>      newexpr = substitute_actual_parameters(newexpr, funcform->pronargs,
>                                             args, usecounts);
>
> !     /* Now check for parameter usage */
>      i = 0;
>      foreach(arg, args)
>      {
> --- 4879,4889 ----
>      newexpr = substitute_actual_parameters(newexpr, funcform->pronargs,
>                                             args, usecounts);
>
> !     /*
> !      * Check for parameter usage, and estimate the delta in evaluation cost
> !      * resulting from parameters possibly being evaluated more than once.
> !      */
> !     delta_param_cost = 0.0;
>      i = 0;
>      foreach(arg, args)
>      {
> *************** inline_function(Oid funcid, Oid result_t
> *** 4887,4922 ****
>              /* Param not used at all: uncool if func is strict */
>              if (funcform->proisstrict)
>                  goto fail;
>          }
>          else if (usecounts[i] != 1)
>          {
> !             /* Param used multiple times: uncool if expensive or volatile */
>              QualCost    eval_cost;
>
>              /*
> !              * We define "expensive" as "contains any subplan or more than 10
> !              * operators".  Note that the subplan search has to be done
> !              * explicitly, since cost_qual_eval() will barf on unplanned
> !              * subselects.
>               */
>              if (contain_subplans(param))
>                  goto fail;
> -             cost_qual_eval(&eval_cost, list_make1(param), NULL);
> -             if (eval_cost.startup + eval_cost.per_tuple >
> -                 10 * cpu_operator_cost)
> -                 goto fail;
>
>              /*
> !              * Check volatility last since this is more expensive than the
> !              * above tests
>               */
> !             if (contain_volatile_functions(param))
> !                 goto fail;
>          }
>          i++;
>      }
>
>      /*
>       * Whew --- we can make the substitution.  Copy the modified expression
>       * out of the temporary memory context, and clean up.
>       */
> --- 4894,4949 ----
>              /* Param not used at all: uncool if func is strict */
>              if (funcform->proisstrict)
>                  goto fail;
> +
> +             /*
> +              * In principle, we should credit the eval cost of the param expr
> +              * as a savings from inlining.  However, it's hard to deal with
> +              * subplans properly, so we don't bother.  This case is unlikely
> +              * anyway, so we just try to be semantically correct.
> +              */
>          }
>          else if (usecounts[i] != 1)
>          {
> !             /* Param used multiple times: uncool if volatile */
>              QualCost    eval_cost;
>
> +             if (contain_volatile_functions(param))
> +                 goto fail;
> +
>              /*
> !              * If the param contains any SubLinks, cost_qual_eval() will barf
> !              * on it.  Arbitrarily assume that the true cost of a subplan is
> !              * high enough that we should not inline.
>               */
>              if (contain_subplans(param))
>                  goto fail;
>
>              /*
> !              * Otherwise, estimate the evaluation cost of the param, and
> !              * charge for the N-1 extra evaluations.  Note that we consider
> !              * only per-tuple cost not startup cost.  Ideally we'd account for
> !              * that somehow, but it's hard since we have as yet no idea how
> !              * many times the query will execute this expression.
>               */
> !             cost_qual_eval_node(&eval_cost, param, context->root);
> !             delta_param_cost += eval_cost.per_tuple * (usecounts[i] - 1);
>          }
>          i++;
>      }
>
>      /*
> +      * If the incremental cost of multiple parameter evaluation exceeds the
> +      * nominal cost of the function we're inlining, don't inline.  With the
> +      * default cost for a SQL-language function, this rule causes us to not
> +      * inline if multiple evaluation would add more than 100 times
> +      * cpu_operator_cost.  However, function authors can adjust the claimed
> +      * cost of inlinable functions to encourage or discourage inlining in the
> +      * presence of multiple parameter usage.
> +      */
> +     if (delta_param_cost > funcform->procost * cpu_operator_cost)
> +         goto fail;
> +
> +     /*
>       * Whew --- we can make the substitution.  Copy the modified expression
>       * out of the temporary memory context, and clean up.
>       */
> diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
> index 99c5ad9..eb311fa 100644
> *** a/src/backend/optimizer/path/costsize.c
> --- b/src/backend/optimizer/path/costsize.c
> *************** cost_qual_eval_walker(Node *node, cost_q
> *** 3929,3936 ****
>      }
>      else if (IsA(node, SubLink))
>      {
> !         /* This routine should not be applied to un-planned expressions */
> !         elog(ERROR, "cannot handle unplanned sub-select");
>      }
>      else if (IsA(node, SubPlan))
>      {
> --- 3929,3943 ----
>      }
>      else if (IsA(node, SubLink))
>      {
> !         /*
> !          * In normal usage we won't see un-planned SubLinks here, because they
> !          * will previously have been converted to SubPlans or InitPlans.
> !          * However, we can reach here due to inline_function() wishing to
> !          * estimate costs for function parameter expressions.  Arbitrarily
> !          * return a fairly large cost to discourage inlining when that would
> !          * cause multiple eval of a subplan.
> !          */
> !         context->total.per_tuple += 100000 * cpu_operator_cost;
>      }
>      else if (IsA(node, SubPlan))
>      {
> diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
> index f0ef102..84fdcc6 100644
> *** a/src/backend/optimizer/util/clauses.c
> --- b/src/backend/optimizer/util/clauses.c
> *************** evaluate_function(Oid funcid, Oid result
> *** 4645,4651 ****
>   * unreasonably expensive to evaluate.  The expensiveness check not only
>   * prevents us from doing multiple evaluations of an expensive parameter
>   * at runtime, but is a safety value to limit growth of an expression due
> !  * to repeated inlining.
>   *
>   * We must also beware of changing the volatility or strictness status of
>   * functions by inlining them.
> --- 4645,4653 ----
>   * unreasonably expensive to evaluate.  The expensiveness check not only
>   * prevents us from doing multiple evaluations of an expensive parameter
>   * at runtime, but is a safety value to limit growth of an expression due
> !  * to repeated inlining.  (Currently, the expensiveness check is expressed
> !  * as a limit on the total additional cost of extra parameter evaluations,
> !  * rather than of any one parameter.)
>   *
>   * We must also beware of changing the volatility or strictness status of
>   * functions by inlining them.
> *************** inline_function(Oid funcid, Oid result_t
> *** 4682,4687 ****
> --- 4684,4690 ----
>      Query       *querytree;
>      Node       *newexpr;
>      int           *usecounts;
> +     Cost        delta_param_cost;
>      ListCell   *arg;
>      int            i;
>
> *************** inline_function(Oid funcid, Oid result_t
> *** 4876,4882 ****
>      newexpr = substitute_actual_parameters(newexpr, funcform->pronargs,
>                                             args, usecounts);
>
> !     /* Now check for parameter usage */
>      i = 0;
>      foreach(arg, args)
>      {
> --- 4879,4889 ----
>      newexpr = substitute_actual_parameters(newexpr, funcform->pronargs,
>                                             args, usecounts);
>
> !     /*
> !      * Check for parameter usage, and estimate the delta in evaluation cost
> !      * resulting from parameters possibly being evaluated more than once.
> !      */
> !     delta_param_cost = 0.0;
>      i = 0;
>      foreach(arg, args)
>      {
> *************** inline_function(Oid funcid, Oid result_t
> *** 4888,4922 ****
>              if (funcform->proisstrict)
>                  goto fail;
>          }
> !         else if (usecounts[i] != 1)
>          {
> !             /* Param used multiple times: uncool if expensive or volatile */
> !             QualCost    eval_cost;
> !
> !             /*
> !              * We define "expensive" as "contains any subplan or more than 10
> !              * operators".  Note that the subplan search has to be done
> !              * explicitly, since cost_qual_eval() will barf on unplanned
> !              * subselects.
> !              */
> !             if (contain_subplans(param))
> !                 goto fail;
> !             cost_qual_eval(&eval_cost, list_make1(param), NULL);
> !             if (eval_cost.startup + eval_cost.per_tuple >
> !                 10 * cpu_operator_cost)
>                  goto fail;
>
>              /*
> !              * Check volatility last since this is more expensive than the
> !              * above tests
>               */
> !             if (contain_volatile_functions(param))
> !                 goto fail;
>          }
>          i++;
>      }
>
>      /*
>       * Whew --- we can make the substitution.  Copy the modified expression
>       * out of the temporary memory context, and clean up.
>       */
> --- 4895,4939 ----
>              if (funcform->proisstrict)
>                  goto fail;
>          }
> !         else if (usecounts[i] > 1)
>          {
> !             /* Param used multiple times: uncool if volatile */
> !             if (contain_volatile_functions(param))
>                  goto fail;
> +         }
>
> +         if (usecounts[i] != 1)
> +         {
>              /*
> !              * Estimate the evaluation cost of the param, and charge for the
> !              * N-1 extra evaluations --- or, if it's not used at all, credit
> !              * the savings.  Note that we consider only per-tuple cost, not
> !              * startup cost.  Ideally we'd account for that somehow, but it's
> !              * hard since we have as yet no idea how many times the query will
> !              * execute this expression.
>               */
> !             QualCost    eval_cost;
> !
> !             cost_qual_eval_node(&eval_cost, param, context->root);
> !             delta_param_cost += eval_cost.per_tuple * (usecounts[i] - 1);
>          }
> +
>          i++;
>      }
>
>      /*
> +      * If the incremental cost of multiple parameter evaluation exceeds the
> +      * nominal cost of the function we're inlining, don't inline.  With the
> +      * default cost for a SQL-language function, this rule causes us to not
> +      * inline if multiple evaluation would add more than 100 times
> +      * cpu_operator_cost.  However, function authors can adjust the claimed
> +      * cost of inlinable functions to encourage or discourage inlining in the
> +      * presence of multiple parameter usage.
> +      */
> +     if (delta_param_cost > funcform->procost * cpu_operator_cost)
> +         goto fail;
> +
> +     /*
>       * Whew --- we can make the substitution.  Copy the modified expression
>       * out of the temporary memory context, and clean up.
>       */