From 5765cfe18c595b5d8a7df3a62d253f60a00718ce Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Thu, 10 Aug 2023 12:43:53 -0700 Subject: [PATCH] Transform proconfig for faster execution. Store function config settings in lists to avoid the need to parse and allocate for each function execution. Speedup is modest but significant. Additionally, this change also seems cleaner and supports some other performance improvements under discussion. Discussion: https://postgr.es/m/04c8592dbd694e4114a3ed87139a7a04e4363030.camel@j-davis.com Reviewed-by: Nathan Bossart --- src/backend/utils/fmgr/fmgr.c | 29 +++++++++++++++------- src/backend/utils/misc/guc.c | 45 +++++++++++++++++++++++++++++------ src/include/utils/guc.h | 2 ++ 3 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c index 9208c31fe0..6b2d7d7be3 100644 --- a/src/backend/utils/fmgr/fmgr.c +++ b/src/backend/utils/fmgr/fmgr.c @@ -612,7 +612,8 @@ struct fmgr_security_definer_cache { FmgrInfo flinfo; /* lookup info for target function */ Oid userid; /* userid to set, or InvalidOid */ - ArrayType *proconfig; /* GUC values to set, or NULL */ + List *configNames; /* GUC names to set, or NIL */ + List *configValues; /* GUC values to set, or NIL */ Datum arg; /* passthrough argument for plugin modules */ }; @@ -634,6 +635,8 @@ fmgr_security_definer(PG_FUNCTION_ARGS) FmgrInfo *save_flinfo; Oid save_userid; int save_sec_context; + ListCell *lc1; + ListCell *lc2; volatile int save_nestlevel; PgStat_FunctionCallUsage fcusage; @@ -666,8 +669,11 @@ fmgr_security_definer(PG_FUNCTION_ARGS) &isnull); if (!isnull) { + ArrayType *array; oldcxt = MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt); - fcache->proconfig = DatumGetArrayTypePCopy(datum); + array = DatumGetArrayTypeP(datum); + TransformGUCArray(array, &fcache->configNames, + &fcache->configValues); MemoryContextSwitchTo(oldcxt); } @@ -680,7 +686,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS) /* GetUserIdAndSecContext is cheap enough that no harm in a wasted call */ GetUserIdAndSecContext(&save_userid, &save_sec_context); - if (fcache->proconfig) /* Need a new GUC nesting level */ + if (fcache->configNames != NIL) /* Need a new GUC nesting level */ save_nestlevel = NewGUCNestLevel(); else save_nestlevel = 0; /* keep compiler quiet */ @@ -689,12 +695,17 @@ fmgr_security_definer(PG_FUNCTION_ARGS) SetUserIdAndSecContext(fcache->userid, save_sec_context | SECURITY_LOCAL_USERID_CHANGE); - if (fcache->proconfig) + forboth(lc1, fcache->configNames, lc2, fcache->configValues) { - ProcessGUCArray(fcache->proconfig, - (superuser() ? PGC_SUSET : PGC_USERSET), - PGC_S_SESSION, - GUC_ACTION_SAVE); + 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); + + (void) set_config_option(name, value, + context, source, + action, true, 0, false); } /* function manager hook */ @@ -737,7 +748,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS) fcinfo->flinfo = save_flinfo; - if (fcache->proconfig) + if (fcache->configNames != NIL) AtEOXact_GUC(true, save_nestlevel); if (OidIsValid(fcache->userid)) SetUserIdAndSecContext(save_userid, save_sec_context); diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 5308896c87..3449470953 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -6215,14 +6215,12 @@ ParseLongOption(const char *string, char **name, char **value) /* - * Handle options fetched from pg_db_role_setting.setconfig, - * pg_proc.proconfig, etc. Caller must specify proper context/source/action. - * - * The array parameter must be an array of TEXT (it must not be NULL). + * Transform array of GUC settings into lists of names and values. The lists + * are faster to process in cases where the settings must be applied + * repeatedly (e.g. for each function invocation). */ void -ProcessGUCArray(ArrayType *array, - GucContext context, GucSource source, GucAction action) +TransformGUCArray(ArrayType *array, List **names, List **values) { int i; @@ -6231,6 +6229,8 @@ ProcessGUCArray(ArrayType *array, Assert(ARR_NDIM(array) == 1); Assert(ARR_LBOUND(array)[0] == 1); + *names = NIL; + *values = NIL; for (i = 1; i <= ARR_DIMS(array)[0]; i++) { Datum d; @@ -6262,14 +6262,45 @@ ProcessGUCArray(ArrayType *array, continue; } + *names = lappend(*names, name); + *values = lappend(*values, value); + + pfree(s); + } +} + + +/* + * Handle options fetched from pg_db_role_setting.setconfig, + * pg_proc.proconfig, etc. Caller must specify proper context/source/action. + * + * The array parameter must be an array of TEXT (it must not be NULL). + */ +void +ProcessGUCArray(ArrayType *array, + GucContext context, GucSource source, GucAction action) +{ + List *gucNames; + List *gucValues; + ListCell *lc1; + ListCell *lc2; + + TransformGUCArray(array, &gucNames, &gucValues); + forboth(lc1, gucNames, lc2, gucValues) + { + char *name = lfirst(lc1); + char *value = lfirst(lc2); + (void) set_config_option(name, value, context, source, action, true, 0, false); pfree(name); pfree(value); - pfree(s); } + + list_free(gucNames); + list_free(gucValues); } diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index 223a19f80d..e89083ee0e 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -391,6 +391,8 @@ extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt); extern char *GetConfigOptionByName(const char *name, const char **varname, bool missing_ok); +extern void TransformGUCArray(ArrayType *array, List **configNames, + List **configValues); extern void ProcessGUCArray(ArrayType *array, GucContext context, GucSource source, GucAction action); extern ArrayType *GUCArrayAdd(ArrayType *array, const char *name, const char *value);