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

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.

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.
       */