Cache opaque handle for GUC option to avoid repeasted lookups.

When setting GUCs from proconfig, performance is important, and hash
lookups in the GUC table are significant.

Per suggestion from Robert Haas.

Discussion: https://postgr.es/m/CA+TgmoYpKxhR3HOD9syK2XwcAUVPa0+ba0XPnwWBcYxtKLkyxA@mail.gmail.com
Reviewed-by: John Naylor

diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 9dfdf890c51b47219c984349856a5a6f960cf197..f8f770fd5c0cca8b1d8788f254fa22554df514e6 100644 (file)
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -613,6 +613,7 @@ struct fmgr_security_definer_cache
        FmgrInfo        flinfo;                 /* lookup info for target function */
        Oid                     userid;                 /* userid to set, or InvalidOid */
        List       *configNames;        /* GUC names to set, or NIL */
+       List       *configHandles;      /* GUC handles to set, or NIL */
        List       *configValues;       /* GUC values to set, or NIL */
        Datum           arg;                    /* passthrough argument for plugin modules */
 };
@@ -635,8 +636,9 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
        FmgrInfo   *save_flinfo;
        Oid                     save_userid;
        int                     save_sec_context;
-       ListCell   *lc1;
-       ListCell   *lc2;
+       ListCell   *lc1,
+                          *lc2,
+                          *lc3;
        volatile int save_nestlevel;
        PgStat_FunctionCallUsage fcusage;
 
@@ -670,11 +672,23 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
                if (!isnull)
                {
                        ArrayType  *array;
+                       ListCell   *lc;
 
                        oldcxt = MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt);
                        array = DatumGetArrayTypeP(datum);
                        TransformGUCArray(array, &fcache->configNames,
                                                          &fcache->configValues);
+
+                       /* transform names to config handles to avoid lookup cost */
+                       fcache->configHandles = NIL;
+                       foreach(lc, fcache->configNames)
+                       {
+                               char       *name = (char *) lfirst(lc);
+
+                               fcache->configHandles = lappend(fcache->configHandles,
+                                                                                               get_config_handle(name));
+                       }
+
                        MemoryContextSwitchTo(oldcxt);
                }
 
@@ -696,17 +710,20 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
                SetUserIdAndSecContext(fcache->userid,
                                                           save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
 
-       forboth(lc1, fcache->configNames, lc2, fcache->configValues)
+       forthree(lc1, fcache->configNames,
+                        lc2, fcache->configHandles,
+                        lc3, fcache->configValues)
        {
                GucContext      context = superuser() ? PGC_SUSET : PGC_USERSET;
                GucSource       source = PGC_S_SESSION;
                GucAction       action = GUC_ACTION_SAVE;
                char       *name = lfirst(lc1);
-               char       *value = lfirst(lc2);
+               config_handle *handle = lfirst(lc2);
+               char       *value = lfirst(lc3);
 
-               (void) set_config_option(name, value,
-                                                                context, source,
-                                                                action, true, 0, false);
+               (void) set_config_with_handle(name, handle, value,
+                                                                         context, source, GetUserId(),
+                                                                         action, true, 0, false);
        }
 
        /* function manager hook */

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index e76c0830035c85df70b0439834e9cd1423bf48c4..959a1c76bff710b47b91d001038181ae43736f86 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3329,10 +3329,10 @@ set_config_option(const char *name, const char *value,
        else
                srole = BOOTSTRAP_SUPERUSERID;
 
-       return set_config_option_ext(name, value,
-                                                                context, source, srole,
-                                                                action, changeVal, elevel,
-                                                                is_reload);
+       return set_config_with_handle(name, NULL, value,
+                                                                 context, source, srole,
+                                                                 action, changeVal, elevel,
+                                                                 is_reload);
 }
 
 /*
@@ -3355,6 +3355,27 @@ set_config_option_ext(const char *name, const char *value,
                                          GucContext context, GucSource source, Oid srole,
                                          GucAction action, bool changeVal, int elevel,
                                          bool is_reload)
+{
+       return set_config_with_handle(name, NULL, value,
+                                                                 context, source, srole,
+                                                                 action, changeVal, elevel,
+                                                                 is_reload);
+}
+
+
+/*
+ * set_config_with_handle: takes an optional 'handle' argument, which can be
+ * obtained by the caller from get_config_handle().
+ *
+ * This should be used by callers which repeatedly set the same config
+ * option(s), and want to avoid the overhead of a hash lookup each time.
+ */
+int
+set_config_with_handle(const char *name, config_handle *handle,
+                                          const char *value,
+                                          GucContext context, GucSource source, Oid srole,
+                                          GucAction action, bool changeVal, int elevel,
+                                          bool is_reload)
 {
        struct config_generic *record;
        union config_var_val newval_union;
@@ -3395,9 +3416,15 @@ set_config_option_ext(const char *name, const char *value,
                                (errcode(ERRCODE_INVALID_TRANSACTION_STATE),
                                 errmsg("cannot set parameters during a parallel operation")));
 
-       record = find_option(name, true, false, elevel);
-       if (record == NULL)
-               return 0;
+       /* if handle is specified, no need to look up option */
+       if (!handle)
+       {
+               record = find_option(name, true, false, elevel);
+               if (record == NULL)
+                       return 0;
+       }
+       else
+               record = handle;
 
        /*
         * Check if the option can be set at this time. See guc.h for the precise
@@ -4166,6 +4193,22 @@ set_config_option_ext(const char *name, const char *value,
 }
 
 
+/*
+ * Retrieve a config_handle for the given name, suitable for calling
+ * set_config_with_handle(). Only return handle to permanent GUC.
+ */
+config_handle *
+get_config_handle(const char *name)
+{
+       struct config_generic *gen = find_option(name, false, false, 0);
+
+       if (gen && ((gen->flags & GUC_CUSTOM_PLACEHOLDER) == 0))
+               return gen;
+
+       return NULL;
+}
+
+
 /*
  * Set the fields for source file and line number the setting came from.
  */

diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 20fe13702b18ca4052b105f9cdf9918859e5a3f3..49ee046cf0f3dfb9282644ed25c5b28a6478426f 100644 (file)
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -144,6 +144,8 @@ typedef struct ConfigVariable
        struct ConfigVariable *next;
 } ConfigVariable;
 
+typedef struct config_generic config_handle;
+
 extern bool ParseConfigFile(const char *config_file, bool strict,
                                                        const char *calling_file, int calling_lineno,
                                                        int depth, int elevel,
@@ -387,6 +389,13 @@ extern int set_config_option_ext(const char *name, const char *value,
                                                                  Oid srole,
                                                                  GucAction action, bool changeVal, int elevel,
                                                                  bool is_reload);
+extern int     set_config_with_handle(const char *name, config_handle *handle,
+                                                                  const char *value,
+                                                                  GucContext context, GucSource source,
+                                                                  Oid srole,
+                                                                  GucAction action, bool changeVal,
+                                                                  int elevel, bool is_reload);
+extern config_handle *get_config_handle(const char *name);
 extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt);
 extern char *GetConfigOptionByName(const char *name, const char **varname,
                                                                   bool missing_ok);

diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 1053f676c38956a1cabbe4e8c99c2843e54b04b3..ba41149b881fc63471432c12602fa53031f57e7f 100644 (file)
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3247,6 +3247,7 @@ collation_cache_entry
 color
 colormaprange
 compare_context
+config_handle
 config_var_value
 contain_aggs_of_level_context
 contain_placeholder_references_context