Re: Inlining of couple of functions in pl_exec.c improves performance - Mailing list pgsql-hackers

I did some performance testing on 0001+0002 here, and found that
for me, there's basically no change on x86_64 but a win of 2 to 3
percent on aarch64, using Pavel's pi_est_1() as a representative
case for simple plpgsql statements.  That squares with your original
results I believe.  It's not clear to me whether any of the later
tests in this thread measured these changes in isolation, or only
with 0003 added.

Anyway, that's good enough for me, so I pushed 0001+0002 after a
little bit of additional cosmetic tweaking.

I attach your original 0003 here (it still applies, with some line
offsets).  That's just so the cfbot doesn't get confused about what
it's supposed to test now.

            regards, tom lane

From 56aac7dff8243ff6dc9b8e72651cb1d9a018f1b3 Mon Sep 17 00:00:00 2001
From: Amit Khandekar <amitdkhan.pg@gmail.com>
Date: Sat, 23 May 2020 21:53:24 +0800
Subject: [PATCH 3/3] Inline exec_cast_value().

This function does not do the casting if not required. So move the
code that actually does the cast into a separate function, so as to
reduce the exec_cast_value() code and make it inline.

There are frequent calls of this function, so inlining it has shown to
improve performance by as much as 14%
---
 src/pl/plpgsql/src/pl_exec.c | 63 ++++++++++++++++++++++++------------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index d5377a6dad..4028a3f0f6 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -425,10 +425,14 @@ static void instantiate_empty_record_variable(PLpgSQL_execstate *estate,
                                               PLpgSQL_rec *rec);
 static char *convert_value_to_string(PLpgSQL_execstate *estate,
                                      Datum value, Oid valtype);
-static Datum exec_cast_value(PLpgSQL_execstate *estate,
+static inline Datum exec_cast_value(PLpgSQL_execstate *estate,
                              Datum value, bool *isnull,
                              Oid valtype, int32 valtypmod,
                              Oid reqtype, int32 reqtypmod);
+static Datum do_cast_value(PLpgSQL_execstate *estate,
+                Datum value, bool *isnull,
+                Oid valtype, int32 valtypmod,
+                Oid reqtype, int32 reqtypmod);
 static plpgsql_CastHashEntry *get_cast_hashentry(PLpgSQL_execstate *estate,
                                                  Oid srctype, int32 srctypmod,
                                                  Oid dsttype, int32 dsttypmod);
@@ -7764,9 +7768,11 @@ convert_value_to_string(PLpgSQL_execstate *estate, Datum value, Oid valtype)
  * also contain the result Datum if we have to do a conversion to a pass-
  * by-reference data type.  Be sure to do an exec_eval_cleanup() call when
  * done with the result.
+ * The actual code to cast is kept outside of this function, to keep it short
+ * since it is an inline function, being called frequently.
  * ----------
  */
-static Datum
+static inline Datum
 exec_cast_value(PLpgSQL_execstate *estate,
                 Datum value, bool *isnull,
                 Oid valtype, int32 valtypmod,
@@ -7777,31 +7783,48 @@ exec_cast_value(PLpgSQL_execstate *estate,
      */
     if (valtype != reqtype ||
         (valtypmod != reqtypmod && reqtypmod != -1))
-    {
-        plpgsql_CastHashEntry *cast_entry;
+        value = do_cast_value(estate, value, isnull, valtype, valtypmod,
+                              reqtype, reqtypmod);

-        cast_entry = get_cast_hashentry(estate,
-                                        valtype, valtypmod,
-                                        reqtype, reqtypmod);
-        if (cast_entry)
-        {
-            ExprContext *econtext = estate->eval_econtext;
-            MemoryContext oldcontext;
+    return value;
+}

-            oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate));
+/* ----------
+ * do_cast_value            cast the input value.
+ *
+ * Returns the cast value.
+ * Check comments in the wrapper function exec_cast_value().
+ * ----------
+ */
+static Datum
+do_cast_value(PLpgSQL_execstate *estate,
+                Datum value, bool *isnull,
+                Oid valtype, int32 valtypmod,
+                Oid reqtype, int32 reqtypmod)
+{
+    plpgsql_CastHashEntry *cast_entry;

-            econtext->caseValue_datum = value;
-            econtext->caseValue_isNull = *isnull;
+    cast_entry = get_cast_hashentry(estate,
+                                    valtype, valtypmod,
+                                    reqtype, reqtypmod);
+    if (cast_entry)
+    {
+        ExprContext *econtext = estate->eval_econtext;
+        MemoryContext oldcontext;

-            cast_entry->cast_in_use = true;
+        oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate));

-            value = ExecEvalExpr(cast_entry->cast_exprstate, econtext,
-                                 isnull);
+        econtext->caseValue_datum = value;
+        econtext->caseValue_isNull = *isnull;

-            cast_entry->cast_in_use = false;
+        cast_entry->cast_in_use = true;

-            MemoryContextSwitchTo(oldcontext);
-        }
+        value = ExecEvalExpr(cast_entry->cast_exprstate, econtext,
+                             isnull);
+
+        cast_entry->cast_in_use = false;
+
+        MemoryContextSwitchTo(oldcontext);
     }

     return value;
--
2.17.1