Re: Modernizing our GUC infrastructure - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Re: Modernizing our GUC infrastructure |
| Date | |
| Msg-id | 3336389.1665171086@sss.pgh.pa.us Whole thread Raw |
| In response to | Re: Modernizing our GUC infrastructure (Tom Lane <tgl@sss.pgh.pa.us>) |
| List | pgsql-hackers |
I wrote:
>> Attached is a patch series that attempts to modernize our GUC
>> infrastructure, in particular removing the performance bottlenecks
>> it has when there are lots of GUC variables.
> Rebased over 0a20ff54f.
Here's a v3 rebased up to HEAD. The only real change is that I added
a couple of "Assert(GetMemoryChunkContext(ptr) == GUCMemoryContext)"
checks in hopes of improving detection of not-updated code that is
still using malloc/free where it should be using guc_malloc/guc_free.
This is per the nearby discussion of whether the mcxt.c infrastructure
could recognize that [1]. I experimented a bit with leaving out parts
of the 0002 patch to simulate such mistakes, and at least on a Linux
box that seems to produce fairly intelligible errors now. In the case
of free'ing a palloc'd pointer, what you get is a message from glibc
followed by abort(), so their error detection is pretty solid too.
I'm feeling pretty good about this patchset now. Does anyone want
to review it further?
regards, tom lane
[1] https://postgr.es/m/2910981.1665080361%40sss.pgh.pa.us
commit 1b48708e28c00df94df1911548e43aaeec65ad76
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri Oct 7 14:39:07 2022 -0400
Preliminary improvements in memory-context infrastructure.
We lack a version of repalloc() that supports MCXT_ALLOC_NO_OOM
semantics, so invent repalloc_extended(). repalloc_huge()
becomes a legacy wrapper for that.
Also, fix dynahash.c so that it can support HASH_ENTER_NULL
requests when using the default palloc-based allocator.
The only reason it was like that was the lack of the
MCXT_ALLOC_NO_OOM option when that code was written, ages ago.
While here, simplify a few overcomplicated tests in mcxt.c.
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 3babde8d70..4f62958883 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -289,7 +289,8 @@ static void *
DynaHashAlloc(Size size)
{
Assert(MemoryContextIsValid(CurrentDynaHashCxt));
- return MemoryContextAlloc(CurrentDynaHashCxt, size);
+ return MemoryContextAllocExtended(CurrentDynaHashCxt, size,
+ MCXT_ALLOC_NO_OOM);
}
@@ -939,9 +940,7 @@ calc_bucket(HASHHDR *hctl, uint32 hash_val)
*
* HASH_ENTER will normally ereport a generic "out of memory" error if
* it is unable to create a new entry. The HASH_ENTER_NULL operation is
- * the same except it will return NULL if out of memory. Note that
- * HASH_ENTER_NULL cannot be used with the default palloc-based allocator,
- * since palloc internally ereports on out-of-memory.
+ * the same except it will return NULL if out of memory.
*
* If foundPtr isn't NULL, then *foundPtr is set true if we found an
* existing entry in the table, false otherwise. This is needed in the
@@ -1084,12 +1083,8 @@ hash_search_with_hash_value(HTAB *hashp,
}
return NULL;
- case HASH_ENTER_NULL:
- /* ENTER_NULL does not work with palloc-based allocator */
- Assert(hashp->alloc != DynaHashAlloc);
- /* FALL THRU */
-
case HASH_ENTER:
+ case HASH_ENTER_NULL:
/* Return existing element if found, else create one */
if (currBucket != NULL)
return (void *) ELEMENTKEY(currBucket);
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index b1a3c74830..012517be5c 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1114,8 +1114,8 @@ MemoryContextAllocExtended(MemoryContext context, Size size, int flags)
AssertArg(MemoryContextIsValid(context));
AssertNotInCriticalSection(context);
- if (((flags & MCXT_ALLOC_HUGE) != 0 && !AllocHugeSizeIsValid(size)) ||
- ((flags & MCXT_ALLOC_HUGE) == 0 && !AllocSizeIsValid(size)))
+ if (!((flags & MCXT_ALLOC_HUGE) != 0 ? AllocHugeSizeIsValid(size) :
+ AllocSizeIsValid(size)))
elog(ERROR, "invalid memory alloc request size %zu", size);
context->isReset = false;
@@ -1269,8 +1269,8 @@ palloc_extended(Size size, int flags)
AssertArg(MemoryContextIsValid(context));
AssertNotInCriticalSection(context);
- if (((flags & MCXT_ALLOC_HUGE) != 0 && !AllocHugeSizeIsValid(size)) ||
- ((flags & MCXT_ALLOC_HUGE) == 0 && !AllocSizeIsValid(size)))
+ if (!((flags & MCXT_ALLOC_HUGE) != 0 ? AllocHugeSizeIsValid(size) :
+ AllocSizeIsValid(size)))
elog(ERROR, "invalid memory alloc request size %zu", size);
context->isReset = false;
@@ -1351,6 +1351,50 @@ repalloc(void *pointer, Size size)
return ret;
}
+/*
+ * repalloc_extended
+ * Adjust the size of a previously allocated chunk,
+ * with HUGE and NO_OOM options.
+ */
+void *
+repalloc_extended(void *pointer, Size size, int flags)
+{
+#if defined(USE_ASSERT_CHECKING) || defined(USE_VALGRIND)
+ MemoryContext context = GetMemoryChunkContext(pointer);
+#endif
+ void *ret;
+
+ if (!((flags & MCXT_ALLOC_HUGE) != 0 ? AllocHugeSizeIsValid(size) :
+ AllocSizeIsValid(size)))
+ elog(ERROR, "invalid memory alloc request size %zu", size);
+
+ AssertNotInCriticalSection(context);
+
+ /* isReset must be false already */
+ Assert(!context->isReset);
+
+ ret = MCXT_METHOD(pointer, realloc) (pointer, size);
+ if (unlikely(ret == NULL))
+ {
+ if ((flags & MCXT_ALLOC_NO_OOM) == 0)
+ {
+ MemoryContext cxt = GetMemoryChunkContext(pointer);
+
+ MemoryContextStats(TopMemoryContext);
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory"),
+ errdetail("Failed on request of size %zu in memory context \"%s\".",
+ size, cxt->name)));
+ }
+ return NULL;
+ }
+
+ VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
+
+ return ret;
+}
+
/*
* MemoryContextAllocHuge
* Allocate (possibly-expansive) space within the specified context.
@@ -1394,35 +1438,8 @@ MemoryContextAllocHuge(MemoryContext context, Size size)
void *
repalloc_huge(void *pointer, Size size)
{
-#if defined(USE_ASSERT_CHECKING) || defined(USE_VALGRIND)
- MemoryContext context = GetMemoryChunkContext(pointer);
-#endif
- void *ret;
-
- if (!AllocHugeSizeIsValid(size))
- elog(ERROR, "invalid memory alloc request size %zu", size);
-
- AssertNotInCriticalSection(context);
-
- /* isReset must be false already */
- Assert(!context->isReset);
-
- ret = MCXT_METHOD(pointer, realloc) (pointer, size);
- if (unlikely(ret == NULL))
- {
- MemoryContext cxt = GetMemoryChunkContext(pointer);
-
- MemoryContextStats(TopMemoryContext);
- ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory"),
- errdetail("Failed on request of size %zu in memory context \"%s\".",
- size, cxt->name)));
- }
-
- VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
-
- return ret;
+ /* this one seems not worth its own implementation */
+ return repalloc_extended(pointer, size, MCXT_ALLOC_HUGE);
}
/*
diff --git a/src/include/utils/palloc.h b/src/include/utils/palloc.h
index a0b62aa7b0..8eee0e2938 100644
--- a/src/include/utils/palloc.h
+++ b/src/include/utils/palloc.h
@@ -78,6 +78,8 @@ extern void *palloc(Size size);
extern void *palloc0(Size size);
extern void *palloc_extended(Size size, int flags);
extern pg_nodiscard void *repalloc(void *pointer, Size size);
+extern pg_nodiscard void *repalloc_extended(void *pointer,
+ Size size, int flags);
extern void pfree(void *pointer);
/*
commit 57215a9cc2daaeefd7d846500b519cfd1cab069c
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri Oct 7 15:05:02 2022 -0400
Store GUC data in a memory context, not with malloc().
The only real argument for using malloc directly was that we needed
the ability to not throw error on OOM; but mcxt.c grew that feature
awhile ago.
Keeping the data in a memory context improves accountability and
debuggability --- for example, without this it's almost impossible
to detect memory leaks in the GUC code with anything less costly
than valgrind. Moreover, the next patch in this series will add a
hash table for GUC lookup, and it'd be pretty silly to be using
palloc-dependent hash facilities along with malloc storage of the
underlying data.
This is a bit invasive though, in particular causing an API break
for GUC check hooks that want to modify the GUC's value or use an
"extra" data structure. My guess is that not very many extensions
will be affected by that, but perhaps I'm wrong.
One note is that this changes ParseLongOption() to return short-lived
palloc'd not malloc'd data. There wasn't any caller for which the
latter was better.
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 22635f8094..58247d826d 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -287,8 +287,8 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
}
SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
- free(name);
- free(value);
+ pfree(name);
+ pfree(value);
break;
}
default:
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index b69ff37dbb..45b30ca566 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -1290,8 +1290,8 @@ check_temp_tablespaces(char **newval, void **extra, GucSource source)
}
/* Now prepare an "extra" struct for assign_temp_tablespaces */
- myextra = malloc(offsetof(temp_tablespaces_extra, tblSpcs) +
- numSpcs * sizeof(Oid));
+ myextra = guc_malloc(LOG, offsetof(temp_tablespaces_extra, tblSpcs) +
+ numSpcs * sizeof(Oid));
if (!myextra)
return false;
myextra->numSpcs = numSpcs;
diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index e555fb3150..791bac6715 100644
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -148,7 +148,7 @@ check_datestyle(char **newval, void **extra, GucSource source)
char *subval;
void *subextra = NULL;
- subval = strdup(GetConfigOptionResetString("datestyle"));
+ subval = guc_strdup(LOG, GetConfigOptionResetString("datestyle"));
if (!subval)
{
ok = false;
@@ -156,7 +156,7 @@ check_datestyle(char **newval, void **extra, GucSource source)
}
if (!check_datestyle(&subval, &subextra, source))
{
- free(subval);
+ guc_free(subval);
ok = false;
break;
}
@@ -165,8 +165,8 @@ check_datestyle(char **newval, void **extra, GucSource source)
newDateStyle = myextra[0];
if (!have_order)
newDateOrder = myextra[1];
- free(subval);
- free(subextra);
+ guc_free(subval);
+ guc_free(subextra);
}
else
{
@@ -187,9 +187,9 @@ check_datestyle(char **newval, void **extra, GucSource source)
}
/*
- * Prepare the canonical string to return. GUC wants it malloc'd.
+ * Prepare the canonical string to return. GUC wants it guc_malloc'd.
*/
- result = (char *) malloc(32);
+ result = (char *) guc_malloc(LOG, 32);
if (!result)
return false;
@@ -221,13 +221,13 @@ check_datestyle(char **newval, void **extra, GucSource source)
break;
}
- free(*newval);
+ guc_free(*newval);
*newval = result;
/*
* Set up the "extra" struct actually used by assign_datestyle.
*/
- myextra = (int *) malloc(2 * sizeof(int));
+ myextra = (int *) guc_malloc(LOG, 2 * sizeof(int));
if (!myextra)
return false;
myextra[0] = newDateStyle;
@@ -366,7 +366,7 @@ check_timezone(char **newval, void **extra, GucSource source)
/*
* Pass back data for assign_timezone to use
*/
- *extra = malloc(sizeof(pg_tz *));
+ *extra = guc_malloc(LOG, sizeof(pg_tz *));
if (!*extra)
return false;
*((pg_tz **) *extra) = new_tz;
@@ -439,7 +439,7 @@ check_log_timezone(char **newval, void **extra, GucSource source)
/*
* Pass back data for assign_log_timezone to use
*/
- *extra = malloc(sizeof(pg_tz *));
+ *extra = guc_malloc(LOG, sizeof(pg_tz *));
if (!*extra)
return false;
*((pg_tz **) *extra) = new_tz;
@@ -500,7 +500,7 @@ check_timezone_abbreviations(char **newval, void **extra, GucSource source)
return true;
}
- /* OK, load the file and produce a malloc'd TimeZoneAbbrevTable */
+ /* OK, load the file and produce a guc_malloc'd TimeZoneAbbrevTable */
*extra = load_tzoffsets(*newval);
/* tzparser.c returns NULL on failure, reporting via GUC_check_errmsg */
@@ -647,7 +647,7 @@ check_transaction_deferrable(bool *newval, void **extra, GucSource source)
bool
check_random_seed(double *newval, void **extra, GucSource source)
{
- *extra = malloc(sizeof(int));
+ *extra = guc_malloc(LOG, sizeof(int));
if (!*extra)
return false;
/* Arm the assign only if source of value is an interactive SET */
@@ -735,8 +735,8 @@ check_client_encoding(char **newval, void **extra, GucSource source)
if (strcmp(*newval, canonical_name) != 0 &&
strcmp(*newval, "UNICODE") != 0)
{
- free(*newval);
- *newval = strdup(canonical_name);
+ guc_free(*newval);
+ *newval = guc_strdup(LOG, canonical_name);
if (!*newval)
return false;
}
@@ -744,7 +744,7 @@ check_client_encoding(char **newval, void **extra, GucSource source)
/*
* Save the encoding's ID in *extra, for use by assign_client_encoding.
*/
- *extra = malloc(sizeof(int));
+ *extra = guc_malloc(LOG, sizeof(int));
if (!*extra)
return false;
*((int *) *extra) = encoding;
@@ -847,7 +847,7 @@ check_session_authorization(char **newval, void **extra, GucSource source)
ReleaseSysCache(roleTup);
/* Set up "extra" struct for assign_session_authorization to use */
- myextra = (role_auth_extra *) malloc(sizeof(role_auth_extra));
+ myextra = (role_auth_extra *) guc_malloc(LOG, sizeof(role_auth_extra));
if (!myextra)
return false;
myextra->roleid = roleid;
@@ -957,7 +957,7 @@ check_role(char **newval, void **extra, GucSource source)
}
/* Set up "extra" struct for assign_role to use */
- myextra = (role_auth_extra *) malloc(sizeof(role_auth_extra));
+ myextra = (role_auth_extra *) guc_malloc(LOG, sizeof(role_auth_extra));
if (!myextra)
return false;
myextra->roleid = roleid;
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 383bc4776e..30fb576ac3 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -849,8 +849,8 @@ PostmasterMain(int argc, char *argv[])
}
SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
- free(name);
- free(value);
+ pfree(name);
+ pfree(value);
break;
}
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index e360d925b0..1a022b11a0 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -1054,9 +1054,9 @@ check_synchronous_standby_names(char **newval, void **extra, GucSource source)
return false;
}
- /* GUC extra value must be malloc'd, not palloc'd */
+ /* GUC extra value must be guc_malloc'd, not palloc'd */
pconf = (SyncRepConfigData *)
- malloc(syncrep_parse_result->config_size);
+ guc_malloc(LOG, syncrep_parse_result->config_size);
if (pconf == NULL)
return false;
memcpy(pconf, syncrep_parse_result, syncrep_parse_result->config_size);
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 5352d5f4c6..6e744780cc 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3871,8 +3871,8 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
optarg)));
}
SetConfigOption(name, value, ctx, gucsource);
- free(name);
- free(value);
+ pfree(name);
+ pfree(value);
break;
}
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index a8b025f43f..c1559d267c 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -29,6 +29,7 @@
#include "utils/builtins.h"
#include "utils/date.h"
#include "utils/datetime.h"
+#include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/tzparser.h"
@@ -4782,8 +4783,8 @@ TemporalSimplify(int32 max_precis, Node *node)
* to create the final array of timezone tokens. The argument array
* is already sorted in name order.
*
- * The result is a TimeZoneAbbrevTable (which must be a single malloc'd chunk)
- * or NULL on malloc failure. No other error conditions are defined.
+ * The result is a TimeZoneAbbrevTable (which must be a single guc_malloc'd
+ * chunk) or NULL on alloc failure. No other error conditions are defined.
*/
TimeZoneAbbrevTable *
ConvertTimeZoneAbbrevs(struct tzEntry *abbrevs, int n)
@@ -4812,7 +4813,7 @@ ConvertTimeZoneAbbrevs(struct tzEntry *abbrevs, int n)
}
/* Alloc the result ... */
- tbl = malloc(tbl_size);
+ tbl = guc_malloc(LOG, tbl_size);
if (!tbl)
return NULL;
diff --git a/src/backend/utils/cache/ts_cache.c b/src/backend/utils/cache/ts_cache.c
index 890832f353..450ea34336 100644
--- a/src/backend/utils/cache/ts_cache.c
+++ b/src/backend/utils/cache/ts_cache.c
@@ -633,9 +633,9 @@ check_default_text_search_config(char **newval, void **extra, GucSource source)
ReleaseSysCache(tuple);
- /* GUC wants it malloc'd not palloc'd */
- free(*newval);
- *newval = strdup(buf);
+ /* GUC wants it guc_malloc'd not palloc'd */
+ guc_free(*newval);
+ *newval = guc_strdup(LOG, buf);
pfree(buf);
if (!*newval)
return false;
diff --git a/src/backend/utils/misc/README b/src/backend/utils/misc/README
index 6e294386f7..85d97d29b6 100644
--- a/src/backend/utils/misc/README
+++ b/src/backend/utils/misc/README
@@ -51,13 +51,13 @@ out-of-memory.
This might be used for example to canonicalize the spelling of a string
value, round off a buffer size to the nearest supported value, or replace
a special value such as "-1" with a computed default value. If the
-function wishes to replace a string value, it must malloc (not palloc)
-the replacement value, and be sure to free() the previous value.
+function wishes to replace a string value, it must guc_malloc (not palloc)
+the replacement value, and be sure to guc_free() the previous value.
* Derived information, such as the role OID represented by a user name,
-can be stored for use by the assign hook. To do this, malloc (not palloc)
+can be stored for use by the assign hook. To do this, guc_malloc (not palloc)
storage space for the information, and return its address at *extra.
-guc.c will automatically free() this space when the associated GUC setting
+guc.c will automatically guc_free() this space when the associated GUC setting
is no longer of interest. *extra is initialized to NULL before call, so
it can be ignored if not needed.
@@ -255,10 +255,9 @@ maintained by GUC.
GUC Memory Handling
-------------------
-String variable values are allocated with malloc/strdup, not with the
-palloc/pstrdup mechanisms. We would need to keep them in a permanent
-context anyway, and malloc gives us more control over handling
-out-of-memory failures.
+String variable values are allocated with guc_malloc or guc_strdup,
+which ensure that the values are kept in a long-lived context, and provide
+more control over handling out-of-memory failures than bare palloc.
We allow a string variable's actual value, reset_val, boot_val, and stacked
values to point at the same storage. This makes it slightly harder to free
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f997ec0f82..3a4c4814a7 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -188,6 +188,9 @@ static const char *const map_old_guc_names[] = {
};
+/* Memory context holding all GUC-related data */
+static MemoryContext GUCMemoryContext;
+
/*
* Actual lookup of variables is done through this single, sorted array.
*/
@@ -595,19 +598,22 @@ bail_out:
return head;
}
+
/*
- * Some infrastructure for checking malloc/strdup/realloc calls
+ * Some infrastructure for GUC-related memory allocation
+ *
+ * These functions are generally modeled on libc's malloc/realloc/etc,
+ * but any OOM issue is reported at the specified elevel.
+ * (Thus, control returns only if that's less than ERROR.)
*/
void *
guc_malloc(int elevel, size_t size)
{
void *data;
- /* Avoid unportable behavior of malloc(0) */
- if (size == 0)
- size = 1;
- data = malloc(size);
- if (data == NULL)
+ data = MemoryContextAllocExtended(GUCMemoryContext, size,
+ MCXT_ALLOC_NO_OOM);
+ if (unlikely(data == NULL))
ereport(elevel,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
@@ -619,11 +625,20 @@ guc_realloc(int elevel, void *old, size_t size)
{
void *data;
- /* Avoid unportable behavior of realloc(NULL, 0) */
- if (old == NULL && size == 0)
- size = 1;
- data = realloc(old, size);
- if (data == NULL)
+ if (old != NULL)
+ {
+ /* This is to help catch old code that malloc's GUC data. */
+ Assert(GetMemoryChunkContext(old) == GUCMemoryContext);
+ data = repalloc_extended(old, size,
+ MCXT_ALLOC_NO_OOM);
+ }
+ else
+ {
+ /* Like realloc(3), but not like repalloc(), we allow old == NULL. */
+ data = MemoryContextAllocExtended(GUCMemoryContext, size,
+ MCXT_ALLOC_NO_OOM);
+ }
+ if (unlikely(data == NULL))
ereport(elevel,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
@@ -634,15 +649,29 @@ char *
guc_strdup(int elevel, const char *src)
{
char *data;
+ size_t len = strlen(src) + 1;
- data = strdup(src);
- if (data == NULL)
- ereport(elevel,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ data = guc_malloc(elevel, len);
+ if (likely(data != NULL))
+ memcpy(data, src, len);
return data;
}
+void
+guc_free(void *ptr)
+{
+ /*
+ * Historically, GUC-related code has relied heavily on the ability to do
+ * free(NULL), so we allow that here even though pfree() doesn't.
+ */
+ if (ptr != NULL)
+ {
+ /* This is to help catch old code that malloc's GUC data. */
+ Assert(GetMemoryChunkContext(ptr) == GUCMemoryContext);
+ pfree(ptr);
+ }
+}
+
/*
* Detect whether strval is referenced anywhere in a GUC string item
@@ -680,7 +709,7 @@ set_string_field(struct config_string *conf, char **field, char *newval)
/* Free old value if it's not NULL and isn't referenced anymore */
if (oldval && !string_field_used(conf, oldval))
- free(oldval);
+ guc_free(oldval);
}
/*
@@ -741,7 +770,7 @@ set_extra_field(struct config_generic *gconf, void **field, void *newval)
/* Free old value if it's not NULL and isn't referenced anymore */
if (oldval && !extra_field_used(gconf, oldval))
- free(oldval);
+ guc_free(oldval);
}
/*
@@ -749,7 +778,7 @@ set_extra_field(struct config_generic *gconf, void **field, void *newval)
* The "extra" field associated with the active value is copied, too.
*
* NB: be sure stringval and extra fields of a new stack entry are
- * initialized to NULL before this is used, else we'll try to free() them.
+ * initialized to NULL before this is used, else we'll try to guc_free() them.
*/
static void
set_stack_value(struct config_generic *gconf, config_var_value *val)
@@ -817,9 +846,9 @@ get_guc_variables(void)
/*
- * Build the sorted array. This is split out so that it could be
- * re-executed after startup (e.g., we could allow loadable modules to
- * add vars, and then we'd need to re-sort).
+ * Build the sorted array. This is split out so that help_config.c can
+ * extract all the variables without running all of InitializeGUCOptions.
+ * It's not meant for use anyplace else.
*/
void
build_guc_variables(void)
@@ -829,6 +858,17 @@ build_guc_variables(void)
struct config_generic **guc_vars;
int i;
+ /*
+ * Create the memory context that will hold all GUC-related data.
+ */
+ Assert(GUCMemoryContext == NULL);
+ GUCMemoryContext = AllocSetContextCreate(TopMemoryContext,
+ "GUCMemoryContext",
+ ALLOCSET_DEFAULT_SIZES);
+
+ /*
+ * Count all the built-in variables, and set their vartypes correctly.
+ */
for (i = 0; ConfigureNamesBool[i].gen.name; i++)
{
struct config_bool *conf = &ConfigureNamesBool[i];
@@ -895,7 +935,7 @@ build_guc_variables(void)
for (i = 0; ConfigureNamesEnum[i].gen.name; i++)
guc_vars[num_vars++] = &ConfigureNamesEnum[i].gen;
- free(guc_variables);
+ guc_free(guc_variables);
guc_variables = guc_vars;
num_guc_variables = num_vars;
size_guc_variables = size_vars;
@@ -1001,7 +1041,7 @@ add_placeholder_variable(const char *name, int elevel)
gen->name = guc_strdup(elevel, name);
if (gen->name == NULL)
{
- free(var);
+ guc_free(var);
return NULL;
}
@@ -1020,8 +1060,8 @@ add_placeholder_variable(const char *name, int elevel)
if (!add_guc_variable((struct config_generic *) var, elevel))
{
- free(unconstify(char *, gen->name));
- free(var);
+ guc_free(unconstify(char *, gen->name));
+ guc_free(var);
return NULL;
}
@@ -1255,7 +1295,7 @@ InitializeGUCOptions(void)
pg_timezone_initialize();
/*
- * Build sorted array of all GUC variables.
+ * Create GUCMemoryContext and build sorted array of all GUC variables.
*/
build_guc_variables();
@@ -1482,6 +1522,7 @@ SelectConfigFiles(const char *userDoption, const char *progname)
{
char *configdir;
char *fname;
+ bool fname_is_malloced;
struct stat stat_buf;
struct config_string *data_directory_rec;
@@ -1509,12 +1550,16 @@ SelectConfigFiles(const char *userDoption, const char *progname)
* the same way by future backends.
*/
if (ConfigFileName)
+ {
fname = make_absolute_path(ConfigFileName);
+ fname_is_malloced = true;
+ }
else if (configdir)
{
fname = guc_malloc(FATAL,
strlen(configdir) + strlen(CONFIG_FILENAME) + 2);
sprintf(fname, "%s/%s", configdir, CONFIG_FILENAME);
+ fname_is_malloced = false;
}
else
{
@@ -1530,7 +1575,11 @@ SelectConfigFiles(const char *userDoption, const char *progname)
* it can't be overridden later.
*/
SetConfigOption("config_file", fname, PGC_POSTMASTER, PGC_S_OVERRIDE);
- free(fname);
+
+ if (fname_is_malloced)
+ free(fname);
+ else
+ guc_free(fname);
/*
* Now read the config file for the first time.
@@ -1604,12 +1653,16 @@ SelectConfigFiles(const char *userDoption, const char *progname)
* Figure out where pg_hba.conf is, and make sure the path is absolute.
*/
if (HbaFileName)
+ {
fname = make_absolute_path(HbaFileName);
+ fname_is_malloced = true;
+ }
else if (configdir)
{
fname = guc_malloc(FATAL,
strlen(configdir) + strlen(HBA_FILENAME) + 2);
sprintf(fname, "%s/%s", configdir, HBA_FILENAME);
+ fname_is_malloced = false;
}
else
{
@@ -1621,18 +1674,26 @@ SelectConfigFiles(const char *userDoption, const char *progname)
return false;
}
SetConfigOption("hba_file", fname, PGC_POSTMASTER, PGC_S_OVERRIDE);
- free(fname);
+
+ if (fname_is_malloced)
+ free(fname);
+ else
+ guc_free(fname);
/*
* Likewise for pg_ident.conf.
*/
if (IdentFileName)
+ {
fname = make_absolute_path(IdentFileName);
+ fname_is_malloced = true;
+ }
else if (configdir)
{
fname = guc_malloc(FATAL,
strlen(configdir) + strlen(IDENT_FILENAME) + 2);
sprintf(fname, "%s/%s", configdir, IDENT_FILENAME);
+ fname_is_malloced = false;
}
else
{
@@ -1644,7 +1705,11 @@ SelectConfigFiles(const char *userDoption, const char *progname)
return false;
}
SetConfigOption("ident_file", fname, PGC_POSTMASTER, PGC_S_OVERRIDE);
- free(fname);
+
+ if (fname_is_malloced)
+ free(fname);
+ else
+ guc_free(fname);
free(configdir);
@@ -2289,12 +2354,12 @@ ReportGUCOption(struct config_generic *record)
pq_endmessage(&msgbuf);
/*
- * We need a long-lifespan copy. If strdup() fails due to OOM, we'll
- * set last_reported to NULL and thereby possibly make a duplicate
- * report later.
+ * We need a long-lifespan copy. If guc_strdup() fails due to OOM,
+ * we'll set last_reported to NULL and thereby possibly make a
+ * duplicate report later.
*/
- free(record->last_reported);
- record->last_reported = strdup(val);
+ guc_free(record->last_reported);
+ record->last_reported = guc_strdup(LOG, val);
}
pfree(val);
@@ -2893,7 +2958,7 @@ parse_and_validate_value(struct config_generic *record,
if (!call_string_check_hook(conf, &newval->stringval, newextra,
source, elevel))
{
- free(newval->stringval);
+ guc_free(newval->stringval);
newval->stringval = NULL;
return false;
}
@@ -3328,7 +3393,7 @@ set_config_option_ext(const char *name, const char *value,
{
/* Release newextra, unless it's reset_extra */
if (newextra && !extra_field_used(&conf->gen, newextra))
- free(newextra);
+ guc_free(newextra);
if (*conf->variable != newval)
{
@@ -3387,7 +3452,7 @@ set_config_option_ext(const char *name, const char *value,
/* Perhaps we didn't install newextra anywhere */
if (newextra && !extra_field_used(&conf->gen, newextra))
- free(newextra);
+ guc_free(newextra);
break;
#undef newval
@@ -3426,7 +3491,7 @@ set_config_option_ext(const char *name, const char *value,
{
/* Release newextra, unless it's reset_extra */
if (newextra && !extra_field_used(&conf->gen, newextra))
- free(newextra);
+ guc_free(newextra);
if (*conf->variable != newval)
{
@@ -3485,7 +3550,7 @@ set_config_option_ext(const char *name, const char *value,
/* Perhaps we didn't install newextra anywhere */
if (newextra && !extra_field_used(&conf->gen, newextra))
- free(newextra);
+ guc_free(newextra);
break;
#undef newval
@@ -3524,7 +3589,7 @@ set_config_option_ext(const char *name, const char *value,
{
/* Release newextra, unless it's reset_extra */
if (newextra && !extra_field_used(&conf->gen, newextra))
- free(newextra);
+ guc_free(newextra);
if (*conf->variable != newval)
{
@@ -3583,7 +3648,7 @@ set_config_option_ext(const char *name, const char *value,
/* Perhaps we didn't install newextra anywhere */
if (newextra && !extra_field_used(&conf->gen, newextra))
- free(newextra);
+ guc_free(newextra);
break;
#undef newval
@@ -3617,7 +3682,7 @@ set_config_option_ext(const char *name, const char *value,
if (!call_string_check_hook(conf, &newval, &newextra,
source, elevel))
{
- free(newval);
+ guc_free(newval);
return 0;
}
}
@@ -3645,10 +3710,10 @@ set_config_option_ext(const char *name, const char *value,
/* Release newval, unless it's reset_val */
if (newval && !string_field_used(conf, newval))
- free(newval);
+ guc_free(newval);
/* Release newextra, unless it's reset_extra */
if (newextra && !extra_field_used(&conf->gen, newextra))
- free(newextra);
+ guc_free(newextra);
if (newval_different)
{
@@ -3709,10 +3774,10 @@ set_config_option_ext(const char *name, const char *value,
/* Perhaps we didn't install newval anywhere */
if (newval && !string_field_used(conf, newval))
- free(newval);
+ guc_free(newval);
/* Perhaps we didn't install newextra anywhere */
if (newextra && !extra_field_used(&conf->gen, newextra))
- free(newextra);
+ guc_free(newextra);
break;
#undef newval
@@ -3751,7 +3816,7 @@ set_config_option_ext(const char *name, const char *value,
{
/* Release newextra, unless it's reset_extra */
if (newextra && !extra_field_used(&conf->gen, newextra))
- free(newextra);
+ guc_free(newextra);
if (*conf->variable != newval)
{
@@ -3810,7 +3875,7 @@ set_config_option_ext(const char *name, const char *value,
/* Perhaps we didn't install newextra anywhere */
if (newextra && !extra_field_used(&conf->gen, newextra))
- free(newextra);
+ guc_free(newextra);
break;
#undef newval
@@ -3848,7 +3913,7 @@ set_config_sourcefile(const char *name, char *sourcefile, int sourceline)
return;
sourcefile = guc_strdup(elevel, sourcefile);
- free(record->sourcefile);
+ guc_free(record->sourcefile);
record->sourcefile = sourcefile;
record->sourceline = sourceline;
}
@@ -4239,8 +4304,8 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
name, value)));
if (record->vartype == PGC_STRING && newval.stringval != NULL)
- free(newval.stringval);
- free(newextra);
+ guc_free(newval.stringval);
+ guc_free(newextra);
/*
* We must also reject values containing newlines, because the
@@ -4535,7 +4600,7 @@ define_custom_variable(struct config_generic *variable)
set_string_field(pHolder, pHolder->variable, NULL);
set_string_field(pHolder, &pHolder->reset_val, NULL);
- free(pHolder);
+ guc_free(pHolder);
}
/*
@@ -4814,7 +4879,7 @@ MarkGUCPrefixReserved(const char *className)
}
/* And remember the name so we can prevent future mistakes. */
- oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+ oldcontext = MemoryContextSwitchTo(GUCMemoryContext);
reserved_class_prefix = lappend(reserved_class_prefix, pstrdup(className));
MemoryContextSwitchTo(oldcontext);
}
@@ -5287,9 +5352,9 @@ read_nondefault_variables(void)
if (varsourcefile[0])
set_config_sourcefile(varname, varsourcefile, varsourceline);
- free(varname);
- free(varvalue);
- free(varsourcefile);
+ guc_free(varname);
+ guc_free(varvalue);
+ guc_free(varsourcefile);
}
FreeFile(fp);
@@ -5731,9 +5796,9 @@ RestoreGUCState(void *gucstate)
* pointers.
*/
Assert(gconf->stack == NULL);
- free(gconf->extra);
- free(gconf->last_reported);
- free(gconf->sourcefile);
+ guc_free(gconf->extra);
+ guc_free(gconf->last_reported);
+ guc_free(gconf->sourcefile);
switch (gconf->vartype)
{
case PGC_BOOL:
@@ -5741,7 +5806,7 @@ RestoreGUCState(void *gucstate)
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->reset_extra && conf->reset_extra != gconf->extra)
- free(conf->reset_extra);
+ guc_free(conf->reset_extra);
break;
}
case PGC_INT:
@@ -5749,7 +5814,7 @@ RestoreGUCState(void *gucstate)
struct config_int *conf = (struct config_int *) gconf;
if (conf->reset_extra && conf->reset_extra != gconf->extra)
- free(conf->reset_extra);
+ guc_free(conf->reset_extra);
break;
}
case PGC_REAL:
@@ -5757,18 +5822,18 @@ RestoreGUCState(void *gucstate)
struct config_real *conf = (struct config_real *) gconf;
if (conf->reset_extra && conf->reset_extra != gconf->extra)
- free(conf->reset_extra);
+ guc_free(conf->reset_extra);
break;
}
case PGC_STRING:
{
struct config_string *conf = (struct config_string *) gconf;
- free(*conf->variable);
+ guc_free(*conf->variable);
if (conf->reset_val && conf->reset_val != *conf->variable)
- free(conf->reset_val);
+ guc_free(conf->reset_val);
if (conf->reset_extra && conf->reset_extra != gconf->extra)
- free(conf->reset_extra);
+ guc_free(conf->reset_extra);
break;
}
case PGC_ENUM:
@@ -5776,7 +5841,7 @@ RestoreGUCState(void *gucstate)
struct config_enum *conf = (struct config_enum *) gconf;
if (conf->reset_extra && conf->reset_extra != gconf->extra)
- free(conf->reset_extra);
+ guc_free(conf->reset_extra);
break;
}
}
@@ -5838,7 +5903,7 @@ RestoreGUCState(void *gucstate)
/*
* A little "long argument" simulation, although not quite GNU
* compliant. Takes a string of the form "some-option=some value" and
- * returns name = "some_option" and value = "some value" in malloc'ed
+ * returns name = "some_option" and value = "some value" in palloc'ed
* storage. Note that '-' is converted to '_' in the option name. If
* there is no '=' in the input string then value will be NULL.
*/
@@ -5856,15 +5921,15 @@ ParseLongOption(const char *string, char **name, char **value)
if (string[equal_pos] == '=')
{
- *name = guc_malloc(FATAL, equal_pos + 1);
+ *name = palloc(equal_pos + 1);
strlcpy(*name, string, equal_pos + 1);
- *value = guc_strdup(FATAL, &string[equal_pos + 1]);
+ *value = pstrdup(&string[equal_pos + 1]);
}
else
{
/* no equal sign in string */
- *name = guc_strdup(FATAL, string);
+ *name = pstrdup(string);
*value = NULL;
}
@@ -5898,8 +5963,6 @@ ProcessGUCArray(ArrayType *array,
char *s;
char *name;
char *value;
- char *namecopy;
- char *valuecopy;
d = array_ref(array, 1, &i,
-1 /* varlenarray */ ,
@@ -5920,22 +5983,16 @@ ProcessGUCArray(ArrayType *array,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("could not parse setting for parameter \"%s\"",
name)));
- free(name);
+ pfree(name);
continue;
}
- /* free malloc'd strings immediately to avoid leak upon error */
- namecopy = pstrdup(name);
- free(name);
- valuecopy = pstrdup(value);
- free(value);
-
- (void) set_config_option(namecopy, valuecopy,
+ (void) set_config_option(name, value,
context, source,
action, true, 0, false);
- pfree(namecopy);
- pfree(valuecopy);
+ pfree(name);
+ pfree(value);
pfree(s);
}
}
@@ -6399,7 +6456,7 @@ call_string_check_hook(struct config_string *conf, char **newval, void **extra,
}
PG_CATCH();
{
- free(*newval);
+ guc_free(*newval);
PG_RE_THROW();
}
PG_END_TRY();
diff --git a/src/backend/utils/misc/tzparser.c b/src/backend/utils/misc/tzparser.c
index 8f2c95f055..e291cb63b0 100644
--- a/src/backend/utils/misc/tzparser.c
+++ b/src/backend/utils/misc/tzparser.c
@@ -439,7 +439,7 @@ ParseTzFile(const char *filename, int depth,
* load_tzoffsets --- read and parse the specified timezone offset file
*
* On success, return a filled-in TimeZoneAbbrevTable, which must have been
- * malloc'd not palloc'd. On failure, return NULL, using GUC_check_errmsg
+ * guc_malloc'd not palloc'd. On failure, return NULL, using GUC_check_errmsg
* and friends to give details of the problem.
*/
TimeZoneAbbrevTable *
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 1788361974..c8b65c5bb8 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -401,6 +401,7 @@ extern ArrayType *GUCArrayReset(ArrayType *array);
extern void *guc_malloc(int elevel, size_t size);
extern pg_nodiscard void *guc_realloc(int elevel, void *old, size_t size);
extern char *guc_strdup(int elevel, const char *src);
+extern void guc_free(void *ptr);
#ifdef EXEC_BACKEND
extern void write_nondefault_variables(GucContext context);
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index 190d286f1c..5dc334b61b 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -114,7 +114,7 @@ plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSource source)
list_free(elemlist);
}
- myextra = (int *) malloc(sizeof(int));
+ myextra = (int *) guc_malloc(LOG, sizeof(int));
if (!myextra)
return false;
*myextra = extrachecks;
commit b9e3702a9274e95075c96c10d3ea7f516dd20aeb
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri Oct 7 15:13:50 2022 -0400
Replace sorted array of GUC variables with a hash table.
Get rid of bsearch() in favor of hashed lookup. The main
advantage is that it becomes far cheaper to add new GUCs,
since we needn't re-sort the pointer array. Adding N new
GUCs had been O(N^2 log N), but now it's closer to O(N).
Also, merge GetNumConfigOptions() into get_guc_variables(),
because in a world where the set of GUCs isn't fairly static
you really want to consider those two results as correlated.
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 3a4c4814a7..86a39cfcce 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -36,6 +36,7 @@
#include "guc_internal.h"
#include "libpq/pqformat.h"
#include "parser/scansup.h"
+#include "port/pg_bitutils.h"
#include "storage/fd.h"
#include "storage/lwlock.h"
#include "storage/shmem.h"
@@ -192,16 +193,17 @@ static const char *const map_old_guc_names[] = {
static MemoryContext GUCMemoryContext;
/*
- * Actual lookup of variables is done through this single, sorted array.
+ * We use a dynahash table to look up GUCs by name, or to iterate through
+ * all the GUCs. The gucname field is redundant with gucvar->name, but
+ * dynahash makes it too painful to not store the hash key separately.
*/
-static struct config_generic **guc_variables;
-
-/* Current number of variables contained in the vector */
-static int num_guc_variables;
-
-/* Vector capacity */
-static int size_guc_variables;
+typedef struct
+{
+ const char *gucname; /* hash key */
+ struct config_generic *gucvar; /* -> GUC's defining structure */
+} GUCHashEntry;
+static HTAB *guc_hashtab; /* entries are GUCHashEntrys */
static bool guc_dirty; /* true if need to do commit/abort work */
@@ -213,6 +215,8 @@ static int GUCNestLevel = 0; /* 1 when in main transaction */
static int guc_var_compare(const void *a, const void *b);
+static uint32 guc_name_hash(const void *key, Size keysize);
+static int guc_name_match(const void *key1, const void *key2, Size keysize);
static void InitializeGUCOptionsFromEnvironment(void);
static void InitializeOneGUCOption(struct config_generic *gconf);
static void pg_timezone_abbrev_initialize(void);
@@ -262,7 +266,8 @@ ProcessConfigFileInternal(GucContext context, bool applySettings, int elevel)
ConfigVariable *item,
*head,
*tail;
- int i;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
/* Parse the main config file into a list of option names and values */
ConfFileWithError = ConfigFileName;
@@ -337,9 +342,10 @@ ProcessConfigFileInternal(GucContext context, bool applySettings, int elevel)
* need this so that we can tell below which ones have been removed from
* the file since we last processed it.
*/
- for (i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
{
- struct config_generic *gconf = guc_variables[i];
+ struct config_generic *gconf = hentry->gucvar;
gconf->status &= ~GUC_IS_IN_FILE;
}
@@ -423,9 +429,10 @@ ProcessConfigFileInternal(GucContext context, bool applySettings, int elevel)
* boot-time defaults. If such a variable can't be changed after startup,
* report that and continue.
*/
- for (i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
{
- struct config_generic *gconf = guc_variables[i];
+ struct config_generic *gconf = hentry->gucvar;
GucStack *stack;
if (gconf->reset_source != PGC_S_FILE ||
@@ -836,17 +843,38 @@ discard_stack_value(struct config_generic *gconf, config_var_value *val)
/*
- * Fetch the sorted array pointer (exported for help_config.c's use ONLY)
+ * Fetch a palloc'd, sorted array of GUC struct pointers
+ *
+ * The array length is returned into *num_vars.
*/
struct config_generic **
-get_guc_variables(void)
+get_guc_variables(int *num_vars)
{
- return guc_variables;
+ struct config_generic **result;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
+ int i;
+
+ *num_vars = hash_get_num_entries(guc_hashtab);
+ result = palloc(sizeof(struct config_generic *) * *num_vars);
+
+ /* Extract pointers from the hash table */
+ i = 0;
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
+ result[i++] = hentry->gucvar;
+ Assert(i == *num_vars);
+
+ /* Sort by name */
+ qsort(result, *num_vars,
+ sizeof(struct config_generic *), guc_var_compare);
+
+ return result;
}
/*
- * Build the sorted array. This is split out so that help_config.c can
+ * Build the GUC hash table. This is split out so that help_config.c can
* extract all the variables without running all of InitializeGUCOptions.
* It's not meant for use anyplace else.
*/
@@ -855,7 +883,9 @@ build_guc_variables(void)
{
int size_vars;
int num_vars = 0;
- struct config_generic **guc_vars;
+ HASHCTL hash_ctl;
+ GUCHashEntry *hentry;
+ bool found;
int i;
/*
@@ -911,74 +941,106 @@ build_guc_variables(void)
}
/*
- * Create table with 20% slack
+ * Create hash table with 20% slack
*/
size_vars = num_vars + num_vars / 4;
- guc_vars = (struct config_generic **)
- guc_malloc(FATAL, size_vars * sizeof(struct config_generic *));
-
- num_vars = 0;
+ hash_ctl.keysize = sizeof(char *);
+ hash_ctl.entrysize = sizeof(GUCHashEntry);
+ hash_ctl.hash = guc_name_hash;
+ hash_ctl.match = guc_name_match;
+ hash_ctl.hcxt = GUCMemoryContext;
+ guc_hashtab = hash_create("GUC hash table",
+ size_vars,
+ &hash_ctl,
+ HASH_ELEM | HASH_FUNCTION | HASH_COMPARE | HASH_CONTEXT);
for (i = 0; ConfigureNamesBool[i].gen.name; i++)
- guc_vars[num_vars++] = &ConfigureNamesBool[i].gen;
+ {
+ struct config_generic *gucvar = &ConfigureNamesBool[i].gen;
+
+ hentry = (GUCHashEntry *) hash_search(guc_hashtab,
+ &gucvar->name,
+ HASH_ENTER,
+ &found);
+ Assert(!found);
+ hentry->gucvar = gucvar;
+ }
for (i = 0; ConfigureNamesInt[i].gen.name; i++)
- guc_vars[num_vars++] = &ConfigureNamesInt[i].gen;
+ {
+ struct config_generic *gucvar = &ConfigureNamesInt[i].gen;
+
+ hentry = (GUCHashEntry *) hash_search(guc_hashtab,
+ &gucvar->name,
+ HASH_ENTER,
+ &found);
+ Assert(!found);
+ hentry->gucvar = gucvar;
+ }
for (i = 0; ConfigureNamesReal[i].gen.name; i++)
- guc_vars[num_vars++] = &ConfigureNamesReal[i].gen;
+ {
+ struct config_generic *gucvar = &ConfigureNamesReal[i].gen;
+
+ hentry = (GUCHashEntry *) hash_search(guc_hashtab,
+ &gucvar->name,
+ HASH_ENTER,
+ &found);
+ Assert(!found);
+ hentry->gucvar = gucvar;
+ }
for (i = 0; ConfigureNamesString[i].gen.name; i++)
- guc_vars[num_vars++] = &ConfigureNamesString[i].gen;
+ {
+ struct config_generic *gucvar = &ConfigureNamesString[i].gen;
+
+ hentry = (GUCHashEntry *) hash_search(guc_hashtab,
+ &gucvar->name,
+ HASH_ENTER,
+ &found);
+ Assert(!found);
+ hentry->gucvar = gucvar;
+ }
for (i = 0; ConfigureNamesEnum[i].gen.name; i++)
- guc_vars[num_vars++] = &ConfigureNamesEnum[i].gen;
+ {
+ struct config_generic *gucvar = &ConfigureNamesEnum[i].gen;
+
+ hentry = (GUCHashEntry *) hash_search(guc_hashtab,
+ &gucvar->name,
+ HASH_ENTER,
+ &found);
+ Assert(!found);
+ hentry->gucvar = gucvar;
+ }
- guc_free(guc_variables);
- guc_variables = guc_vars;
- num_guc_variables = num_vars;
- size_guc_variables = size_vars;
- qsort((void *) guc_variables, num_guc_variables,
- sizeof(struct config_generic *), guc_var_compare);
+ Assert(num_vars == hash_get_num_entries(guc_hashtab));
}
/*
- * Add a new GUC variable to the list of known variables. The
- * list is expanded if needed.
+ * Add a new GUC variable to the hash of known variables. The
+ * hash is expanded if needed.
*/
static bool
add_guc_variable(struct config_generic *var, int elevel)
{
- if (num_guc_variables + 1 >= size_guc_variables)
+ GUCHashEntry *hentry;
+ bool found;
+
+ hentry = (GUCHashEntry *) hash_search(guc_hashtab,
+ &var->name,
+ HASH_ENTER_NULL,
+ &found);
+ if (unlikely(hentry == NULL))
{
- /*
- * Increase the vector by 25%
- */
- int size_vars = size_guc_variables + size_guc_variables / 4;
- struct config_generic **guc_vars;
-
- if (size_vars == 0)
- {
- size_vars = 100;
- guc_vars = (struct config_generic **)
- guc_malloc(elevel, size_vars * sizeof(struct config_generic *));
- }
- else
- {
- guc_vars = (struct config_generic **)
- guc_realloc(elevel, guc_variables, size_vars * sizeof(struct config_generic *));
- }
-
- if (guc_vars == NULL)
- return false; /* out of memory */
-
- guc_variables = guc_vars;
- size_guc_variables = size_vars;
+ ereport(elevel,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+ return false; /* out of memory */
}
- guc_variables[num_guc_variables++] = var;
- qsort((void *) guc_variables, num_guc_variables,
- sizeof(struct config_generic *), guc_var_compare);
+ Assert(!found);
+ hentry->gucvar = var;
return true;
}
@@ -1087,23 +1149,18 @@ struct config_generic *
find_option(const char *name, bool create_placeholders, bool skip_errors,
int elevel)
{
- const char **key = &name;
- struct config_generic **res;
+ GUCHashEntry *hentry;
int i;
Assert(name);
- /*
- * By equating const char ** with struct config_generic *, we are assuming
- * the name field is first in config_generic.
- */
- res = (struct config_generic **) bsearch((void *) &key,
- (void *) guc_variables,
- num_guc_variables,
- sizeof(struct config_generic *),
- guc_var_compare);
- if (res)
- return *res;
+ /* Look it up using the hash table. */
+ hentry = (GUCHashEntry *) hash_search(guc_hashtab,
+ &name,
+ HASH_FIND,
+ NULL);
+ if (hentry)
+ return hentry->gucvar;
/*
* See if the name is an obsolete name for a variable. We assume that the
@@ -1176,7 +1233,7 @@ find_option(const char *name, bool create_placeholders, bool skip_errors,
/*
- * comparator for qsorting and bsearching guc_variables array
+ * comparator for qsorting an array of GUC pointers
*/
static int
guc_var_compare(const void *a, const void *b)
@@ -1195,7 +1252,7 @@ guc_name_compare(const char *namea, const char *nameb)
{
/*
* The temptation to use strcasecmp() here must be resisted, because the
- * array ordering has to remain stable across setlocale() calls. So, build
+ * hash mapping has to remain stable across setlocale() calls. So, build
* our own with a simple ASCII-only downcasing.
*/
while (*namea && *nameb)
@@ -1217,6 +1274,42 @@ guc_name_compare(const char *namea, const char *nameb)
return 0;
}
+/*
+ * Hash function that's compatible with guc_name_compare
+ */
+static uint32
+guc_name_hash(const void *key, Size keysize)
+{
+ uint32 result = 0;
+ const char *name = *(const char *const *) key;
+
+ while (*name)
+ {
+ char ch = *name++;
+
+ /* Case-fold in the same way as guc_name_compare */
+ if (ch >= 'A' && ch <= 'Z')
+ ch += 'a' - 'A';
+
+ /* Merge into hash ... not very bright, but it needn't be */
+ result = pg_rotate_left32(result, 5);
+ result ^= (uint32) ch;
+ }
+ return result;
+}
+
+/*
+ * Dynahash match function to use in guc_hashtab
+ */
+static int
+guc_name_match(const void *key1, const void *key2, Size keysize)
+{
+ const char *name1 = *(const char *const *) key1;
+ const char *name2 = *(const char *const *) key2;
+
+ return guc_name_compare(name1, name2);
+}
+
/*
* Convert a GUC name to the form that should be used in pg_parameter_acl.
@@ -1286,7 +1379,8 @@ check_GUC_name_for_parameter_acl(const char *name)
void
InitializeGUCOptions(void)
{
- int i;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
/*
* Before log_line_prefix could possibly receive a nonempty setting, make
@@ -1295,7 +1389,7 @@ InitializeGUCOptions(void)
pg_timezone_initialize();
/*
- * Create GUCMemoryContext and build sorted array of all GUC variables.
+ * Create GUCMemoryContext and build hash table of all GUC variables.
*/
build_guc_variables();
@@ -1303,9 +1397,10 @@ InitializeGUCOptions(void)
* Load all variables with their compiled-in defaults, and initialize
* status fields as needed.
*/
- for (i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
{
- InitializeOneGUCOption(guc_variables[i]);
+ InitializeOneGUCOption(hentry->gucvar);
}
guc_dirty = false;
@@ -1740,11 +1835,13 @@ pg_timezone_abbrev_initialize(void)
void
ResetAllOptions(void)
{
- int i;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
- for (i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
{
- struct config_generic *gconf = guc_variables[i];
+ struct config_generic *gconf = hentry->gucvar;
/* Don't reset non-SET-able values */
if (gconf->context != PGC_SUSET &&
@@ -1962,7 +2059,8 @@ void
AtEOXact_GUC(bool isCommit, int nestLevel)
{
bool still_dirty;
- int i;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
/*
* Note: it's possible to get here with GUCNestLevel == nestLevel-1 during
@@ -1981,9 +2079,10 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
}
still_dirty = false;
- for (i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
{
- struct config_generic *gconf = guc_variables[i];
+ struct config_generic *gconf = hentry->gucvar;
GucStack *stack;
/*
@@ -2252,7 +2351,8 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
void
BeginReportingGUCOptions(void)
{
- int i;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
/*
* Don't do anything unless talking to an interactive frontend.
@@ -2275,9 +2375,10 @@ BeginReportingGUCOptions(void)
PGC_INTERNAL, PGC_S_OVERRIDE);
/* Transmit initial values of interesting variables */
- for (i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
{
- struct config_generic *conf = guc_variables[i];
+ struct config_generic *conf = hentry->gucvar;
if (conf->flags & GUC_REPORT)
ReportGUCOption(conf);
@@ -2302,6 +2403,9 @@ BeginReportingGUCOptions(void)
void
ReportChangedGUCOptions(void)
{
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
+
/* Quick exit if not (yet) enabled */
if (!reporting_enabled)
return;
@@ -2321,9 +2425,10 @@ ReportChangedGUCOptions(void)
return;
/* Transmit new values of interesting variables */
- for (int i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
{
- struct config_generic *conf = guc_variables[i];
+ struct config_generic *conf = hentry->gucvar;
if ((conf->flags & GUC_REPORT) && (conf->status & GUC_NEEDS_REPORT))
ReportGUCOption(conf);
@@ -4506,25 +4611,23 @@ init_custom_variable(const char *name,
/*
* Common code for DefineCustomXXXVariable subroutines: insert the new
- * variable into the GUC variable array, replacing any placeholder.
+ * variable into the GUC variable hash, replacing any placeholder.
*/
static void
define_custom_variable(struct config_generic *variable)
{
const char *name = variable->name;
- const char **nameAddr = &name;
+ GUCHashEntry *hentry;
struct config_string *pHolder;
- struct config_generic **res;
/*
* See if there's a placeholder by the same name.
*/
- res = (struct config_generic **) bsearch((void *) &nameAddr,
- (void *) guc_variables,
- num_guc_variables,
- sizeof(struct config_generic *),
- guc_var_compare);
- if (res == NULL)
+ hentry = (GUCHashEntry *) hash_search(guc_hashtab,
+ &name,
+ HASH_FIND,
+ NULL);
+ if (hentry == NULL)
{
/*
* No placeholder to replace, so we can just add it ... but first,
@@ -4538,13 +4641,13 @@ define_custom_variable(struct config_generic *variable)
/*
* This better be a placeholder
*/
- if (((*res)->flags & GUC_CUSTOM_PLACEHOLDER) == 0)
+ if ((hentry->gucvar->flags & GUC_CUSTOM_PLACEHOLDER) == 0)
ereport(ERROR,
(errcode(ERRCODE_INTERNAL_ERROR),
errmsg("attempt to redefine parameter \"%s\"", name)));
- Assert((*res)->vartype == PGC_STRING);
- pHolder = (struct config_string *) (*res);
+ Assert(hentry->gucvar->vartype == PGC_STRING);
+ pHolder = (struct config_string *) hentry->gucvar;
/*
* First, set the variable to its default value. We must do this even
@@ -4554,10 +4657,11 @@ define_custom_variable(struct config_generic *variable)
InitializeOneGUCOption(variable);
/*
- * Replace the placeholder. We aren't changing the name, so no re-sorting
- * is necessary
+ * Replace the placeholder in the hash table. We aren't changing the name
+ * (at least up to case-folding), so the hash value is unchanged.
*/
- *res = variable;
+ hentry->gucname = name;
+ hentry->gucvar = variable;
/*
* Assign the string value(s) stored in the placeholder to the real
@@ -4849,7 +4953,8 @@ void
MarkGUCPrefixReserved(const char *className)
{
int classLen = strlen(className);
- int i;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
MemoryContext oldcontext;
/*
@@ -4858,9 +4963,10 @@ MarkGUCPrefixReserved(const char *className)
* don't bother trying to free associated memory, since this shouldn't
* happen often.)
*/
- for (i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
{
- struct config_generic *var = guc_variables[i];
+ struct config_generic *var = hentry->gucvar;
if ((var->flags & GUC_CUSTOM_PLACEHOLDER) != 0 &&
strncmp(className, var->name, classLen) == 0 &&
@@ -4872,9 +4978,10 @@ MarkGUCPrefixReserved(const char *className)
var->name),
errdetail("\"%s\" is now a reserved prefix.",
className)));
- num_guc_variables--;
- memmove(&guc_variables[i], &guc_variables[i + 1],
- (num_guc_variables - i) * sizeof(struct config_generic *));
+ hash_search(guc_hashtab,
+ &var->name,
+ HASH_REMOVE,
+ NULL);
}
}
@@ -4895,6 +5002,8 @@ struct config_generic **
get_explain_guc_options(int *num)
{
struct config_generic **result;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
*num = 0;
@@ -4902,12 +5011,13 @@ get_explain_guc_options(int *num)
* While only a fraction of all the GUC variables are marked GUC_EXPLAIN,
* it doesn't seem worth dynamically resizing this array.
*/
- result = palloc(sizeof(struct config_generic *) * num_guc_variables);
+ result = palloc(sizeof(struct config_generic *) * hash_get_num_entries(guc_hashtab));
- for (int i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
{
+ struct config_generic *conf = hentry->gucvar;
bool modified;
- struct config_generic *conf = guc_variables[i];
/* return only parameters marked for inclusion in explain */
if (!(conf->flags & GUC_EXPLAIN))
@@ -5010,15 +5120,6 @@ GetConfigOptionByName(const char *name, const char **varname, bool missing_ok)
return ShowGUCOption(record, true);
}
-/*
- * Return the total number of GUC variables
- */
-int
-GetNumConfigOptions(void)
-{
- return num_guc_variables;
-}
-
/*
* ShowGUCOption: get string value of variable
*
@@ -5220,7 +5321,8 @@ write_nondefault_variables(GucContext context)
{
int elevel;
FILE *fp;
- int i;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
@@ -5239,9 +5341,10 @@ write_nondefault_variables(GucContext context)
return;
}
- for (i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
{
- write_one_nondefault_variable(fp, guc_variables[i]);
+ write_one_nondefault_variable(fp, hentry->gucvar);
}
if (FreeFile(fp))
@@ -5515,15 +5618,17 @@ Size
EstimateGUCStateSpace(void)
{
Size size;
- int i;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
/* Add space reqd for saving the data size of the guc state */
size = sizeof(Size);
/* Add up the space needed for each GUC variable */
- for (i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
size = add_size(size,
- estimate_variable_size(guc_variables[i]));
+ estimate_variable_size(hentry->gucvar));
return size;
}
@@ -5662,15 +5767,17 @@ SerializeGUCState(Size maxsize, char *start_address)
char *curptr;
Size actual_size;
Size bytes_left;
- int i;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
/* Reserve space for saving the actual size of the guc state */
Assert(maxsize > sizeof(actual_size));
curptr = start_address + sizeof(actual_size);
bytes_left = maxsize - sizeof(actual_size);
- for (i = 0; i < num_guc_variables; i++)
- serialize_variable(&curptr, &bytes_left, guc_variables[i]);
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
+ serialize_variable(&curptr, &bytes_left, hentry->gucvar);
/* Store actual size without assuming alignment of start_address. */
actual_size = maxsize - bytes_left - sizeof(actual_size);
@@ -5755,7 +5862,8 @@ RestoreGUCState(void *gucstate)
char *srcptr = (char *) gucstate;
char *srcend;
Size len;
- int i;
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
ErrorContextCallback error_context_callback;
/*
@@ -5780,9 +5888,10 @@ RestoreGUCState(void *gucstate)
* also ensures that set_config_option won't refuse to set them because of
* source-priority comparisons.
*/
- for (i = 0; i < num_guc_variables; i++)
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
{
- struct config_generic *gconf = guc_variables[i];
+ struct config_generic *gconf = hentry->gucvar;
/* Do nothing if non-shippable or if already at PGC_S_DEFAULT. */
if (can_skip_gucvar(gconf))
diff --git a/src/backend/utils/misc/guc_funcs.c b/src/backend/utils/misc/guc_funcs.c
index ffc71726f9..fb763df5fe 100644
--- a/src/backend/utils/misc/guc_funcs.c
+++ b/src/backend/utils/misc/guc_funcs.c
@@ -455,13 +455,15 @@ ShowGUCConfigOption(const char *name, DestReceiver *dest)
static void
ShowAllGUCConfig(DestReceiver *dest)
{
- int i;
+ struct config_generic **guc_vars;
+ int num_vars;
TupOutputState *tstate;
TupleDesc tupdesc;
Datum values[3];
bool isnull[3] = {false, false, false};
- struct config_generic **guc_variables = get_guc_variables();
- int num_guc_variables = GetNumConfigOptions();
+
+ /* collect the variables, in sorted order */
+ guc_vars = get_guc_variables(&num_vars);
/* need a tuple descriptor representing three TEXT columns */
tupdesc = CreateTemplateTupleDesc(3);
@@ -475,9 +477,9 @@ ShowAllGUCConfig(DestReceiver *dest)
/* prepare for projection of tuples */
tstate = begin_tup_output_tupdesc(dest, tupdesc, &TTSOpsVirtual);
- for (i = 0; i < num_guc_variables; i++)
+ for (int i = 0; i < num_vars; i++)
{
- struct config_generic *conf = guc_variables[i];
+ struct config_generic *conf = guc_vars[i];
char *setting;
if ((conf->flags & GUC_NO_SHOW_ALL) ||
@@ -570,20 +572,13 @@ pg_settings_get_flags(PG_FUNCTION_ARGS)
}
/*
- * Return GUC variable value by variable number; optionally return canonical
- * form of name. Return value is palloc'd.
+ * Extract fields to show in pg_settings for given variable.
*/
static void
-GetConfigOptionByNum(int varnum, const char **values, bool *noshow)
+GetConfigOptionValues(struct config_generic *conf, const char **values,
+ bool *noshow)
{
char buffer[256];
- struct config_generic *conf;
- struct config_generic **guc_variables = get_guc_variables();
-
- /* check requested variable number valid */
- Assert((varnum >= 0) && (varnum < GetNumConfigOptions()));
-
- conf = guc_variables[varnum];
if (noshow)
{
@@ -849,6 +844,8 @@ Datum
show_all_settings(PG_FUNCTION_ARGS)
{
FuncCallContext *funcctx;
+ struct config_generic **guc_vars;
+ int num_vars;
TupleDesc tupdesc;
int call_cntr;
int max_calls;
@@ -913,8 +910,14 @@ show_all_settings(PG_FUNCTION_ARGS)
attinmeta = TupleDescGetAttInMetadata(tupdesc);
funcctx->attinmeta = attinmeta;
+ /* collect the variables, in sorted order */
+ guc_vars = get_guc_variables(&num_vars);
+
+ /* use user_fctx to remember the array location */
+ funcctx->user_fctx = guc_vars;
+
/* total number of tuples to be returned */
- funcctx->max_calls = GetNumConfigOptions();
+ funcctx->max_calls = num_vars;
MemoryContextSwitchTo(oldcontext);
}
@@ -922,6 +925,7 @@ show_all_settings(PG_FUNCTION_ARGS)
/* stuff done on every call of the function */
funcctx = SRF_PERCALL_SETUP();
+ guc_vars = (struct config_generic **) funcctx->user_fctx;
call_cntr = funcctx->call_cntr;
max_calls = funcctx->max_calls;
attinmeta = funcctx->attinmeta;
@@ -938,7 +942,8 @@ show_all_settings(PG_FUNCTION_ARGS)
*/
do
{
- GetConfigOptionByNum(call_cntr, (const char **) values, &noshow);
+ GetConfigOptionValues(guc_vars[call_cntr], (const char **) values,
+ &noshow);
if (noshow)
{
/* bump the counter and get the next config setting */
diff --git a/src/backend/utils/misc/help_config.c b/src/backend/utils/misc/help_config.c
index 61c83f3590..59d2e36548 100644
--- a/src/backend/utils/misc/help_config.c
+++ b/src/backend/utils/misc/help_config.c
@@ -49,11 +49,10 @@ GucInfoMain(void)
int numOpts,
i;
- /* Initialize the guc_variables[] array */
+ /* Initialize the GUC hash table */
build_guc_variables();
- guc_vars = get_guc_variables();
- numOpts = GetNumConfigOptions();
+ guc_vars = get_guc_variables(&numOpts);
for (i = 0; i < numOpts; i++)
{
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index c8b65c5bb8..b3aaff9665 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -390,7 +390,6 @@ extern int set_config_option_ext(const char *name, const char *value,
extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt);
extern char *GetConfigOptionByName(const char *name, const char **varname,
bool missing_ok);
-extern int GetNumConfigOptions(void);
extern void ProcessGUCArray(ArrayType *array,
GucContext context, GucSource source, GucAction action);
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index b3d2a959c3..99740e7e48 100644
--- a/src/include/utils/guc_tables.h
+++ b/src/include/utils/guc_tables.h
@@ -281,7 +281,7 @@ extern struct config_generic **get_explain_guc_options(int *num);
extern char *ShowGUCOption(struct config_generic *record, bool use_units);
/* get the current set of variables */
-extern struct config_generic **get_guc_variables(void);
+extern struct config_generic **get_guc_variables(int *num_vars);
extern void build_guc_variables(void);
commit 9593237fde085e0720b4751346a7dbece1a72e15
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri Oct 7 15:19:15 2022 -0400
Add auxiliary lists to GUC data structures for performance.
The previous patch made addition of new GUCs cheap, but other
GUC operations aren't improved and indeed get a bit slower,
because hash_seq_search() is slower than just scanning a
pointer array.
However, most performance-critical GUC operations only need
to touch a relatively small fraction of the GUCs; especially
so for AtEOXact_GUC(). We can improve matters at the cost
of a bit more space by adding dlist or slist links to the
GUC data structures. This patch invents lists that track
(1) all GUCs with non-default "source";
(2) all GUCs with nonempty state stack (implying they've
been changed in the current transaction);
(3) all GUCs due for reporting to the client.
All of the performance-critical cases can make use of one
or another of these lists to avoid searching the hash table.
In particular, the stack list means that transaction end
doesn't take time proportional to the number of GUCs, but
only to the number changed in the current transaction.
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 86a39cfcce..6f21752b84 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -205,12 +205,23 @@ typedef struct
static HTAB *guc_hashtab; /* entries are GUCHashEntrys */
-static bool guc_dirty; /* true if need to do commit/abort work */
+/*
+ * In addition to the hash table, variables having certain properties are
+ * linked into these lists, so that we can find them without scanning the
+ * whole hash table. In most applications, only a small fraction of the
+ * GUCs appear in these lists at any given time. The usage of the stack
+ * and report lists is stylized enough that they can be slists, but the
+ * nondef list has to be a dlist to avoid O(N) deletes in common cases.
+ */
+static dlist_head guc_nondef_list; /* list of variables that have source
+ * different from PGC_S_DEFAULT */
+static slist_head guc_stack_list; /* list of variables that have non-NULL
+ * stack */
+static slist_head guc_report_list; /* list of variables that have the
+ * GUC_NEEDS_REPORT bit set in status */
static bool reporting_enabled; /* true to enable GUC_REPORT */
-static bool report_needed; /* true if any GUC_REPORT reports are needed */
-
static int GUCNestLevel = 0; /* 1 when in main transaction */
@@ -219,6 +230,8 @@ static uint32 guc_name_hash(const void *key, Size keysize);
static int guc_name_match(const void *key1, const void *key2, Size keysize);
static void InitializeGUCOptionsFromEnvironment(void);
static void InitializeOneGUCOption(struct config_generic *gconf);
+static void RemoveGUCFromLists(struct config_generic *gconf);
+static void set_guc_source(struct config_generic *gconf, GucSource newsource);
static void pg_timezone_abbrev_initialize(void);
static void push_old_value(struct config_generic *gconf, GucAction action);
static void ReportGUCOption(struct config_generic *record);
@@ -465,7 +478,7 @@ ProcessConfigFileInternal(GucContext context, bool applySettings, int elevel)
if (gconf->reset_source == PGC_S_FILE)
gconf->reset_source = PGC_S_DEFAULT;
if (gconf->source == PGC_S_FILE)
- gconf->source = PGC_S_DEFAULT;
+ set_guc_source(gconf, PGC_S_DEFAULT);
for (stack = gconf->stack; stack; stack = stack->prev)
{
if (stack->source == PGC_S_FILE)
@@ -1403,8 +1416,6 @@ InitializeGUCOptions(void)
InitializeOneGUCOption(hentry->gucvar);
}
- guc_dirty = false;
-
reporting_enabled = false;
/*
@@ -1600,6 +1611,23 @@ InitializeOneGUCOption(struct config_generic *gconf)
}
}
+/*
+ * Summarily remove a GUC variable from any linked lists it's in.
+ *
+ * We use this in cases where the variable is about to be deleted or reset.
+ * These aren't common operations, so it's okay if this is a bit slow.
+ */
+static void
+RemoveGUCFromLists(struct config_generic *gconf)
+{
+ if (gconf->source != PGC_S_DEFAULT)
+ dlist_delete(&gconf->nondef_link);
+ if (gconf->stack != NULL)
+ slist_delete(&guc_stack_list, &gconf->stack_link);
+ if (gconf->status & GUC_NEEDS_REPORT)
+ slist_delete(&guc_report_list, &gconf->report_link);
+}
+
/*
* Select the configuration files and data directory to be used, and
@@ -1835,13 +1863,13 @@ pg_timezone_abbrev_initialize(void)
void
ResetAllOptions(void)
{
- HASH_SEQ_STATUS status;
- GUCHashEntry *hentry;
+ dlist_mutable_iter iter;
- hash_seq_init(&status, guc_hashtab);
- while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
+ /* We need only consider GUCs not already at PGC_S_DEFAULT */
+ dlist_foreach_modify(iter, &guc_nondef_list)
{
- struct config_generic *gconf = hentry->gucvar;
+ struct config_generic *gconf = dlist_container(struct config_generic,
+ nondef_link, iter.cur);
/* Don't reset non-SET-able values */
if (gconf->context != PGC_SUSET &&
@@ -1921,19 +1949,44 @@ ResetAllOptions(void)
}
}
- gconf->source = gconf->reset_source;
+ set_guc_source(gconf, gconf->reset_source);
gconf->scontext = gconf->reset_scontext;
gconf->srole = gconf->reset_srole;
- if (gconf->flags & GUC_REPORT)
+ if ((gconf->flags & GUC_REPORT) && !(gconf->status & GUC_NEEDS_REPORT))
{
gconf->status |= GUC_NEEDS_REPORT;
- report_needed = true;
+ slist_push_head(&guc_report_list, &gconf->report_link);
}
}
}
+/*
+ * Apply a change to a GUC variable's "source" field.
+ *
+ * Use this rather than just assigning, to ensure that the variable's
+ * membership in guc_nondef_list is updated correctly.
+ */
+static void
+set_guc_source(struct config_generic *gconf, GucSource newsource)
+{
+ /* Adjust nondef list membership if appropriate for change */
+ if (gconf->source == PGC_S_DEFAULT)
+ {
+ if (newsource != PGC_S_DEFAULT)
+ dlist_push_tail(&guc_nondef_list, &gconf->nondef_link);
+ }
+ else
+ {
+ if (newsource == PGC_S_DEFAULT)
+ dlist_delete(&gconf->nondef_link);
+ }
+ /* Now update the source field */
+ gconf->source = newsource;
+}
+
+
/*
* push_old_value
* Push previous state during transactional assignment to a GUC variable.
@@ -1980,7 +2033,6 @@ push_old_value(struct config_generic *gconf, GucAction action)
Assert(stack->state == GUC_SAVE);
break;
}
- Assert(guc_dirty); /* must be set already */
return;
}
@@ -2011,10 +2063,9 @@ push_old_value(struct config_generic *gconf, GucAction action)
stack->srole = gconf->srole;
set_stack_value(gconf, &stack->prior);
+ if (gconf->stack == NULL)
+ slist_push_head(&guc_stack_list, &gconf->stack_link);
gconf->stack = stack;
-
- /* Ensure we remember to pop at end of xact */
- guc_dirty = true;
}
@@ -2058,9 +2109,7 @@ NewGUCNestLevel(void)
void
AtEOXact_GUC(bool isCommit, int nestLevel)
{
- bool still_dirty;
- HASH_SEQ_STATUS status;
- GUCHashEntry *hentry;
+ slist_mutable_iter iter;
/*
* Note: it's possible to get here with GUCNestLevel == nestLevel-1 during
@@ -2071,18 +2120,11 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
(nestLevel <= GUCNestLevel ||
(nestLevel == GUCNestLevel + 1 && !isCommit)));
- /* Quick exit if nothing's changed in this transaction */
- if (!guc_dirty)
+ /* We need only process GUCs having nonempty stacks */
+ slist_foreach_modify(iter, &guc_stack_list)
{
- GUCNestLevel = nestLevel - 1;
- return;
- }
-
- still_dirty = false;
- hash_seq_init(&status, guc_hashtab);
- while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
- {
- struct config_generic *gconf = hentry->gucvar;
+ struct config_generic *gconf = slist_container(struct config_generic,
+ stack_link, iter.cur);
GucStack *stack;
/*
@@ -2315,30 +2357,30 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
set_extra_field(gconf, &(stack->masked.extra), NULL);
/* And restore source information */
- gconf->source = newsource;
+ set_guc_source(gconf, newsource);
gconf->scontext = newscontext;
gconf->srole = newsrole;
}
- /* Finish popping the state stack */
+ /*
+ * Pop the GUC's state stack; if it's now empty, remove the GUC
+ * from guc_stack_list.
+ */
gconf->stack = prev;
+ if (prev == NULL)
+ slist_delete_current(&iter);
pfree(stack);
/* Report new value if we changed it */
- if (changed && (gconf->flags & GUC_REPORT))
+ if (changed && (gconf->flags & GUC_REPORT) &&
+ !(gconf->status & GUC_NEEDS_REPORT))
{
gconf->status |= GUC_NEEDS_REPORT;
- report_needed = true;
+ slist_push_head(&guc_report_list, &gconf->report_link);
}
} /* end of stack-popping loop */
-
- if (stack != NULL)
- still_dirty = true;
}
- /* If there are no remaining stack entries, we can reset guc_dirty */
- guc_dirty = still_dirty;
-
/* Update nesting level */
GUCNestLevel = nestLevel - 1;
}
@@ -2383,8 +2425,6 @@ BeginReportingGUCOptions(void)
if (conf->flags & GUC_REPORT)
ReportGUCOption(conf);
}
-
- report_needed = false;
}
/*
@@ -2403,8 +2443,7 @@ BeginReportingGUCOptions(void)
void
ReportChangedGUCOptions(void)
{
- HASH_SEQ_STATUS status;
- GUCHashEntry *hentry;
+ slist_mutable_iter iter;
/* Quick exit if not (yet) enabled */
if (!reporting_enabled)
@@ -2420,28 +2459,24 @@ ReportChangedGUCOptions(void)
SetConfigOption("in_hot_standby", "false",
PGC_INTERNAL, PGC_S_OVERRIDE);
- /* Quick exit if no values have been changed */
- if (!report_needed)
- return;
-
/* Transmit new values of interesting variables */
- hash_seq_init(&status, guc_hashtab);
- while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
+ slist_foreach_modify(iter, &guc_report_list)
{
- struct config_generic *conf = hentry->gucvar;
+ struct config_generic *conf = slist_container(struct config_generic,
+ report_link, iter.cur);
- if ((conf->flags & GUC_REPORT) && (conf->status & GUC_NEEDS_REPORT))
- ReportGUCOption(conf);
+ Assert((conf->flags & GUC_REPORT) && (conf->status & GUC_NEEDS_REPORT));
+ ReportGUCOption(conf);
+ conf->status &= ~GUC_NEEDS_REPORT;
+ slist_delete_current(&iter);
}
-
- report_needed = false;
}
/*
* ReportGUCOption: if appropriate, transmit option value to frontend
*
* We need not transmit the value if it's the same as what we last
- * transmitted. However, clear the NEEDS_REPORT flag in any case.
+ * transmitted.
*/
static void
ReportGUCOption(struct config_generic *record)
@@ -2468,8 +2503,6 @@ ReportGUCOption(struct config_generic *record)
}
pfree(val);
-
- record->status &= ~GUC_NEEDS_REPORT;
}
/*
@@ -3524,7 +3557,7 @@ set_config_option_ext(const char *name, const char *value,
*conf->variable = newval;
set_extra_field(&conf->gen, &conf->gen.extra,
newextra);
- conf->gen.source = source;
+ set_guc_source(&conf->gen, source);
conf->gen.scontext = context;
conf->gen.srole = srole;
}
@@ -3622,7 +3655,7 @@ set_config_option_ext(const char *name, const char *value,
*conf->variable = newval;
set_extra_field(&conf->gen, &conf->gen.extra,
newextra);
- conf->gen.source = source;
+ set_guc_source(&conf->gen, source);
conf->gen.scontext = context;
conf->gen.srole = srole;
}
@@ -3720,7 +3753,7 @@ set_config_option_ext(const char *name, const char *value,
*conf->variable = newval;
set_extra_field(&conf->gen, &conf->gen.extra,
newextra);
- conf->gen.source = source;
+ set_guc_source(&conf->gen, source);
conf->gen.scontext = context;
conf->gen.srole = srole;
}
@@ -3844,7 +3877,7 @@ set_config_option_ext(const char *name, const char *value,
set_string_field(conf, conf->variable, newval);
set_extra_field(&conf->gen, &conf->gen.extra,
newextra);
- conf->gen.source = source;
+ set_guc_source(&conf->gen, source);
conf->gen.scontext = context;
conf->gen.srole = srole;
}
@@ -3947,7 +3980,7 @@ set_config_option_ext(const char *name, const char *value,
*conf->variable = newval;
set_extra_field(&conf->gen, &conf->gen.extra,
newextra);
- conf->gen.source = source;
+ set_guc_source(&conf->gen, source);
conf->gen.scontext = context;
conf->gen.srole = srole;
}
@@ -3987,10 +4020,11 @@ set_config_option_ext(const char *name, const char *value,
}
}
- if (changeVal && (record->flags & GUC_REPORT))
+ if (changeVal && (record->flags & GUC_REPORT) &&
+ !(record->status & GUC_NEEDS_REPORT))
{
record->status |= GUC_NEEDS_REPORT;
- report_needed = true;
+ slist_push_head(&guc_report_list, &record->report_link);
}
return changeVal ? 1 : -1;
@@ -4663,6 +4697,11 @@ define_custom_variable(struct config_generic *variable)
hentry->gucname = name;
hentry->gucvar = variable;
+ /*
+ * Remove the placeholder from any lists it's in, too.
+ */
+ RemoveGUCFromLists(&pHolder->gen);
+
/*
* Assign the string value(s) stored in the placeholder to the real
* variable. Essentially, we need to duplicate all the active and stacked
@@ -4794,7 +4833,11 @@ reapply_stacked_values(struct config_generic *variable,
(void) set_config_option_ext(name, curvalue,
curscontext, cursource, cursrole,
GUC_ACTION_SET, true, WARNING, false);
- variable->stack = NULL;
+ if (variable->stack != NULL)
+ {
+ slist_delete(&guc_stack_list, &variable->stack_link);
+ variable->stack = NULL;
+ }
}
}
}
@@ -4978,10 +5021,13 @@ MarkGUCPrefixReserved(const char *className)
var->name),
errdetail("\"%s\" is now a reserved prefix.",
className)));
+ /* Remove it from the hash table */
hash_search(guc_hashtab,
&var->name,
HASH_REMOVE,
NULL);
+ /* Remove it from any lists it's in, too */
+ RemoveGUCFromLists(var);
}
}
@@ -5002,8 +5048,7 @@ struct config_generic **
get_explain_guc_options(int *num)
{
struct config_generic **result;
- HASH_SEQ_STATUS status;
- GUCHashEntry *hentry;
+ dlist_iter iter;
*num = 0;
@@ -5013,10 +5058,11 @@ get_explain_guc_options(int *num)
*/
result = palloc(sizeof(struct config_generic *) * hash_get_num_entries(guc_hashtab));
- hash_seq_init(&status, guc_hashtab);
- while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
+ /* We need only consider GUCs with source not PGC_S_DEFAULT */
+ dlist_foreach(iter, &guc_nondef_list)
{
- struct config_generic *conf = hentry->gucvar;
+ struct config_generic *conf = dlist_container(struct config_generic,
+ nondef_link, iter.cur);
bool modified;
/* return only parameters marked for inclusion in explain */
@@ -5251,8 +5297,7 @@ ShowGUCOption(struct config_generic *record, bool use_units)
static void
write_one_nondefault_variable(FILE *fp, struct config_generic *gconf)
{
- if (gconf->source == PGC_S_DEFAULT)
- return;
+ Assert(gconf->source != PGC_S_DEFAULT);
fprintf(fp, "%s", gconf->name);
fputc(0, fp);
@@ -5321,8 +5366,7 @@ write_nondefault_variables(GucContext context)
{
int elevel;
FILE *fp;
- HASH_SEQ_STATUS status;
- GUCHashEntry *hentry;
+ dlist_iter iter;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
@@ -5341,10 +5385,13 @@ write_nondefault_variables(GucContext context)
return;
}
- hash_seq_init(&status, guc_hashtab);
- while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
+ /* We need only consider GUCs with source not PGC_S_DEFAULT */
+ dlist_foreach(iter, &guc_nondef_list)
{
- write_one_nondefault_variable(fp, hentry->gucvar);
+ struct config_generic *gconf = dlist_container(struct config_generic,
+ nondef_link, iter.cur);
+
+ write_one_nondefault_variable(fp, gconf);
}
if (FreeFile(fp))
@@ -5618,17 +5665,23 @@ Size
EstimateGUCStateSpace(void)
{
Size size;
- HASH_SEQ_STATUS status;
- GUCHashEntry *hentry;
+ dlist_iter iter;
/* Add space reqd for saving the data size of the guc state */
size = sizeof(Size);
- /* Add up the space needed for each GUC variable */
- hash_seq_init(&status, guc_hashtab);
- while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
- size = add_size(size,
- estimate_variable_size(hentry->gucvar));
+ /*
+ * Add up the space needed for each GUC variable.
+ *
+ * We need only process non-default GUCs.
+ */
+ dlist_foreach(iter, &guc_nondef_list)
+ {
+ struct config_generic *gconf = dlist_container(struct config_generic,
+ nondef_link, iter.cur);
+
+ size = add_size(size, estimate_variable_size(gconf));
+ }
return size;
}
@@ -5767,17 +5820,21 @@ SerializeGUCState(Size maxsize, char *start_address)
char *curptr;
Size actual_size;
Size bytes_left;
- HASH_SEQ_STATUS status;
- GUCHashEntry *hentry;
+ dlist_iter iter;
/* Reserve space for saving the actual size of the guc state */
Assert(maxsize > sizeof(actual_size));
curptr = start_address + sizeof(actual_size);
bytes_left = maxsize - sizeof(actual_size);
- hash_seq_init(&status, guc_hashtab);
- while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
- serialize_variable(&curptr, &bytes_left, hentry->gucvar);
+ /* We need only consider GUCs with source not PGC_S_DEFAULT */
+ dlist_foreach(iter, &guc_nondef_list)
+ {
+ struct config_generic *gconf = dlist_container(struct config_generic,
+ nondef_link, iter.cur);
+
+ serialize_variable(&curptr, &bytes_left, gconf);
+ }
/* Store actual size without assuming alignment of start_address. */
actual_size = maxsize - bytes_left - sizeof(actual_size);
@@ -5862,8 +5919,7 @@ RestoreGUCState(void *gucstate)
char *srcptr = (char *) gucstate;
char *srcend;
Size len;
- HASH_SEQ_STATUS status;
- GUCHashEntry *hentry;
+ dlist_mutable_iter iter;
ErrorContextCallback error_context_callback;
/*
@@ -5888,10 +5944,10 @@ RestoreGUCState(void *gucstate)
* also ensures that set_config_option won't refuse to set them because of
* source-priority comparisons.
*/
- hash_seq_init(&status, guc_hashtab);
- while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
+ dlist_foreach_modify(iter, &guc_nondef_list)
{
- struct config_generic *gconf = hentry->gucvar;
+ struct config_generic *gconf = dlist_container(struct config_generic,
+ nondef_link, iter.cur);
/* Do nothing if non-shippable or if already at PGC_S_DEFAULT. */
if (can_skip_gucvar(gconf))
@@ -5902,7 +5958,8 @@ RestoreGUCState(void *gucstate)
* first we must free any existing subsidiary data to avoid leaking
* memory. The stack must be empty, but we have to clean up all other
* fields. Beware that there might be duplicate value or "extra"
- * pointers.
+ * pointers. We also have to be sure to take it out of any lists it's
+ * in.
*/
Assert(gconf->stack == NULL);
guc_free(gconf->extra);
@@ -5954,6 +6011,8 @@ RestoreGUCState(void *gucstate)
break;
}
}
+ /* Remove it from any lists it's in. */
+ RemoveGUCFromLists(gconf);
/* Now we can reset the struct to PGS_S_DEFAULT state. */
InitializeOneGUCOption(gconf);
}
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index 99740e7e48..51f7f09fde 100644
--- a/src/include/utils/guc_tables.h
+++ b/src/include/utils/guc_tables.h
@@ -14,6 +14,7 @@
#ifndef GUC_TABLES_H
#define GUC_TABLES_H 1
+#include "lib/ilist.h"
#include "utils/guc.h"
/*
@@ -138,6 +139,11 @@ typedef struct guc_stack
* if the value came from an internal source or the config file. Similarly
* for reset_srole (which is usually BOOTSTRAP_SUPERUSERID, but not always).
*
+ * Variables that are currently of active interest for maintenance
+ * operations are linked into various lists using the xxx_link fields.
+ * The link fields are unused/garbage in variables not currently having
+ * the specified properties.
+ *
* Note that sourcefile/sourceline are kept here, and not pushed into stacked
* values, although in principle they belong with some stacked value if the
* active value is session- or transaction-local. This is to avoid bloating
@@ -163,6 +169,12 @@ struct config_generic
Oid reset_srole; /* role that set the reset value */
GucStack *stack; /* stacked prior values */
void *extra; /* "extra" pointer for current actual value */
+ dlist_node nondef_link; /* list link for variables that have source
+ * different from PGC_S_DEFAULT */
+ slist_node stack_link; /* list link for variables that have non-NULL
+ * stack */
+ slist_node report_link; /* list link for variables that have the
+ * GUC_NEEDS_REPORT bit set in status */
char *last_reported; /* if variable is GUC_REPORT, value last sent
* to client (NULL if not yet sent) */
char *sourcefile; /* file current setting is from (NULL if not
pgsql-hackers by date: