Backend memory dump analysis
Hi,
I investigate an out of memory-related case for PostgreSQL 9.6.5, and it
looks like MemoryContextStatsDetail + gdb are the only friends there.
MemoryContextStatsDetail does print some info, however it is rarely
possible to associate the used memory with business cases.
For insance:
CachedPlanSource: 146224 total in 8 blocks; 59768 free (3 chunks); 86456
used
CachedPlanQuery: 130048 total in 7 blocks; 29952 free (2 chunks);
100096 used
It does look like a 182KiB has been spent for some SQL, however there's no
clear way to tell which SQL is to blame.
Another case: PL/pgSQL function context: 57344 total in 3 blocks; 17200
free (2 chunks); 40144 used
It is not clear what is there inside, which "cached plans" are referenced
by that pgsql context (if any), etc.
It would be great if there was an ability to dump the memory in a
machine-readable format (e.g. Java's HPROF).
Eclipse Memory Analyzer (https://www.eclipse.org/mat/) can visualize Java
memory dumps quite well, and I think HPROF format is trivial to generate
(the generation is easy, the hard part is to parse memory contents).
That is we could get analysis UI for free if PostgreSQL produces the dump.
Is it something welcome or non-welcome?
Is it something worth including in-core?
Vladimir
Hi,
On 2018-03-23 16:18:52 +0000, Vladimir Sitnikov wrote:
Hi,
I investigate an out of memory-related case for PostgreSQL 9.6.5, and it
looks like MemoryContextStatsDetail + gdb are the only friends there.MemoryContextStatsDetail does print some info, however it is rarely
possible to associate the used memory with business cases.
For insance:
CachedPlanSource: 146224 total in 8 blocks; 59768 free (3 chunks); 86456
used
CachedPlanQuery: 130048 total in 7 blocks; 29952 free (2 chunks);
100096 usedIt does look like a 182KiB has been spent for some SQL, however there's no
clear way to tell which SQL is to blame.Another case: PL/pgSQL function context: 57344 total in 3 blocks; 17200
free (2 chunks); 40144 used
It is not clear what is there inside, which "cached plans" are referenced
by that pgsql context (if any), etc.It would be great if there was an ability to dump the memory in a
machine-readable format (e.g. Java's HPROF).Eclipse Memory Analyzer (https://www.eclipse.org/mat/) can visualize Java
memory dumps quite well, and I think HPROF format is trivial to generate
(the generation is easy, the hard part is to parse memory contents).
That is we could get analysis UI for free if PostgreSQL produces the dump.Is it something welcome or non-welcome?
Is it something worth including in-core?
The overhead required for it (in cycles, in higher memory usage due to
additional bookeeping, in maintenance) makes me highly doubtful it's
worth going there. While I definitely can see the upside, it doesn't
seem to justify the cost.
Greetings,
Andres Freund
Andres>The overhead required for it (in cycles, in higher memory usage due
to
additional bookeeping
Does that mean the memory contexts are unparseable? (there's not enough
information to enumerate contents)
What if memory dump is produced by walking the C structures?
For instance, I assume statament cache is stored in some sort of a hash
table, so there should be a way to enumerate it in a programmatic way. Of
course it would take time, however I do not think it creates cpu/memory
overheads. The overhead is to maintain "walker" code.
Vladimir
On 2018-03-23 18:05:38 +0000, Vladimir Sitnikov wrote:
Andres>The overhead required for it (in cycles, in higher memory usage due
to
additional bookeepingDoes that mean the memory contexts are unparseable? (there's not enough
information to enumerate contents)
You can enumerate them (that's what the stats dump you're referring to
do), but you can't associate them with individual statements etc without
further bookepping.
What if memory dump is produced by walking the C structures?
We don't know the types of individual allocations.
For instance, I assume statament cache is stored in some sort of a hash
table, so there should be a way to enumerate it in a programmatic way. Of
course it would take time, however I do not think it creates cpu/memory
overheads. The overhead is to maintain "walker" code.
Sure, you could, entirely independent of the memory stats dump, do
that. But what information would you actually gain from it? Which row
something in the catcache belongs to isn't *that* interesting.
- Andres
Andres Freund <andres@anarazel.de> writes:
On 2018-03-23 18:05:38 +0000, Vladimir Sitnikov wrote:
For instance, I assume statament cache is stored in some sort of a hash
table, so there should be a way to enumerate it in a programmatic way. Of
course it would take time, however I do not think it creates cpu/memory
overheads. The overhead is to maintain "walker" code.
Sure, you could, entirely independent of the memory stats dump, do
that. But what information would you actually gain from it? Which row
something in the catcache belongs to isn't *that* interesting.
It'd certainly be easy to define this in a way that makes it require
a bunch of support code, which we'd be unlikely to want to write and
maintain. However, I've often wished that the contexts in a memory
dump were less anonymous. If you didn't just see a pile of "PL/pgSQL
function context" entries, but could (say) see the name of each function,
that would be a big step forward. Similarly, if we could see the source
text for each CachedPlanSource in a dump, that'd be useful. I mention
these things because we do actually store them already, in many cases
--- but the memory stats code doesn't know about them.
Now, commit 9fa6f00b1 already introduced a noticeable penalty for
contexts with nonconstant names, so trying to stick extra info like
this into the context name is not appetizing. But what if we allowed
the context name to have two parts, a fixed part and a variable part?
We could actually require that the fixed part be a compile-time-constant
string, simplifying matters on that end. The variable part would best
be assigned later than initial context creation, because you'd need a
chance to copy the string into the context before pointing to it.
So maybe, for contexts where this is worth doing, it'd look something
like this for plpgsql:
func_cxt = AllocSetContextCreate(TopMemoryContext,
"PL/pgSQL function context",
ALLOCSET_DEFAULT_SIZES);
plpgsql_compile_tmp_cxt = MemoryContextSwitchTo(func_cxt);
function->fn_signature = format_procedure(fcinfo->flinfo->fn_oid);
+ MemoryContextSetIdentifier(func_cxt, function->fn_signature);
function->fn_oid = fcinfo->flinfo->fn_oid;
function->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
This would cost an extra char * field in struct MemoryContextData,
which is slightly annoying but it doesn't exactly seem like a killer.
Then the memory stats dump code would just need to know to print this
field if it isn't NULL.
If we wanted to do this I'd suggest sneaking it into v11, so that
if people have to adapt their code because of 9fa6f00b1 breaking
usages with nonconstant context names, they have a solution to turn to
immediately rather than having to change things again in v12.
Thoughts?
regards, tom lane
Hi,
On 2018-03-23 14:33:25 -0400, Tom Lane wrote:
func_cxt = AllocSetContextCreate(TopMemoryContext,
"PL/pgSQL function context",
ALLOCSET_DEFAULT_SIZES);
plpgsql_compile_tmp_cxt = MemoryContextSwitchTo(func_cxt);function->fn_signature = format_procedure(fcinfo->flinfo->fn_oid);
+ MemoryContextSetIdentifier(func_cxt, function->fn_signature);
function->fn_oid = fcinfo->flinfo->fn_oid;
function->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);This would cost an extra char * field in struct MemoryContextData,
which is slightly annoying but it doesn't exactly seem like a killer.
Then the memory stats dump code would just need to know to print this
field if it isn't NULL.
That's not a bad idea. How about storing a Node* instead of a char*?
Then we could have MemoryContextStats etc support digging out details
for a few types, without having to generate strings at runtime.
If we wanted to do this I'd suggest sneaking it into v11, so that
if people have to adapt their code because of 9fa6f00b1 breaking
usages with nonconstant context names, they have a solution to turn to
immediately rather than having to change things again in v12.
Yea, that'd make sense.
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
On 2018-03-23 14:33:25 -0400, Tom Lane wrote:
+ MemoryContextSetIdentifier(func_cxt, function->fn_signature);
This would cost an extra char * field in struct MemoryContextData,
which is slightly annoying but it doesn't exactly seem like a killer.
Then the memory stats dump code would just need to know to print this
field if it isn't NULL.
That's not a bad idea. How about storing a Node* instead of a char*?
Then we could have MemoryContextStats etc support digging out details
for a few types, without having to generate strings at runtime.
Well, in the cases I'm thinking of at the moment, there's no handy Node
to point at, just module-private structs like PLpgSQL_function. So doing
anything like that would add nonzero overhead to construct something.
Not sure we want to pay that. There's also the fact that we don't want
MemoryContextStats doing anything very complicated, because of the risk of
failure and the likelihood that any attempt to palloc would fail (if we're
there because we're up against OOM already).
If we wanted to do this I'd suggest sneaking it into v11, so that
if people have to adapt their code because of 9fa6f00b1 breaking
usages with nonconstant context names, they have a solution to turn to
immediately rather than having to change things again in v12.
Yea, that'd make sense.
I'll put together a draft patch.
regards, tom lane
On 2018-03-23 15:12:43 -0400, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
On 2018-03-23 14:33:25 -0400, Tom Lane wrote:
+ MemoryContextSetIdentifier(func_cxt, function->fn_signature);
This would cost an extra char * field in struct MemoryContextData,
which is slightly annoying but it doesn't exactly seem like a killer.
Then the memory stats dump code would just need to know to print this
field if it isn't NULL.That's not a bad idea. How about storing a Node* instead of a char*?
Then we could have MemoryContextStats etc support digging out details
for a few types, without having to generate strings at runtime.Well, in the cases I'm thinking of at the moment, there's no handy Node
to point at, just module-private structs like PLpgSQL_function.
Well, the cases Vladimir were concerned about seem less clear
though. It'd be nice if we could just point to a CachedPlanSource and
such.
So doing anything like that would add nonzero overhead to construct
something.
I'm not that sure there aren't easy way to overcome those - couldn't we
"just" make FmgrInfo etc be tagged types? The space overhead of that
can't matter in comparison to the size of the relevant structs.
There's also the fact that we don't want MemoryContextStats doing
anything very complicated, because of the risk of failure and the
likelihood that any attempt to palloc would fail (if we're there
because we're up against OOM already).
That's true. But I'm not sure there's a meaningful difference in risk
here. Obviously you shouldn't try to print a node tree or something, but
an if statement looking
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
On 2018-03-23 15:12:43 -0400, Tom Lane wrote:
Well, in the cases I'm thinking of at the moment, there's no handy Node
to point at, just module-private structs like PLpgSQL_function.
Well, the cases Vladimir were concerned about seem less clear
though. It'd be nice if we could just point to a CachedPlanSource and
such.
You could imagine adding *two* pointers to memory contexts, a callback
function and an arg to pass to it, so that the callback localizes the
knowledge of how to dig an identifier string out of whatever struct
is involved. I really doubt this is worth that much overhead though.
I think all of the actually interesting cases have a string available
already (though I might find out differently while doing the patch).
Furthermore, if they don't have a string available already, I'm not
real clear on how the callback would create one without doing a palloc.
I'm not that sure there aren't easy way to overcome those - couldn't we
"just" make FmgrInfo etc be tagged types? The space overhead of that
can't matter in comparison to the size of the relevant structs.
Not for extensions, eg PLs, which would be one of the bigger use-cases
IMO.
regards, tom lane
Hi!
Some help you could get from
https://github.com/postgrespro/memstat
Vladimir Sitnikov wrote:
Hi,
I investigate an out of memory-related case for PostgreSQL 9.6.5, and it
looks likeО©╫MemoryContextStatsDetailО©╫+ gdbО©╫are the only friends there.
--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/
I wrote:
I'll put together a draft patch.
Here's a draft patch for this. Some notes:
* I'm generally pretty happy with the way this turned out. For instance,
you can now tell the difference between index info, partition descriptor,
and RLS policy sub-contexts of CacheMemoryContext; previously they were
all just labeled with the name of their relation, which is useful but not
really enough anymore. I've attached sample MemoryContextStats output
captured near the end of the plpgsql.sql regression test.
* I reverted the addition of the "flags" parameter to the context creation
functions, since it no longer had any use. We could have left it there
for future expansion, but doing so seems like an unnecessary deviation
from the v10 APIs. We can cross that bridge when and if we come to it.
* I'd have liked to get rid of the AllocSetContextCreate wrapper macro
entirely, reverting that to the way it was in v10 as well, but I don't see
any way to do so without giving up the __builtin_constant_p(name) check,
which seems like it'd be a bad idea.
* In some places there's a pstrdup more than there was before (hidden
inside MemoryContextCopySetIdentifier), but I don't think this is really
much more expensive than the previous behavior with MEMCONTEXT_COPY_NAME.
In particular, the fact that having a non-constant identifier no longer
disqualifies a context from participating in aset.c's freelist scheme
probably buys back the overhead. I haven't done any performance testing,
though.
* It seemed worth expending a pstrdup to copy the source string for a
CachedPlan into its context so it could be labeled properly. We can't
just point to the source string in the originating CachedPlanSource
because that might have a different/shorter lifespan. I think in the
long run this would come out to be "free" anyway because someday we're
going to insist on having the source string available at execution for
error-reporting reasons.
* On the other hand, I didn't copy the source string into SPI Plan
contexts. Looking at the sample output, I started to get a bit annoyed
that we have those as separate contexts at all --- at least in the
standard case of one subsidiary CachedPlanSource, seems like we could
combine the contexts. That's fit material for a separate patch, though.
* While I didn't do anything about it here, I think it'd likely be a
good idea for MemoryContextStats printout to truncate the context ID
strings at 100 characters or so. Otherwise, in situations where we
have long queries with cached plans, the output would get unreasonably
bulky. Any thoughts about the exact rule?
* With the preceding idea in mind, I decided to fix things so that the
formatting of MemoryContextStats output is centralized in one place
instead of being repeated in each context module. So there's now a
callback-function API there. This has the additional benefit that people
could write extensions that collect stats output and do $whatever with it,
without relying on anything more than what memnodes.h exposes.
Comments, objections?
regards, tom lane
Attachments:
add-context-identifiers-1.patchtext/x-diff; charset=us-ascii; name=add-context-identifiers-1.patchDownload
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 5d1b902..cfc6201 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -999,7 +999,6 @@ AtStart_Memory(void)
TransactionAbortContext =
AllocSetContextCreateExtended(TopMemoryContext,
"TransactionAbortContext",
- 0,
32 * 1024,
32 * 1024,
32 * 1024);
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 53855f5..c0ba02d 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -521,10 +521,11 @@ RelationBuildPartitionDesc(Relation rel)
}
/* Now build the actual relcache partition descriptor */
- rel->rd_pdcxt = AllocSetContextCreateExtended(CacheMemoryContext,
- RelationGetRelationName(rel),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_DEFAULT_SIZES);
+ rel->rd_pdcxt = AllocSetContextCreate(CacheMemoryContext,
+ "partition descriptor",
+ ALLOCSET_DEFAULT_SIZES);
+ MemoryContextCopySetIdentifier(rel->rd_pdcxt, RelationGetRelationName(rel));
+
oldcxt = MemoryContextSwitchTo(rel->rd_pdcxt);
result = (PartitionDescData *) palloc0(sizeof(PartitionDescData));
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 280a14a..cfaf32c 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -214,6 +214,9 @@ RelationBuildRowSecurity(Relation relation)
SysScanDesc sscan;
HeapTuple tuple;
+ MemoryContextCopySetIdentifier(rscxt,
+ RelationGetRelationName(relation));
+
rsdesc = MemoryContextAllocZero(rscxt, sizeof(RowSecurityDesc));
rsdesc->rscxt = rscxt;
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 1c00ac9..2354589 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -612,7 +612,7 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
* must be a child of whatever context holds the FmgrInfo.
*/
fcontext = AllocSetContextCreate(finfo->fn_mcxt,
- "SQL function data",
+ "SQL function",
ALLOCSET_DEFAULT_SIZES);
oldcontext = MemoryContextSwitchTo(fcontext);
@@ -635,9 +635,11 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
procedureStruct = (Form_pg_proc) GETSTRUCT(procedureTuple);
/*
- * copy function name immediately for use by error reporting callback
+ * copy function name immediately for use by error reporting callback, and
+ * for use as memory context identifier
*/
fcache->fname = pstrdup(NameStr(procedureStruct->proname));
+ MemoryContextSetIdentifier(fcontext, fcache->fname);
/*
* get the result type from the procedure tuple, and check for polymorphic
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 5ffe638..b4016ed 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -243,19 +243,16 @@ ReorderBufferAllocate(void)
buffer->change_context = SlabContextCreate(new_ctx,
"Change",
- 0,
SLAB_DEFAULT_BLOCK_SIZE,
sizeof(ReorderBufferChange));
buffer->txn_context = SlabContextCreate(new_ctx,
"TXN",
- 0,
SLAB_DEFAULT_BLOCK_SIZE,
sizeof(ReorderBufferTXN));
buffer->tup_context = GenerationContextCreate(new_ctx,
"Tuples",
- 0,
SLAB_LARGE_BLOCK_SIZE);
hash_ctl.keysize = sizeof(TransactionId);
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index d34d5c3..6c836f6 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -74,7 +74,8 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MemoryContext cxt;
MemoryContext oldcxt;
- cxt = AllocSetContextCreate(CurrentMemoryContext, "stats ext",
+ cxt = AllocSetContextCreate(CurrentMemoryContext,
+ "BuildRelationExtStatistics",
ALLOCSET_DEFAULT_SIZES);
oldcxt = MemoryContextSwitchTo(cxt);
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 8d7d8e0..85bb7b91 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -180,6 +180,7 @@ CreateCachedPlan(RawStmt *raw_parse_tree,
plansource->magic = CACHEDPLANSOURCE_MAGIC;
plansource->raw_parse_tree = copyObject(raw_parse_tree);
plansource->query_string = pstrdup(query_string);
+ MemoryContextSetIdentifier(source_context, plansource->query_string);
plansource->commandTag = commandTag;
plansource->param_types = NULL;
plansource->num_params = 0;
@@ -951,6 +952,7 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
plan_context = AllocSetContextCreate(CurrentMemoryContext,
"CachedPlan",
ALLOCSET_START_SMALL_SIZES);
+ MemoryContextCopySetIdentifier(plan_context, plansource->query_string);
/*
* Copy plan into the new context.
@@ -1346,6 +1348,7 @@ CopyCachedPlan(CachedPlanSource *plansource)
newsource->magic = CACHEDPLANSOURCE_MAGIC;
newsource->raw_parse_tree = copyObject(plansource->raw_parse_tree);
newsource->query_string = pstrdup(plansource->query_string);
+ MemoryContextSetIdentifier(source_context, newsource->query_string);
newsource->commandTag = plansource->commandTag;
if (plansource->num_params > 0)
{
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 6ab4db2..12ddabc 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -673,11 +673,12 @@ RelationBuildRuleLock(Relation relation)
/*
* Make the private context. Assume it'll not contain much data.
*/
- rulescxt = AllocSetContextCreateExtended(CacheMemoryContext,
- RelationGetRelationName(relation),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ rulescxt = AllocSetContextCreate(CacheMemoryContext,
+ "relation rules",
+ ALLOCSET_SMALL_SIZES);
relation->rd_rulescxt = rulescxt;
+ MemoryContextCopySetIdentifier(rulescxt,
+ RelationGetRelationName(relation));
/*
* allocate an array to hold the rewrite rules (the array is extended if
@@ -850,10 +851,11 @@ RelationBuildPartitionKey(Relation relation)
if (!HeapTupleIsValid(tuple))
return;
- partkeycxt = AllocSetContextCreateExtended(CurTransactionContext,
- RelationGetRelationName(relation),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ partkeycxt = AllocSetContextCreate(CurTransactionContext,
+ "partition key",
+ ALLOCSET_SMALL_SIZES);
+ MemoryContextCopySetIdentifier(partkeycxt,
+ RelationGetRelationName(relation));
key = (PartitionKey) MemoryContextAllocZero(partkeycxt,
sizeof(PartitionKeyData));
@@ -1531,11 +1533,12 @@ RelationInitIndexAccessInfo(Relation relation)
* a context, and not just a couple of pallocs, is so that we won't leak
* any subsidiary info attached to fmgr lookup records.
*/
- indexcxt = AllocSetContextCreateExtended(CacheMemoryContext,
- RelationGetRelationName(relation),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ indexcxt = AllocSetContextCreate(CacheMemoryContext,
+ "index info",
+ ALLOCSET_SMALL_SIZES);
relation->rd_indexcxt = indexcxt;
+ MemoryContextCopySetIdentifier(indexcxt,
+ RelationGetRelationName(relation));
/*
* Now we can fetch the index AM's API struct
@@ -5505,12 +5508,12 @@ load_relcache_init_file(bool shared)
* prepare index info context --- parameters should match
* RelationInitIndexAccessInfo
*/
- indexcxt =
- AllocSetContextCreateExtended(CacheMemoryContext,
- RelationGetRelationName(rel),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ indexcxt = AllocSetContextCreate(CacheMemoryContext,
+ "index info",
+ ALLOCSET_SMALL_SIZES);
rel->rd_indexcxt = indexcxt;
+ MemoryContextCopySetIdentifier(indexcxt,
+ RelationGetRelationName(rel));
/*
* Now we can fetch the index AM's API struct. (We can't store
diff --git a/src/backend/utils/cache/ts_cache.c b/src/backend/utils/cache/ts_cache.c
index 3d5c194..9734778 100644
--- a/src/backend/utils/cache/ts_cache.c
+++ b/src/backend/utils/cache/ts_cache.c
@@ -294,16 +294,19 @@ lookup_ts_dictionary_cache(Oid dictId)
Assert(!found); /* it wasn't there a moment ago */
/* Create private memory context the first time through */
- saveCtx = AllocSetContextCreateExtended(CacheMemoryContext,
- NameStr(dict->dictname),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ saveCtx = AllocSetContextCreate(CacheMemoryContext,
+ "TS dictionary",
+ ALLOCSET_SMALL_SIZES);
+ MemoryContextCopySetIdentifier(saveCtx, NameStr(dict->dictname));
}
else
{
/* Clear the existing entry's private context */
saveCtx = entry->dictCtx;
- MemoryContextResetAndDeleteChildren(saveCtx);
+ /* Don't let context's ident pointer dangle while we reset it */
+ MemoryContextSetIdentifier(saveCtx, NULL);
+ MemoryContextReset(saveCtx);
+ MemoryContextCopySetIdentifier(saveCtx, NameStr(dict->dictname));
}
MemSet(entry, 0, sizeof(TSDictionaryCacheEntry));
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 5281cd5..63fdc46 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -341,10 +341,9 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
else
CurrentDynaHashCxt = TopMemoryContext;
CurrentDynaHashCxt =
- AllocSetContextCreateExtended(CurrentDynaHashCxt,
- tabname,
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_DEFAULT_SIZES);
+ AllocSetContextCreate(CurrentDynaHashCxt,
+ "dynahash",
+ ALLOCSET_DEFAULT_SIZES);
}
/* Initialize the hash header, plus a copy of the table name */
@@ -354,6 +353,10 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
hashp->tabname = (char *) (hashp + 1);
strcpy(hashp->tabname, tabname);
+ /* If we have a private context, label it with hashtable's name */
+ if (!(flags & HASH_SHARED_MEM))
+ MemoryContextSetIdentifier(CurrentDynaHashCxt, hashp->tabname);
+
/*
* Select the appropriate hash function (see comments at head of file).
*/
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 3f9b188..e3d2c4e 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -130,7 +130,6 @@ typedef struct AllocSetContext
Size initBlockSize; /* initial block size */
Size maxBlockSize; /* maximum block size */
Size nextBlockSize; /* next block size to allocate */
- Size headerSize; /* allocated size of context header */
Size allocChunkLimit; /* effective chunk size limit */
AllocBlock keeper; /* keep this block over resets */
/* freelist this context could be put in, or -1 if not a candidate: */
@@ -228,10 +227,7 @@ typedef struct AllocChunkData
* only its initial malloc chunk and no others. To be a candidate for a
* freelist, a context must have the same minContextSize/initBlockSize as
* other contexts in the list; but its maxBlockSize is irrelevant since that
- * doesn't affect the size of the initial chunk. Also, candidate contexts
- * *must not* use MEMCONTEXT_COPY_NAME since that would make their header size
- * variable. (We currently insist that all flags be zero, since other flags
- * would likely make the contexts less interchangeable, too.)
+ * doesn't affect the size of the initial chunk.
*
* We currently provide one freelist for ALLOCSET_DEFAULT_SIZES contexts
* and one for ALLOCSET_SMALL_SIZES contexts; the latter works for
@@ -276,7 +272,8 @@ static void AllocSetReset(MemoryContext context);
static void AllocSetDelete(MemoryContext context);
static Size AllocSetGetChunkSpace(MemoryContext context, void *pointer);
static bool AllocSetIsEmpty(MemoryContext context);
-static void AllocSetStats(MemoryContext context, int level, bool print,
+static void AllocSetStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals);
#ifdef MEMORY_CONTEXT_CHECKING
@@ -378,14 +375,11 @@ AllocSetFreeIndex(Size size)
* Create a new AllocSet context.
*
* parent: parent context, or NULL if top-level context
- * name: name of context (for debugging only, need not be unique)
- * flags: bitmask of MEMCONTEXT_XXX option flags
+ * name: name of context (must be statically allocated)
* minContextSize: minimum context size
* initBlockSize: initial allocation block size
* maxBlockSize: maximum allocation block size
*
- * Notes: if flags & MEMCONTEXT_COPY_NAME, the name string will be copied into
- * context-lifespan storage; otherwise, it had better be statically allocated.
* Most callers should abstract the context size parameters using a macro
* such as ALLOCSET_DEFAULT_SIZES. (This is now *required* when going
* through the AllocSetContextCreate macro.)
@@ -393,13 +387,11 @@ AllocSetFreeIndex(Size size)
MemoryContext
AllocSetContextCreateExtended(MemoryContext parent,
const char *name,
- int flags,
Size minContextSize,
Size initBlockSize,
Size maxBlockSize)
{
int freeListIndex;
- Size headerSize;
Size firstBlockSize;
AllocSet set;
AllocBlock block;
@@ -431,12 +423,10 @@ AllocSetContextCreateExtended(MemoryContext parent,
* Check whether the parameters match either available freelist. We do
* not need to demand a match of maxBlockSize.
*/
- if (flags == 0 &&
- minContextSize == ALLOCSET_DEFAULT_MINSIZE &&
+ if (minContextSize == ALLOCSET_DEFAULT_MINSIZE &&
initBlockSize == ALLOCSET_DEFAULT_INITSIZE)
freeListIndex = 0;
- else if (flags == 0 &&
- minContextSize == ALLOCSET_SMALL_MINSIZE &&
+ else if (minContextSize == ALLOCSET_SMALL_MINSIZE &&
initBlockSize == ALLOCSET_SMALL_INITSIZE)
freeListIndex = 1;
else
@@ -462,25 +452,17 @@ AllocSetContextCreateExtended(MemoryContext parent,
/* Reinitialize its header, installing correct name and parent */
MemoryContextCreate((MemoryContext) set,
T_AllocSetContext,
- set->headerSize,
- sizeof(AllocSetContext),
&AllocSetMethods,
parent,
- name,
- flags);
+ name);
return (MemoryContext) set;
}
}
- /* Size of the memory context header, including name storage if needed */
- if (flags & MEMCONTEXT_COPY_NAME)
- headerSize = MAXALIGN(sizeof(AllocSetContext) + strlen(name) + 1);
- else
- headerSize = MAXALIGN(sizeof(AllocSetContext));
-
/* Determine size of initial block */
- firstBlockSize = headerSize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ;
+ firstBlockSize = MAXALIGN(sizeof(AllocSetContext)) +
+ ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ;
if (minContextSize != 0)
firstBlockSize = Max(firstBlockSize, minContextSize);
else
@@ -508,7 +490,7 @@ AllocSetContextCreateExtended(MemoryContext parent,
*/
/* Fill in the initial block's block header */
- block = (AllocBlock) (((char *) set) + headerSize);
+ block = (AllocBlock) (((char *) set) + MAXALIGN(sizeof(AllocSetContext)));
block->aset = set;
block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
block->endptr = ((char *) set) + firstBlockSize;
@@ -529,7 +511,6 @@ AllocSetContextCreateExtended(MemoryContext parent,
set->initBlockSize = initBlockSize;
set->maxBlockSize = maxBlockSize;
set->nextBlockSize = initBlockSize;
- set->headerSize = headerSize;
set->freeListIndex = freeListIndex;
/*
@@ -559,12 +540,9 @@ AllocSetContextCreateExtended(MemoryContext parent,
/* Finally, do the type-independent part of context creation */
MemoryContextCreate((MemoryContext) set,
T_AllocSetContext,
- headerSize,
- sizeof(AllocSetContext),
&AllocSetMethods,
parent,
- name,
- flags);
+ name);
return (MemoryContext) set;
}
@@ -1327,12 +1305,13 @@ AllocSetIsEmpty(MemoryContext context)
* AllocSetStats
* Compute stats about memory consumption of an allocset.
*
- * level: recursion level (0 at top level); used for print indentation.
- * print: true to print stats to stderr.
- * totals: if not NULL, add stats about this allocset into *totals.
+ * printfunc: if not NULL, pass a human-readable stats string to this.
+ * passthru: pass this pointer through to printfunc.
+ * totals: if not NULL, add stats about this context into *totals.
*/
static void
-AllocSetStats(MemoryContext context, int level, bool print,
+AllocSetStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals)
{
AllocSet set = (AllocSet) context;
@@ -1344,7 +1323,7 @@ AllocSetStats(MemoryContext context, int level, bool print,
int fidx;
/* Include context header in totalspace */
- totalspace = set->headerSize;
+ totalspace = MAXALIGN(sizeof(AllocSetContext));
for (block = set->blocks; block != NULL; block = block->next)
{
@@ -1364,16 +1343,15 @@ AllocSetStats(MemoryContext context, int level, bool print,
}
}
- if (print)
+ if (printfunc)
{
- int i;
-
- for (i = 0; i < level; i++)
- fprintf(stderr, " ");
- fprintf(stderr,
- "%s: %zu total in %zd blocks; %zu free (%zd chunks); %zu used\n",
- set->header.name, totalspace, nblocks, freespace, freechunks,
- totalspace - freespace);
+ char stats_string[200];
+
+ snprintf(stats_string, sizeof(stats_string),
+ "%zu total in %zd blocks; %zu free (%zd chunks); %zu used",
+ totalspace, nblocks, freespace, freechunks,
+ totalspace - freespace);
+ printfunc(context, passthru, stats_string);
}
if (totals)
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index 338386a..7ee3c48 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -61,7 +61,6 @@ typedef struct GenerationContext
/* Generational context parameters */
Size blockSize; /* standard block size */
- Size headerSize; /* allocated size of context header */
GenerationBlock *block; /* current (most recently allocated) block */
dlist_head blocks; /* list of blocks */
@@ -154,7 +153,8 @@ static void GenerationReset(MemoryContext context);
static void GenerationDelete(MemoryContext context);
static Size GenerationGetChunkSpace(MemoryContext context, void *pointer);
static bool GenerationIsEmpty(MemoryContext context);
-static void GenerationStats(MemoryContext context, int level, bool print,
+static void GenerationStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals);
#ifdef MEMORY_CONTEXT_CHECKING
@@ -203,14 +203,16 @@ static const MemoryContextMethods GenerationMethods = {
/*
* GenerationContextCreate
* Create a new Generation context.
+ *
+ * parent: parent context, or NULL if top-level context
+ * name: name of context (must be statically allocated)
+ * blockSize: generation block size
*/
MemoryContext
GenerationContextCreate(MemoryContext parent,
const char *name,
- int flags,
Size blockSize)
{
- Size headerSize;
GenerationContext *set;
/* Assert we padded GenerationChunk properly */
@@ -238,13 +240,7 @@ GenerationContextCreate(MemoryContext parent,
* freeing the first generation of allocations.
*/
- /* Size of the memory context header, including name storage if needed */
- if (flags & MEMCONTEXT_COPY_NAME)
- headerSize = MAXALIGN(sizeof(GenerationContext) + strlen(name) + 1);
- else
- headerSize = MAXALIGN(sizeof(GenerationContext));
-
- set = (GenerationContext *) malloc(headerSize);
+ set = (GenerationContext *) malloc(MAXALIGN(sizeof(GenerationContext)));
if (set == NULL)
{
MemoryContextStats(TopMemoryContext);
@@ -262,19 +258,15 @@ GenerationContextCreate(MemoryContext parent,
/* Fill in GenerationContext-specific header fields */
set->blockSize = blockSize;
- set->headerSize = headerSize;
set->block = NULL;
dlist_init(&set->blocks);
/* Finally, do the type-independent part of context creation */
MemoryContextCreate((MemoryContext) set,
T_GenerationContext,
- headerSize,
- sizeof(GenerationContext),
&GenerationMethods,
parent,
- name,
- flags);
+ name);
return (MemoryContext) set;
}
@@ -683,15 +675,16 @@ GenerationIsEmpty(MemoryContext context)
* GenerationStats
* Compute stats about memory consumption of a Generation context.
*
- * level: recursion level (0 at top level); used for print indentation.
- * print: true to print stats to stderr.
- * totals: if not NULL, add stats about this Generation into *totals.
+ * printfunc: if not NULL, pass a human-readable stats string to this.
+ * passthru: pass this pointer through to printfunc.
+ * totals: if not NULL, add stats about this context into *totals.
*
* XXX freespace only accounts for empty space at the end of the block, not
* space of freed chunks (which is unknown).
*/
static void
-GenerationStats(MemoryContext context, int level, bool print,
+GenerationStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals)
{
GenerationContext *set = (GenerationContext *) context;
@@ -703,7 +696,7 @@ GenerationStats(MemoryContext context, int level, bool print,
dlist_iter iter;
/* Include context header in totalspace */
- totalspace = set->headerSize;
+ totalspace = MAXALIGN(sizeof(GenerationContext));
dlist_foreach(iter, &set->blocks)
{
@@ -716,16 +709,15 @@ GenerationStats(MemoryContext context, int level, bool print,
freespace += (block->endptr - block->freeptr);
}
- if (print)
+ if (printfunc)
{
- int i;
-
- for (i = 0; i < level; i++)
- fprintf(stderr, " ");
- fprintf(stderr,
- "Generation: %s: %zu total in %zd blocks (%zd chunks); %zu free (%zd chunks); %zu used\n",
- ((MemoryContext) set)->name, totalspace, nblocks, nchunks, freespace,
- nfreechunks, totalspace - freespace);
+ char stats_string[200];
+
+ snprintf(stats_string, sizeof(stats_string),
+ "%zu total in %zd blocks (%zd chunks); %zu free (%zd chunks); %zu used",
+ totalspace, nblocks, nchunks, freespace,
+ nfreechunks, totalspace - freespace);
+ printfunc(context, passthru, stats_string);
}
if (totals)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index d7baa54..f50cdca 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -55,6 +55,8 @@ static void MemoryContextCallResetCallbacks(MemoryContext context);
static void MemoryContextStatsInternal(MemoryContext context, int level,
bool print, int max_children,
MemoryContextCounters *totals);
+static void MemoryContextStatsPrint(MemoryContext context, void *passthru,
+ const char *stats_string);
/*
* You should not do memory allocations within a critical section, because
@@ -118,7 +120,6 @@ MemoryContextInit(void)
*/
ErrorContext = AllocSetContextCreateExtended(TopMemoryContext,
"ErrorContext",
- 0,
8 * 1024,
8 * 1024,
8 * 1024);
@@ -296,6 +297,23 @@ MemoryContextCallResetCallbacks(MemoryContext context)
}
/*
+ * MemoryContextSetIdentifier
+ * Set the identifier string for a memory context.
+ *
+ * An identifier can be provided to help distinguish among different contexts
+ * of the same kind in memory context stats dumps. The identifier string
+ * must live at least as long as the context it is for; typically it is
+ * allocated inside that context, so that it automatically goes away on
+ * context deletion. Pass id = NULL to forget any old identifier.
+ */
+void
+MemoryContextSetIdentifier(MemoryContext context, const char *id)
+{
+ AssertArg(MemoryContextIsValid(context));
+ context->ident = id;
+}
+
+/*
* MemoryContextSetParent
* Change a context to belong to a new parent (or no parent).
*
@@ -480,7 +498,10 @@ MemoryContextStatsInternal(MemoryContext context, int level,
AssertArg(MemoryContextIsValid(context));
/* Examine the context itself */
- context->methods->stats(context, level, print, totals);
+ context->methods->stats(context,
+ print ? MemoryContextStatsPrint : NULL,
+ (void *) &level,
+ totals);
/*
* Examine children. If there are more than max_children of them, we do
@@ -532,6 +553,28 @@ MemoryContextStatsInternal(MemoryContext context, int level,
}
/*
+ * MemoryContextStatsPrint
+ * Print callback used by MemoryContextStatsInternal
+ *
+ * For now, the passthru pointer just points to "int level"; later we might
+ * make that more complicated.
+ */
+static void
+MemoryContextStatsPrint(MemoryContext context, void *passthru,
+ const char *stats_string)
+{
+ int level = *(int *) passthru;
+ int i;
+
+ for (i = 0; i < level; i++)
+ fprintf(stderr, " ");
+ fprintf(stderr, "%s", context->name);
+ if (context->ident)
+ fprintf(stderr, " %s", context->ident);
+ fprintf(stderr, ": %s\n", stats_string);
+}
+
+/*
* MemoryContextCheck
* Check all chunks in the named context.
*
@@ -612,34 +655,23 @@ MemoryContextContains(MemoryContext context, void *pointer)
*
* node: the as-yet-uninitialized common part of the context header node.
* tag: NodeTag code identifying the memory context type.
- * size: total size of context header including context-type-specific fields,
- * as well as space for the context name if MEMCONTEXT_COPY_NAME is set.
- * nameoffset: where within the "size" space to insert the context name.
* methods: context-type-specific methods (usually statically allocated).
* parent: parent context, or NULL if this will be a top-level context.
- * name: name of context (for debugging only, need not be unique).
- * flags: bitmask of MEMCONTEXT_XXX option flags.
+ * name: name of context (must be statically allocated).
*
* Context routines generally assume that MemoryContextCreate can't fail,
* so this can contain Assert but not elog/ereport.
*/
void
MemoryContextCreate(MemoryContext node,
- NodeTag tag, Size size, Size nameoffset,
+ NodeTag tag,
const MemoryContextMethods *methods,
MemoryContext parent,
- const char *name,
- int flags)
+ const char *name)
{
/* Creating new memory contexts is not allowed in a critical section */
Assert(CritSectionCount == 0);
- /* Check size is sane */
- Assert(nameoffset >= sizeof(MemoryContextData));
- Assert((flags & MEMCONTEXT_COPY_NAME) ?
- size >= nameoffset + strlen(name) + 1 :
- size >= nameoffset);
-
/* Initialize all standard fields of memory context header */
node->type = tag;
node->isReset = true;
@@ -647,22 +679,10 @@ MemoryContextCreate(MemoryContext node,
node->parent = parent;
node->firstchild = NULL;
node->prevchild = NULL;
+ node->name = name;
+ node->ident = NULL;
node->reset_cbs = NULL;
- if (flags & MEMCONTEXT_COPY_NAME)
- {
- /* Insert context name into space reserved for it */
- char *namecopy = ((char *) node) + nameoffset;
-
- node->name = namecopy;
- strcpy(namecopy, name);
- }
- else
- {
- /* Assume the passed-in name is statically allocated */
- node->name = name;
- }
-
/* OK to link node into context tree */
if (parent)
{
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index 58beb64..26b0a15 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -131,7 +131,8 @@ static void SlabReset(MemoryContext context);
static void SlabDelete(MemoryContext context);
static Size SlabGetChunkSpace(MemoryContext context, void *pointer);
static bool SlabIsEmpty(MemoryContext context);
-static void SlabStats(MemoryContext context, int level, bool print,
+static void SlabStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals);
#ifdef MEMORY_CONTEXT_CHECKING
static void SlabCheck(MemoryContext context);
@@ -176,27 +177,22 @@ static const MemoryContextMethods SlabMethods = {
* Create a new Slab context.
*
* parent: parent context, or NULL if top-level context
- * name: name of context (for debugging only, need not be unique)
- * flags: bitmask of MEMCONTEXT_XXX option flags
+ * name: name of context (must be statically allocated)
* blockSize: allocation block size
* chunkSize: allocation chunk size
*
- * Notes: if flags & MEMCONTEXT_COPY_NAME, the name string will be copied into
- * context-lifespan storage; otherwise, it had better be statically allocated.
* The chunkSize may not exceed:
* MAXALIGN_DOWN(SIZE_MAX) - MAXALIGN(sizeof(SlabBlock)) - SLAB_CHUNKHDRSZ
*/
MemoryContext
SlabContextCreate(MemoryContext parent,
const char *name,
- int flags,
Size blockSize,
Size chunkSize)
{
int chunksPerBlock;
Size fullChunkSize;
Size freelistSize;
- Size nameOffset;
Size headerSize;
SlabContext *slab;
int i;
@@ -231,12 +227,8 @@ SlabContextCreate(MemoryContext parent,
* this with the first regular block; not worth the extra complication.
*/
- /* Size of the memory context header, including name storage if needed */
- nameOffset = offsetof(SlabContext, freelist) + freelistSize;
- if (flags & MEMCONTEXT_COPY_NAME)
- headerSize = nameOffset + strlen(name) + 1;
- else
- headerSize = nameOffset;
+ /* Size of the memory context header */
+ headerSize = offsetof(SlabContext, freelist) + freelistSize;
slab = (SlabContext *) malloc(headerSize);
if (slab == NULL)
@@ -270,12 +262,9 @@ SlabContextCreate(MemoryContext parent,
/* Finally, do the type-independent part of context creation */
MemoryContextCreate((MemoryContext) slab,
T_SlabContext,
- headerSize,
- nameOffset,
&SlabMethods,
parent,
- name,
- flags);
+ name);
return (MemoryContext) slab;
}
@@ -634,12 +623,13 @@ SlabIsEmpty(MemoryContext context)
* SlabStats
* Compute stats about memory consumption of a Slab context.
*
- * level: recursion level (0 at top level); used for print indentation.
- * print: true to print stats to stderr.
- * totals: if not NULL, add stats about this Slab into *totals.
+ * printfunc: if not NULL, pass a human-readable stats string to this.
+ * passthru: pass this pointer through to printfunc.
+ * totals: if not NULL, add stats about this context into *totals.
*/
static void
-SlabStats(MemoryContext context, int level, bool print,
+SlabStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals)
{
SlabContext *slab = castNode(SlabContext, context);
@@ -667,14 +657,15 @@ SlabStats(MemoryContext context, int level, bool print,
}
}
- if (print)
+ if (printfunc)
{
- for (i = 0; i < level; i++)
- fprintf(stderr, " ");
- fprintf(stderr,
- "Slab: %s: %zu total in %zd blocks; %zu free (%zd chunks); %zu used\n",
- slab->header.name, totalspace, nblocks, freespace, freechunks,
- totalspace - freespace);
+ char stats_string[200];
+
+ snprintf(stats_string, sizeof(stats_string),
+ "%zu total in %zd blocks; %zu free (%zd chunks); %zu used",
+ totalspace, nblocks, freespace, freechunks,
+ totalspace - freespace);
+ printfunc(context, passthru, stats_string);
}
if (totals)
diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h
index ccb64f0..2a8d83f 100644
--- a/src/include/nodes/memnodes.h
+++ b/src/include/nodes/memnodes.h
@@ -39,18 +39,21 @@ typedef struct MemoryContextCounters
* A logical context in which memory allocations occur.
*
* MemoryContext itself is an abstract type that can have multiple
- * implementations, though for now we have only AllocSetContext.
+ * implementations.
* The function pointers in MemoryContextMethods define one specific
* implementation of MemoryContext --- they are a virtual function table
* in C++ terms.
*
* Node types that are actual implementations of memory contexts must
- * begin with the same fields as MemoryContext.
+ * begin with the same fields as MemoryContextData.
*
* Note: for largely historical reasons, typedef MemoryContext is a pointer
* to the context struct rather than the struct type itself.
*/
+typedef void (*MemoryStatsPrintFunc) (MemoryContext context, void *passthru,
+ const char *stats_string);
+
typedef struct MemoryContextMethods
{
void *(*alloc) (MemoryContext context, Size size);
@@ -61,7 +64,8 @@ typedef struct MemoryContextMethods
void (*delete_context) (MemoryContext context);
Size (*get_chunk_space) (MemoryContext context, void *pointer);
bool (*is_empty) (MemoryContext context);
- void (*stats) (MemoryContext context, int level, bool print,
+ void (*stats) (MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals);
#ifdef MEMORY_CONTEXT_CHECKING
void (*check) (MemoryContext context);
@@ -81,6 +85,7 @@ typedef struct MemoryContextData
MemoryContext prevchild; /* previous child of same parent */
MemoryContext nextchild; /* next child of same parent */
const char *name; /* context name (just for debugging) */
+ const char *ident; /* context ID if any (just for debugging) */
MemoryContextCallback *reset_cbs; /* list of reset/delete callbacks */
} MemoryContextData;
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index 6e6c6f2..4f2ca26 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -76,6 +76,7 @@ extern void MemoryContextDelete(MemoryContext context);
extern void MemoryContextResetOnly(MemoryContext context);
extern void MemoryContextResetChildren(MemoryContext context);
extern void MemoryContextDeleteChildren(MemoryContext context);
+extern void MemoryContextSetIdentifier(MemoryContext context, const char *id);
extern void MemoryContextSetParent(MemoryContext context,
MemoryContext new_parent);
extern Size GetMemoryChunkSpace(void *pointer);
@@ -91,6 +92,10 @@ extern void MemoryContextCheck(MemoryContext context);
#endif
extern bool MemoryContextContains(MemoryContext context, void *pointer);
+/* Handy macro for copying and assigning context ID ... but note double eval */
+#define MemoryContextCopySetIdentifier(cxt, id) \
+ MemoryContextSetIdentifier(cxt, MemoryContextStrdup(cxt, id))
+
/*
* GetMemoryChunkContext
* Given a currently-allocated chunk, determine the context
@@ -133,11 +138,10 @@ GetMemoryChunkContext(void *pointer)
* specific creation routines, and noplace else.
*/
extern void MemoryContextCreate(MemoryContext node,
- NodeTag tag, Size size, Size nameoffset,
+ NodeTag tag,
const MemoryContextMethods *methods,
MemoryContext parent,
- const char *name,
- int flags);
+ const char *name);
/*
@@ -147,47 +151,38 @@ extern void MemoryContextCreate(MemoryContext node,
/* aset.c */
extern MemoryContext AllocSetContextCreateExtended(MemoryContext parent,
const char *name,
- int flags,
Size minContextSize,
Size initBlockSize,
Size maxBlockSize);
/*
- * This backwards compatibility macro only works for constant context names,
- * and you must specify block sizes with one of the abstraction macros below.
+ * This wrapper macro exists to check for non-constant strings used as context
+ * names; that's no longer supported. (Use MemoryContextSetIdentifier if you
+ * want to provide a variable identifier.) Note you must specify block sizes
+ * with one of the abstraction macros below.
*/
#ifdef HAVE__BUILTIN_CONSTANT_P
#define AllocSetContextCreate(parent, name, allocparams) \
(StaticAssertExpr(__builtin_constant_p(name), \
- "Use AllocSetContextCreateExtended with MEMCONTEXT_COPY_NAME for non-constant context names"), \
- AllocSetContextCreateExtended(parent, name, 0, allocparams))
+ "memory context names must be constant strings"), \
+ AllocSetContextCreateExtended(parent, name, allocparams))
#else
#define AllocSetContextCreate(parent, name, allocparams) \
- AllocSetContextCreateExtended(parent, name, 0, allocparams)
+ AllocSetContextCreateExtended(parent, name, allocparams)
#endif
/* slab.c */
extern MemoryContext SlabContextCreate(MemoryContext parent,
const char *name,
- int flags,
Size blockSize,
Size chunkSize);
/* generation.c */
extern MemoryContext GenerationContextCreate(MemoryContext parent,
const char *name,
- int flags,
Size blockSize);
/*
- * Flag option bits for FooContextCreate functions.
- * In future, some of these might be relevant to only some context types.
- *
- * COPY_NAME: FooContextCreate's name argument is not a constant string
- */
-#define MEMCONTEXT_COPY_NAME 0x0001 /* is passed name transient? */
-
-/*
* Recommended default alloc parameters, suitable for "ordinary" contexts
* that might hold quite a lot of data.
*/
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index d44089a..7c0d665 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -2782,10 +2782,9 @@ compile_plperl_function(Oid fn_oid, bool is_trigger, bool is_event_trigger)
/************************************************************
* Allocate a context that will hold all PG data for the procedure.
************************************************************/
- proc_cxt = AllocSetContextCreateExtended(TopMemoryContext,
- NameStr(procStruct->proname),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ proc_cxt = AllocSetContextCreate(TopMemoryContext,
+ "PL/Perl function",
+ ALLOCSET_SMALL_SIZES);
/************************************************************
* Allocate and fill a new procedure description block.
@@ -2794,6 +2793,7 @@ compile_plperl_function(Oid fn_oid, bool is_trigger, bool is_event_trigger)
oldcontext = MemoryContextSwitchTo(proc_cxt);
prodesc = (plperl_proc_desc *) palloc0(sizeof(plperl_proc_desc));
prodesc->proname = pstrdup(NameStr(procStruct->proname));
+ MemoryContextSetIdentifier(proc_cxt, prodesc->proname);
prodesc->fn_cxt = proc_cxt;
prodesc->fn_refcount = 0;
prodesc->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index b1a0c1c..5993325 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -342,11 +342,12 @@ do_compile(FunctionCallInfo fcinfo,
* per-function memory context, so it can be reclaimed easily.
*/
func_cxt = AllocSetContextCreate(TopMemoryContext,
- "PL/pgSQL function context",
+ "PL/pgSQL function",
ALLOCSET_DEFAULT_SIZES);
plpgsql_compile_tmp_cxt = MemoryContextSwitchTo(func_cxt);
function->fn_signature = format_procedure(fcinfo->flinfo->fn_oid);
+ MemoryContextSetIdentifier(func_cxt, function->fn_signature);
function->fn_oid = fcinfo->flinfo->fn_oid;
function->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
function->fn_tid = procTup->t_self;
diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c
index b4c4dcd..16f1278 100644
--- a/src/pl/plpython/plpy_procedure.c
+++ b/src/pl/plpython/plpy_procedure.c
@@ -164,10 +164,9 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
}
/* Create long-lived context that all procedure info will live in */
- cxt = AllocSetContextCreateExtended(TopMemoryContext,
- procName,
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_DEFAULT_SIZES);
+ cxt = AllocSetContextCreate(TopMemoryContext,
+ "PL/Python function",
+ ALLOCSET_DEFAULT_SIZES);
oldcxt = MemoryContextSwitchTo(cxt);
@@ -183,6 +182,7 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
int i;
proc->proname = pstrdup(NameStr(procStruct->proname));
+ MemoryContextSetIdentifier(cxt, proc->proname);
proc->pyname = pstrdup(procName);
proc->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
proc->fn_tid = procTup->t_self;
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 865071b..558cabc 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -1479,12 +1479,10 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
/************************************************************
* Allocate a context that will hold all PG data for the procedure.
- * We use the internal proc name as the context name.
************************************************************/
- proc_cxt = AllocSetContextCreateExtended(TopMemoryContext,
- internal_proname,
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ proc_cxt = AllocSetContextCreate(TopMemoryContext,
+ "PL/Tcl function",
+ ALLOCSET_SMALL_SIZES);
/************************************************************
* Allocate and fill a new procedure description block.
@@ -1493,6 +1491,7 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
oldcontext = MemoryContextSwitchTo(proc_cxt);
prodesc = (pltcl_proc_desc *) palloc0(sizeof(pltcl_proc_desc));
prodesc->user_proname = pstrdup(NameStr(procStruct->proname));
+ MemoryContextSetIdentifier(proc_cxt, prodesc->user_proname);
prodesc->internal_proname = pstrdup(internal_proname);
prodesc->fn_cxt = proc_cxt;
prodesc->fn_refcount = 0;
It looks much better.
While I didn't do anything about it here, I think it'd likely be a
good idea for MemoryContextStats printout to truncate the context ID
strings at 100 characters or so
It would be great if there was an option to show full sql.
For instance, current statistics is not sorted (should it be?), so it just
prints the first 100 items no matter the size of entries.
Luckily there's a function that accepts "number of nodes to print" as an
argument.
It would be better if it printed the 100 top consumers (including
descendants) though.
I think it makes sense to move all the numerics to the front, so the
numbers are located in the more or less the same columns.
Current output is hard to read as you basically have to search for "free"
and "used" markers.
What do you think of (number of bytes are printed with 6 characters, and
number of chunks with 2 characters)
16384 total in 2 blocks; 12448 used; 3936 free ( 5 chunks):
PL/pgSQL function tg_pslot_biu()
instead of
PL/pgSQL function tg_pslot_biu(): 16384 total in 2 blocks; 3936 free
(5 chunks); 12448 used
?
I think "used memory" is more important than "free". As far as I
understand, the main use-case is "analyze memory consumption", so one cares
"what is consuming the memory" more than "what context have enough free
space".
PS. "TopMemoryContext: 2143904 total" and "Grand total: 13115912 bytes"
does confuse. It basically says "TopMemoryContext is 2MiB, and grant total
is somehow 12MiB". It does not clarify if totals are "self memory" or
"including descendant contexts". In your case the difference is 10MiB, and
it is not that obvious why is that.
Vladimir
Vladimir Sitnikov <sitnikov.vladimir@gmail.com> writes:
While I didn't do anything about it here, I think it'd likely be a
good idea for MemoryContextStats printout to truncate the context ID
strings at 100 characters or so
It would be great if there was an option to show full sql.
Well, as I said, you can do anything you want now in an extension.
But we've had complaints specifically about overly-verbose memory maps
getting spewed to the postmaster log --- that's why there's a default
limit to 100 child contexts now. So I think the standard behavior has
to limit the length of the ID printouts.
(I've since updated my draft patch to do that, and also to convert all
ASCII control characters in an ID to spaces, so that the printouts are
back to a single line per context.)
For instance, current statistics is not sorted (should it be?), so it just
prints the first 100 items no matter the size of entries.
It's not going to be sorted, because of the concerns around not consuming
extra memory when we are reporting an ENOMEM problem. Again, if you're
writing an extension that's going to capture memory usage in non-emergency
scenarios, you can make it do whatever you like.
I think it makes sense to move all the numerics to the front, so the
numbers are located in the more or less the same columns.
I don't agree. Actually the key number is the one that already is printed
first, ie the total space consumed by the context; all the rest is detail.
Very occasionally, you might be interested in spotting contexts that have
a disproportionate amount of free space, but IME that's seldom the main
issue.
There might be an argument for putting the context ID at the end, along
the lines of
PL/pgSQL function: 16384 total in 2 blocks; 6672 free (4 chunks); 9712 used (alter_table_under_transition_tables_upd_func())
or maybe better with a colon instead of parens:
CachedPlan: 8192 total in 4 blocks; 1504 free (0 chunks); 6688 used: SELECT (SELECT COUNT(*) FROM (SELECT * FROM new_test UNION ALL SELECT * FROM new_test) ss)
so that the total is still reasonably prominent --- it's certainly true
that long context IDs are going to make it harder to see that number,
if they're in front. But this doesn't look terribly nice otherwise, so
I'm not sure. Thoughts?
regards, tom lane
Tom>Well, as I said, you can do anything you want now in an extension.
That is true. However it basically means "everybody who cares to
troubleshoot the memory use of a production system should install an
extension".
Should
https://wiki.postgresql.org/wiki/Developer_FAQ#Examining_backend_memory_use
provide
a link to the extension then?
Tom>Actually the key number is the one that already is printed
Tom>first, ie the total space consumed by the context
The space used is more important than the context name itself.
What do you think of
8192 (2 blocks) CachedPlan: 1504 free (0 chunks); 6688 used: SELECT
(SELECT COUNT(*) FROM (SELECT * FROM new_test UNION ALL SELECT *
FROM new_test) ss)
?
PS. "1504 free (0 chunks)" reads odd.
Tom>Very occasionally, you might be interested in spotting contexts that
have
Tom>a disproportionate amount of free space, but IME that's seldom the main
Tom>issue.
Fully agree. That is why I suggest "total, used, free" order so it matches
the likelihood of usage.
Vladimir
Vladimir Sitnikov <sitnikov.vladimir@gmail.com> writes:
Tom>Well, as I said, you can do anything you want now in an extension.
That is true. However it basically means "everybody who cares to
troubleshoot the memory use of a production system should install an
extension".
If you're interested in capturing memory usage short of an ENOMEM
condition, or reporting the results anywhere but stderr, you're going
to need such a thing anyway. There's a lot of use-cases that
MemoryContextStats() doesn't cover, and can't be made to cover without
breaking its essential requirement to report ENOMEM successfully.
What do you think of
8192 (2 blocks) CachedPlan: 1504 free (0 chunks); 6688 used: SELECT
(SELECT COUNT(*) FROM (SELECT * FROM new_test UNION ALL SELECT *
FROM new_test) ss)
Not much. Maybe it's just that I've been looking at the existing output
format for too many years. But given the lack of previous complaints,
I'm disinclined to make large changes in it.
One concrete objection to the above is it'd obscure hierarchical
relationships in the context tree, such as
TopPortalContext: 8192 total in 1 blocks; 7656 free (2 chunks); 536 used
PortalContext: 1024 total in 1 blocks; 584 free (0 chunks); 440 used
ExecutorState: 8192 total in 1 blocks; 2960 free (0 chunks); 5232 used
printtup: 8192 total in 1 blocks; 7936 free (0 chunks); 256 used
ExprContext: 8192 total in 1 blocks; 7656 free (0 chunks); 536 used
regards, tom lane
Tom>One concrete objection to the above is it'd obscure hierarchical
relationships in the context tree,
What is the problem with relationships? Context names are aligned as well
provided 8192 is justified to 6-7-8-9 (you pick) characters.
Tom>But given the lack of previous complaints
1) Here it is:
/messages/by-id/547CEE32.9000606@fuzzy.cz
The log lists "TopMemoryContext: 136614192 total", so everybody follows
"ah, there's 130MiB used" route.
Nobody in the thread mentions 300MiB taken by "ExecutorState: 318758960
total".
It takes just a single pass to compute "total" (and it takes no memory), so
it would be much better if "TopMemoryContext: ..." was replaced with
"Total memory used by all contexts is XXX bytes"
Current TopMemoryContext row is extremely misleading.
2) Here is a complain on "100 contexts" limit:
/messages/by-id/55D7F9CE.3040904@2ndquadrant.com
Note: 100 was invented "at random" in response to "let's not print
everything by default". I do agree with having limit by default, however it
would be so much better
if it selected the rows to print even at a cost of increased CPU cycles for
the print procedure.
For instance: pgjdbc limits to 256 server-prepared statements by
default (per backend). That is current "...Stats" would just ignore at
least half of the prepared statements.
3) If you care so much on the number of passes (frankly speaking, I think
one can easily wait for 5-10 seconds for debugging/troubleshooting stuff),
then aggregate summary can still be computed and printed with no additional
passes (and very limited memory) if the tree is printed in "child-parent"
order.
That is print "parent context" information after children iteration is done.
PS. SQL text might involve sensitive information (e.g. logins, passwords,
personal IDs), so there might be security issues with printing SQL by
default.
Vladimir
On 24 March 2018 at 02:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andres Freund <andres@anarazel.de> writes:
On 2018-03-23 18:05:38 +0000, Vladimir Sitnikov wrote:
For instance, I assume statament cache is stored in some sort of a hash
table, so there should be a way to enumerate it in a programmatic way.Of
course it would take time, however I do not think it creates cpu/memory
overheads. The overhead is to maintain "walker" code.Sure, you could, entirely independent of the memory stats dump, do
that. But what information would you actually gain from it? Which row
something in the catcache belongs to isn't *that* interesting.It'd certainly be easy to define this in a way that makes it require a bunch of support code, which we'd be unlikely to want to write and maintain. However, I've often wished that the contexts in a memory dump were less anonymous. If you didn't just see a pile of "PL/pgSQL function context" entries, but could (say) see the name of each function, that would be a big step forward. Similarly, if we could see the source text for each CachedPlanSource in a dump, that'd be useful. I mention these things because we do actually store them already, in many cases --- but the memory stats code doesn't know about them.Now, commit 9fa6f00b1 already introduced a noticeable penalty for
contexts with nonconstant names, so trying to stick extra info like
this into the context name is not appetizing. But what if we allowed
the context name to have two parts, a fixed part and a variable part?
We could actually require that the fixed part be a compile-time-constant
string, simplifying matters on that end. The variable part would best
be assigned later than initial context creation, because you'd need a
chance to copy the string into the context before pointing to it.
So maybe, for contexts where this is worth doing, it'd look something
like this for plpgsql:func_cxt = AllocSetContextCreate(TopMemoryContext,
"PL/pgSQL function context",
ALLOCSET_DEFAULT_SIZES);
plpgsql_compile_tmp_cxt = MemoryContextSwitchTo(func_cxt);function->fn_signature = format_procedure(fcinfo->flinfo->fn_oid);
+ MemoryContextSetIdentifier(func_cxt, function->fn_signature);
function->fn_oid = fcinfo->flinfo->fn_oid;
function->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
I'm a big fan of this, having stared at way too many dumps of "no idea
what's going on in there" memory usage.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On 24 March 2018 at 03:01, Andres Freund <andres@anarazel.de> wrote:
Hi,
On 2018-03-23 14:33:25 -0400, Tom Lane wrote:
func_cxt = AllocSetContextCreate(TopMemoryContext,
"PL/pgSQL function context",
ALLOCSET_DEFAULT_SIZES);
plpgsql_compile_tmp_cxt = MemoryContextSwitchTo(func_cxt);function->fn_signature = format_procedure(fcinfo->flinfo->fn_oid);
+ MemoryContextSetIdentifier(func_cxt, function->fn_signature);
function->fn_oid = fcinfo->flinfo->fn_oid;
function->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);This would cost an extra char * field in struct MemoryContextData,
which is slightly annoying but it doesn't exactly seem like a killer.
Then the memory stats dump code would just need to know to print this
field if it isn't NULL.That's not a bad idea. How about storing a Node* instead of a char*?
Then we could have MemoryContextStats etc support digging out details
for a few types, without having to generate strings at runtime.
That'd render it pretty useless for extensions, though.
I like the idea of being able to introspect state for particular kinds of
contexts, and not have to generate strings that 99.99% of the time won't
get get looked at.
Function pointers instead of char* ? It adds a significant potential
stability risk to MemoryContextStats() calls, but a great deal of
flexibility.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Vladimir Sitnikov <sitnikov.vladimir@gmail.com> writes:
It takes just a single pass to compute "total" (and it takes no memory), so
it would be much better if "TopMemoryContext: ..." was replaced with
"Total memory used by all contexts is XXX bytes"
Current TopMemoryContext row is extremely misleading.
This may or may not be a good thing to do, but in any case it's well
outside the scope of this patch, whose ambition is only to get additional
identification info attached to contexts for which that's useful.
Moreover, seeing how late we are in the v11 cycle, it's hard to justify
doing more; that smells like a new feature and the time for that is past
for this year. The only reason I'm considering this patch now at all
is that it rethinks some API changes we made earlier in v11, and it'd be
nice to avoid an additional round of churn there in v12.
PS. SQL text might involve sensitive information (e.g. logins, passwords,
personal IDs), so there might be security issues with printing SQL by
default.
Indeed, that's something that extensions would need to think about.
I do not believe it's an issue for MemoryContextStats though; the
postmaster log can already contain sensitive data.
regards, tom lane
Here's an updated patch that adjusts the output format per discussion:
- context identifier at the end of the line, so it's easier to see the
numbers
- identifiers truncated at 100 bytes, control characters replaced by
spaces
Also, I hacked things so that dynahash hash tables continue to print
the way they did before, since the hash table name is really what
you want to see there.
Sample output is same test case as last time (dump at end of plpgsql.sql
regression test script).
Barring objection I plan to push this shortly.
regards, tom lane
Attachments:
add-context-identifiers-2.patchtext/x-diff; charset=us-ascii; name=add-context-identifiers-2.patchDownload
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 5d1b902..cfc6201 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -999,7 +999,6 @@ AtStart_Memory(void)
TransactionAbortContext =
AllocSetContextCreateExtended(TopMemoryContext,
"TransactionAbortContext",
- 0,
32 * 1024,
32 * 1024,
32 * 1024);
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index b00a986..39ee773 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -525,10 +525,11 @@ RelationBuildPartitionDesc(Relation rel)
}
/* Now build the actual relcache partition descriptor */
- rel->rd_pdcxt = AllocSetContextCreateExtended(CacheMemoryContext,
- RelationGetRelationName(rel),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_DEFAULT_SIZES);
+ rel->rd_pdcxt = AllocSetContextCreate(CacheMemoryContext,
+ "partition descriptor",
+ ALLOCSET_DEFAULT_SIZES);
+ MemoryContextCopySetIdentifier(rel->rd_pdcxt, RelationGetRelationName(rel));
+
oldcxt = MemoryContextSwitchTo(rel->rd_pdcxt);
result = (PartitionDescData *) palloc0(sizeof(PartitionDescData));
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 280a14a..cfaf32c 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -214,6 +214,9 @@ RelationBuildRowSecurity(Relation relation)
SysScanDesc sscan;
HeapTuple tuple;
+ MemoryContextCopySetIdentifier(rscxt,
+ RelationGetRelationName(relation));
+
rsdesc = MemoryContextAllocZero(rscxt, sizeof(RowSecurityDesc));
rsdesc->rscxt = rscxt;
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 1c00ac9..2354589 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -612,7 +612,7 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
* must be a child of whatever context holds the FmgrInfo.
*/
fcontext = AllocSetContextCreate(finfo->fn_mcxt,
- "SQL function data",
+ "SQL function",
ALLOCSET_DEFAULT_SIZES);
oldcontext = MemoryContextSwitchTo(fcontext);
@@ -635,9 +635,11 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
procedureStruct = (Form_pg_proc) GETSTRUCT(procedureTuple);
/*
- * copy function name immediately for use by error reporting callback
+ * copy function name immediately for use by error reporting callback, and
+ * for use as memory context identifier
*/
fcache->fname = pstrdup(NameStr(procedureStruct->proname));
+ MemoryContextSetIdentifier(fcontext, fcache->fname);
/*
* get the result type from the procedure tuple, and check for polymorphic
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 5ffe638..b4016ed 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -243,19 +243,16 @@ ReorderBufferAllocate(void)
buffer->change_context = SlabContextCreate(new_ctx,
"Change",
- 0,
SLAB_DEFAULT_BLOCK_SIZE,
sizeof(ReorderBufferChange));
buffer->txn_context = SlabContextCreate(new_ctx,
"TXN",
- 0,
SLAB_DEFAULT_BLOCK_SIZE,
sizeof(ReorderBufferTXN));
buffer->tup_context = GenerationContextCreate(new_ctx,
"Tuples",
- 0,
SLAB_LARGE_BLOCK_SIZE);
hash_ctl.keysize = sizeof(TransactionId);
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index d34d5c3..6c836f6 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -74,7 +74,8 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MemoryContext cxt;
MemoryContext oldcxt;
- cxt = AllocSetContextCreate(CurrentMemoryContext, "stats ext",
+ cxt = AllocSetContextCreate(CurrentMemoryContext,
+ "BuildRelationExtStatistics",
ALLOCSET_DEFAULT_SIZES);
oldcxt = MemoryContextSwitchTo(cxt);
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 8d7d8e0..85bb7b91 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -180,6 +180,7 @@ CreateCachedPlan(RawStmt *raw_parse_tree,
plansource->magic = CACHEDPLANSOURCE_MAGIC;
plansource->raw_parse_tree = copyObject(raw_parse_tree);
plansource->query_string = pstrdup(query_string);
+ MemoryContextSetIdentifier(source_context, plansource->query_string);
plansource->commandTag = commandTag;
plansource->param_types = NULL;
plansource->num_params = 0;
@@ -951,6 +952,7 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
plan_context = AllocSetContextCreate(CurrentMemoryContext,
"CachedPlan",
ALLOCSET_START_SMALL_SIZES);
+ MemoryContextCopySetIdentifier(plan_context, plansource->query_string);
/*
* Copy plan into the new context.
@@ -1346,6 +1348,7 @@ CopyCachedPlan(CachedPlanSource *plansource)
newsource->magic = CACHEDPLANSOURCE_MAGIC;
newsource->raw_parse_tree = copyObject(plansource->raw_parse_tree);
newsource->query_string = pstrdup(plansource->query_string);
+ MemoryContextSetIdentifier(source_context, newsource->query_string);
newsource->commandTag = plansource->commandTag;
if (plansource->num_params > 0)
{
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 6ab4db2..12ddabc 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -673,11 +673,12 @@ RelationBuildRuleLock(Relation relation)
/*
* Make the private context. Assume it'll not contain much data.
*/
- rulescxt = AllocSetContextCreateExtended(CacheMemoryContext,
- RelationGetRelationName(relation),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ rulescxt = AllocSetContextCreate(CacheMemoryContext,
+ "relation rules",
+ ALLOCSET_SMALL_SIZES);
relation->rd_rulescxt = rulescxt;
+ MemoryContextCopySetIdentifier(rulescxt,
+ RelationGetRelationName(relation));
/*
* allocate an array to hold the rewrite rules (the array is extended if
@@ -850,10 +851,11 @@ RelationBuildPartitionKey(Relation relation)
if (!HeapTupleIsValid(tuple))
return;
- partkeycxt = AllocSetContextCreateExtended(CurTransactionContext,
- RelationGetRelationName(relation),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ partkeycxt = AllocSetContextCreate(CurTransactionContext,
+ "partition key",
+ ALLOCSET_SMALL_SIZES);
+ MemoryContextCopySetIdentifier(partkeycxt,
+ RelationGetRelationName(relation));
key = (PartitionKey) MemoryContextAllocZero(partkeycxt,
sizeof(PartitionKeyData));
@@ -1531,11 +1533,12 @@ RelationInitIndexAccessInfo(Relation relation)
* a context, and not just a couple of pallocs, is so that we won't leak
* any subsidiary info attached to fmgr lookup records.
*/
- indexcxt = AllocSetContextCreateExtended(CacheMemoryContext,
- RelationGetRelationName(relation),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ indexcxt = AllocSetContextCreate(CacheMemoryContext,
+ "index info",
+ ALLOCSET_SMALL_SIZES);
relation->rd_indexcxt = indexcxt;
+ MemoryContextCopySetIdentifier(indexcxt,
+ RelationGetRelationName(relation));
/*
* Now we can fetch the index AM's API struct
@@ -5505,12 +5508,12 @@ load_relcache_init_file(bool shared)
* prepare index info context --- parameters should match
* RelationInitIndexAccessInfo
*/
- indexcxt =
- AllocSetContextCreateExtended(CacheMemoryContext,
- RelationGetRelationName(rel),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ indexcxt = AllocSetContextCreate(CacheMemoryContext,
+ "index info",
+ ALLOCSET_SMALL_SIZES);
rel->rd_indexcxt = indexcxt;
+ MemoryContextCopySetIdentifier(indexcxt,
+ RelationGetRelationName(rel));
/*
* Now we can fetch the index AM's API struct. (We can't store
diff --git a/src/backend/utils/cache/ts_cache.c b/src/backend/utils/cache/ts_cache.c
index 3d5c194..9734778 100644
--- a/src/backend/utils/cache/ts_cache.c
+++ b/src/backend/utils/cache/ts_cache.c
@@ -294,16 +294,19 @@ lookup_ts_dictionary_cache(Oid dictId)
Assert(!found); /* it wasn't there a moment ago */
/* Create private memory context the first time through */
- saveCtx = AllocSetContextCreateExtended(CacheMemoryContext,
- NameStr(dict->dictname),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ saveCtx = AllocSetContextCreate(CacheMemoryContext,
+ "TS dictionary",
+ ALLOCSET_SMALL_SIZES);
+ MemoryContextCopySetIdentifier(saveCtx, NameStr(dict->dictname));
}
else
{
/* Clear the existing entry's private context */
saveCtx = entry->dictCtx;
- MemoryContextResetAndDeleteChildren(saveCtx);
+ /* Don't let context's ident pointer dangle while we reset it */
+ MemoryContextSetIdentifier(saveCtx, NULL);
+ MemoryContextReset(saveCtx);
+ MemoryContextCopySetIdentifier(saveCtx, NameStr(dict->dictname));
}
MemSet(entry, 0, sizeof(TSDictionaryCacheEntry));
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 5281cd5..785e0fa 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -340,11 +340,9 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
CurrentDynaHashCxt = info->hcxt;
else
CurrentDynaHashCxt = TopMemoryContext;
- CurrentDynaHashCxt =
- AllocSetContextCreateExtended(CurrentDynaHashCxt,
- tabname,
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_DEFAULT_SIZES);
+ CurrentDynaHashCxt = AllocSetContextCreate(CurrentDynaHashCxt,
+ "dynahash",
+ ALLOCSET_DEFAULT_SIZES);
}
/* Initialize the hash header, plus a copy of the table name */
@@ -354,6 +352,10 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
hashp->tabname = (char *) (hashp + 1);
strcpy(hashp->tabname, tabname);
+ /* If we have a private context, label it with hashtable's name */
+ if (!(flags & HASH_SHARED_MEM))
+ MemoryContextSetIdentifier(CurrentDynaHashCxt, hashp->tabname);
+
/*
* Select the appropriate hash function (see comments at head of file).
*/
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 3f9b188..e3d2c4e 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -130,7 +130,6 @@ typedef struct AllocSetContext
Size initBlockSize; /* initial block size */
Size maxBlockSize; /* maximum block size */
Size nextBlockSize; /* next block size to allocate */
- Size headerSize; /* allocated size of context header */
Size allocChunkLimit; /* effective chunk size limit */
AllocBlock keeper; /* keep this block over resets */
/* freelist this context could be put in, or -1 if not a candidate: */
@@ -228,10 +227,7 @@ typedef struct AllocChunkData
* only its initial malloc chunk and no others. To be a candidate for a
* freelist, a context must have the same minContextSize/initBlockSize as
* other contexts in the list; but its maxBlockSize is irrelevant since that
- * doesn't affect the size of the initial chunk. Also, candidate contexts
- * *must not* use MEMCONTEXT_COPY_NAME since that would make their header size
- * variable. (We currently insist that all flags be zero, since other flags
- * would likely make the contexts less interchangeable, too.)
+ * doesn't affect the size of the initial chunk.
*
* We currently provide one freelist for ALLOCSET_DEFAULT_SIZES contexts
* and one for ALLOCSET_SMALL_SIZES contexts; the latter works for
@@ -276,7 +272,8 @@ static void AllocSetReset(MemoryContext context);
static void AllocSetDelete(MemoryContext context);
static Size AllocSetGetChunkSpace(MemoryContext context, void *pointer);
static bool AllocSetIsEmpty(MemoryContext context);
-static void AllocSetStats(MemoryContext context, int level, bool print,
+static void AllocSetStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals);
#ifdef MEMORY_CONTEXT_CHECKING
@@ -378,14 +375,11 @@ AllocSetFreeIndex(Size size)
* Create a new AllocSet context.
*
* parent: parent context, or NULL if top-level context
- * name: name of context (for debugging only, need not be unique)
- * flags: bitmask of MEMCONTEXT_XXX option flags
+ * name: name of context (must be statically allocated)
* minContextSize: minimum context size
* initBlockSize: initial allocation block size
* maxBlockSize: maximum allocation block size
*
- * Notes: if flags & MEMCONTEXT_COPY_NAME, the name string will be copied into
- * context-lifespan storage; otherwise, it had better be statically allocated.
* Most callers should abstract the context size parameters using a macro
* such as ALLOCSET_DEFAULT_SIZES. (This is now *required* when going
* through the AllocSetContextCreate macro.)
@@ -393,13 +387,11 @@ AllocSetFreeIndex(Size size)
MemoryContext
AllocSetContextCreateExtended(MemoryContext parent,
const char *name,
- int flags,
Size minContextSize,
Size initBlockSize,
Size maxBlockSize)
{
int freeListIndex;
- Size headerSize;
Size firstBlockSize;
AllocSet set;
AllocBlock block;
@@ -431,12 +423,10 @@ AllocSetContextCreateExtended(MemoryContext parent,
* Check whether the parameters match either available freelist. We do
* not need to demand a match of maxBlockSize.
*/
- if (flags == 0 &&
- minContextSize == ALLOCSET_DEFAULT_MINSIZE &&
+ if (minContextSize == ALLOCSET_DEFAULT_MINSIZE &&
initBlockSize == ALLOCSET_DEFAULT_INITSIZE)
freeListIndex = 0;
- else if (flags == 0 &&
- minContextSize == ALLOCSET_SMALL_MINSIZE &&
+ else if (minContextSize == ALLOCSET_SMALL_MINSIZE &&
initBlockSize == ALLOCSET_SMALL_INITSIZE)
freeListIndex = 1;
else
@@ -462,25 +452,17 @@ AllocSetContextCreateExtended(MemoryContext parent,
/* Reinitialize its header, installing correct name and parent */
MemoryContextCreate((MemoryContext) set,
T_AllocSetContext,
- set->headerSize,
- sizeof(AllocSetContext),
&AllocSetMethods,
parent,
- name,
- flags);
+ name);
return (MemoryContext) set;
}
}
- /* Size of the memory context header, including name storage if needed */
- if (flags & MEMCONTEXT_COPY_NAME)
- headerSize = MAXALIGN(sizeof(AllocSetContext) + strlen(name) + 1);
- else
- headerSize = MAXALIGN(sizeof(AllocSetContext));
-
/* Determine size of initial block */
- firstBlockSize = headerSize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ;
+ firstBlockSize = MAXALIGN(sizeof(AllocSetContext)) +
+ ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ;
if (minContextSize != 0)
firstBlockSize = Max(firstBlockSize, minContextSize);
else
@@ -508,7 +490,7 @@ AllocSetContextCreateExtended(MemoryContext parent,
*/
/* Fill in the initial block's block header */
- block = (AllocBlock) (((char *) set) + headerSize);
+ block = (AllocBlock) (((char *) set) + MAXALIGN(sizeof(AllocSetContext)));
block->aset = set;
block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
block->endptr = ((char *) set) + firstBlockSize;
@@ -529,7 +511,6 @@ AllocSetContextCreateExtended(MemoryContext parent,
set->initBlockSize = initBlockSize;
set->maxBlockSize = maxBlockSize;
set->nextBlockSize = initBlockSize;
- set->headerSize = headerSize;
set->freeListIndex = freeListIndex;
/*
@@ -559,12 +540,9 @@ AllocSetContextCreateExtended(MemoryContext parent,
/* Finally, do the type-independent part of context creation */
MemoryContextCreate((MemoryContext) set,
T_AllocSetContext,
- headerSize,
- sizeof(AllocSetContext),
&AllocSetMethods,
parent,
- name,
- flags);
+ name);
return (MemoryContext) set;
}
@@ -1327,12 +1305,13 @@ AllocSetIsEmpty(MemoryContext context)
* AllocSetStats
* Compute stats about memory consumption of an allocset.
*
- * level: recursion level (0 at top level); used for print indentation.
- * print: true to print stats to stderr.
- * totals: if not NULL, add stats about this allocset into *totals.
+ * printfunc: if not NULL, pass a human-readable stats string to this.
+ * passthru: pass this pointer through to printfunc.
+ * totals: if not NULL, add stats about this context into *totals.
*/
static void
-AllocSetStats(MemoryContext context, int level, bool print,
+AllocSetStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals)
{
AllocSet set = (AllocSet) context;
@@ -1344,7 +1323,7 @@ AllocSetStats(MemoryContext context, int level, bool print,
int fidx;
/* Include context header in totalspace */
- totalspace = set->headerSize;
+ totalspace = MAXALIGN(sizeof(AllocSetContext));
for (block = set->blocks; block != NULL; block = block->next)
{
@@ -1364,16 +1343,15 @@ AllocSetStats(MemoryContext context, int level, bool print,
}
}
- if (print)
+ if (printfunc)
{
- int i;
-
- for (i = 0; i < level; i++)
- fprintf(stderr, " ");
- fprintf(stderr,
- "%s: %zu total in %zd blocks; %zu free (%zd chunks); %zu used\n",
- set->header.name, totalspace, nblocks, freespace, freechunks,
- totalspace - freespace);
+ char stats_string[200];
+
+ snprintf(stats_string, sizeof(stats_string),
+ "%zu total in %zd blocks; %zu free (%zd chunks); %zu used",
+ totalspace, nblocks, freespace, freechunks,
+ totalspace - freespace);
+ printfunc(context, passthru, stats_string);
}
if (totals)
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index 338386a..7ee3c48 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -61,7 +61,6 @@ typedef struct GenerationContext
/* Generational context parameters */
Size blockSize; /* standard block size */
- Size headerSize; /* allocated size of context header */
GenerationBlock *block; /* current (most recently allocated) block */
dlist_head blocks; /* list of blocks */
@@ -154,7 +153,8 @@ static void GenerationReset(MemoryContext context);
static void GenerationDelete(MemoryContext context);
static Size GenerationGetChunkSpace(MemoryContext context, void *pointer);
static bool GenerationIsEmpty(MemoryContext context);
-static void GenerationStats(MemoryContext context, int level, bool print,
+static void GenerationStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals);
#ifdef MEMORY_CONTEXT_CHECKING
@@ -203,14 +203,16 @@ static const MemoryContextMethods GenerationMethods = {
/*
* GenerationContextCreate
* Create a new Generation context.
+ *
+ * parent: parent context, or NULL if top-level context
+ * name: name of context (must be statically allocated)
+ * blockSize: generation block size
*/
MemoryContext
GenerationContextCreate(MemoryContext parent,
const char *name,
- int flags,
Size blockSize)
{
- Size headerSize;
GenerationContext *set;
/* Assert we padded GenerationChunk properly */
@@ -238,13 +240,7 @@ GenerationContextCreate(MemoryContext parent,
* freeing the first generation of allocations.
*/
- /* Size of the memory context header, including name storage if needed */
- if (flags & MEMCONTEXT_COPY_NAME)
- headerSize = MAXALIGN(sizeof(GenerationContext) + strlen(name) + 1);
- else
- headerSize = MAXALIGN(sizeof(GenerationContext));
-
- set = (GenerationContext *) malloc(headerSize);
+ set = (GenerationContext *) malloc(MAXALIGN(sizeof(GenerationContext)));
if (set == NULL)
{
MemoryContextStats(TopMemoryContext);
@@ -262,19 +258,15 @@ GenerationContextCreate(MemoryContext parent,
/* Fill in GenerationContext-specific header fields */
set->blockSize = blockSize;
- set->headerSize = headerSize;
set->block = NULL;
dlist_init(&set->blocks);
/* Finally, do the type-independent part of context creation */
MemoryContextCreate((MemoryContext) set,
T_GenerationContext,
- headerSize,
- sizeof(GenerationContext),
&GenerationMethods,
parent,
- name,
- flags);
+ name);
return (MemoryContext) set;
}
@@ -683,15 +675,16 @@ GenerationIsEmpty(MemoryContext context)
* GenerationStats
* Compute stats about memory consumption of a Generation context.
*
- * level: recursion level (0 at top level); used for print indentation.
- * print: true to print stats to stderr.
- * totals: if not NULL, add stats about this Generation into *totals.
+ * printfunc: if not NULL, pass a human-readable stats string to this.
+ * passthru: pass this pointer through to printfunc.
+ * totals: if not NULL, add stats about this context into *totals.
*
* XXX freespace only accounts for empty space at the end of the block, not
* space of freed chunks (which is unknown).
*/
static void
-GenerationStats(MemoryContext context, int level, bool print,
+GenerationStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals)
{
GenerationContext *set = (GenerationContext *) context;
@@ -703,7 +696,7 @@ GenerationStats(MemoryContext context, int level, bool print,
dlist_iter iter;
/* Include context header in totalspace */
- totalspace = set->headerSize;
+ totalspace = MAXALIGN(sizeof(GenerationContext));
dlist_foreach(iter, &set->blocks)
{
@@ -716,16 +709,15 @@ GenerationStats(MemoryContext context, int level, bool print,
freespace += (block->endptr - block->freeptr);
}
- if (print)
+ if (printfunc)
{
- int i;
-
- for (i = 0; i < level; i++)
- fprintf(stderr, " ");
- fprintf(stderr,
- "Generation: %s: %zu total in %zd blocks (%zd chunks); %zu free (%zd chunks); %zu used\n",
- ((MemoryContext) set)->name, totalspace, nblocks, nchunks, freespace,
- nfreechunks, totalspace - freespace);
+ char stats_string[200];
+
+ snprintf(stats_string, sizeof(stats_string),
+ "%zu total in %zd blocks (%zd chunks); %zu free (%zd chunks); %zu used",
+ totalspace, nblocks, nchunks, freespace,
+ nfreechunks, totalspace - freespace);
+ printfunc(context, passthru, stats_string);
}
if (totals)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index d7baa54..22be722 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -21,6 +21,7 @@
#include "postgres.h"
+#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "utils/memdebug.h"
#include "utils/memutils.h"
@@ -55,6 +56,8 @@ static void MemoryContextCallResetCallbacks(MemoryContext context);
static void MemoryContextStatsInternal(MemoryContext context, int level,
bool print, int max_children,
MemoryContextCounters *totals);
+static void MemoryContextStatsPrint(MemoryContext context, void *passthru,
+ const char *stats_string);
/*
* You should not do memory allocations within a critical section, because
@@ -118,7 +121,6 @@ MemoryContextInit(void)
*/
ErrorContext = AllocSetContextCreateExtended(TopMemoryContext,
"ErrorContext",
- 0,
8 * 1024,
8 * 1024,
8 * 1024);
@@ -296,6 +298,23 @@ MemoryContextCallResetCallbacks(MemoryContext context)
}
/*
+ * MemoryContextSetIdentifier
+ * Set the identifier string for a memory context.
+ *
+ * An identifier can be provided to help distinguish among different contexts
+ * of the same kind in memory context stats dumps. The identifier string
+ * must live at least as long as the context it is for; typically it is
+ * allocated inside that context, so that it automatically goes away on
+ * context deletion. Pass id = NULL to forget any old identifier.
+ */
+void
+MemoryContextSetIdentifier(MemoryContext context, const char *id)
+{
+ AssertArg(MemoryContextIsValid(context));
+ context->ident = id;
+}
+
+/*
* MemoryContextSetParent
* Change a context to belong to a new parent (or no parent).
*
@@ -480,7 +499,10 @@ MemoryContextStatsInternal(MemoryContext context, int level,
AssertArg(MemoryContextIsValid(context));
/* Examine the context itself */
- context->methods->stats(context, level, print, totals);
+ context->methods->stats(context,
+ print ? MemoryContextStatsPrint : NULL,
+ (void *) &level,
+ totals);
/*
* Examine children. If there are more than max_children of them, we do
@@ -532,6 +554,67 @@ MemoryContextStatsInternal(MemoryContext context, int level,
}
/*
+ * MemoryContextStatsPrint
+ * Print callback used by MemoryContextStatsInternal
+ *
+ * For now, the passthru pointer just points to "int level"; later we might
+ * make that more complicated.
+ */
+static void
+MemoryContextStatsPrint(MemoryContext context, void *passthru,
+ const char *stats_string)
+{
+ int level = *(int *) passthru;
+ const char *name = context->name;
+ const char *ident = context->ident;
+ int i;
+
+ /*
+ * It seems preferable to label dynahash contexts with just the hash table
+ * name. Those are already unique enough, so the "dynahash" part isn't
+ * very helpful, and this way is more consistent with pre-v11 practice.
+ */
+ if (ident && strcmp(name, "dynahash") == 0)
+ {
+ name = ident;
+ ident = NULL;
+ }
+
+ for (i = 0; i < level; i++)
+ fprintf(stderr, " ");
+ fprintf(stderr, "%s: %s", name, stats_string);
+ if (ident)
+ {
+ /*
+ * Some contexts may have very long identifiers (e.g., SQL queries).
+ * Arbitrarily truncate at 100 bytes, but be careful not to break
+ * multibyte characters. Also, replace ASCII control characters, such
+ * as newlines, with spaces.
+ */
+ int idlen = strlen(ident);
+ bool truncated = false;
+
+ if (idlen > 100)
+ {
+ idlen = pg_mbcliplen(ident, idlen, 100);
+ truncated = true;
+ }
+ fprintf(stderr, ": ");
+ while (idlen-- > 0)
+ {
+ unsigned char c = *ident++;
+
+ if (c < ' ')
+ c = ' ';
+ fputc(c, stderr);
+ }
+ if (truncated)
+ fprintf(stderr, "...");
+ }
+ fputc('\n', stderr);
+}
+
+/*
* MemoryContextCheck
* Check all chunks in the named context.
*
@@ -612,34 +695,23 @@ MemoryContextContains(MemoryContext context, void *pointer)
*
* node: the as-yet-uninitialized common part of the context header node.
* tag: NodeTag code identifying the memory context type.
- * size: total size of context header including context-type-specific fields,
- * as well as space for the context name if MEMCONTEXT_COPY_NAME is set.
- * nameoffset: where within the "size" space to insert the context name.
* methods: context-type-specific methods (usually statically allocated).
* parent: parent context, or NULL if this will be a top-level context.
- * name: name of context (for debugging only, need not be unique).
- * flags: bitmask of MEMCONTEXT_XXX option flags.
+ * name: name of context (must be statically allocated).
*
* Context routines generally assume that MemoryContextCreate can't fail,
* so this can contain Assert but not elog/ereport.
*/
void
MemoryContextCreate(MemoryContext node,
- NodeTag tag, Size size, Size nameoffset,
+ NodeTag tag,
const MemoryContextMethods *methods,
MemoryContext parent,
- const char *name,
- int flags)
+ const char *name)
{
/* Creating new memory contexts is not allowed in a critical section */
Assert(CritSectionCount == 0);
- /* Check size is sane */
- Assert(nameoffset >= sizeof(MemoryContextData));
- Assert((flags & MEMCONTEXT_COPY_NAME) ?
- size >= nameoffset + strlen(name) + 1 :
- size >= nameoffset);
-
/* Initialize all standard fields of memory context header */
node->type = tag;
node->isReset = true;
@@ -647,22 +719,10 @@ MemoryContextCreate(MemoryContext node,
node->parent = parent;
node->firstchild = NULL;
node->prevchild = NULL;
+ node->name = name;
+ node->ident = NULL;
node->reset_cbs = NULL;
- if (flags & MEMCONTEXT_COPY_NAME)
- {
- /* Insert context name into space reserved for it */
- char *namecopy = ((char *) node) + nameoffset;
-
- node->name = namecopy;
- strcpy(namecopy, name);
- }
- else
- {
- /* Assume the passed-in name is statically allocated */
- node->name = name;
- }
-
/* OK to link node into context tree */
if (parent)
{
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index 58beb64..26b0a15 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -131,7 +131,8 @@ static void SlabReset(MemoryContext context);
static void SlabDelete(MemoryContext context);
static Size SlabGetChunkSpace(MemoryContext context, void *pointer);
static bool SlabIsEmpty(MemoryContext context);
-static void SlabStats(MemoryContext context, int level, bool print,
+static void SlabStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals);
#ifdef MEMORY_CONTEXT_CHECKING
static void SlabCheck(MemoryContext context);
@@ -176,27 +177,22 @@ static const MemoryContextMethods SlabMethods = {
* Create a new Slab context.
*
* parent: parent context, or NULL if top-level context
- * name: name of context (for debugging only, need not be unique)
- * flags: bitmask of MEMCONTEXT_XXX option flags
+ * name: name of context (must be statically allocated)
* blockSize: allocation block size
* chunkSize: allocation chunk size
*
- * Notes: if flags & MEMCONTEXT_COPY_NAME, the name string will be copied into
- * context-lifespan storage; otherwise, it had better be statically allocated.
* The chunkSize may not exceed:
* MAXALIGN_DOWN(SIZE_MAX) - MAXALIGN(sizeof(SlabBlock)) - SLAB_CHUNKHDRSZ
*/
MemoryContext
SlabContextCreate(MemoryContext parent,
const char *name,
- int flags,
Size blockSize,
Size chunkSize)
{
int chunksPerBlock;
Size fullChunkSize;
Size freelistSize;
- Size nameOffset;
Size headerSize;
SlabContext *slab;
int i;
@@ -231,12 +227,8 @@ SlabContextCreate(MemoryContext parent,
* this with the first regular block; not worth the extra complication.
*/
- /* Size of the memory context header, including name storage if needed */
- nameOffset = offsetof(SlabContext, freelist) + freelistSize;
- if (flags & MEMCONTEXT_COPY_NAME)
- headerSize = nameOffset + strlen(name) + 1;
- else
- headerSize = nameOffset;
+ /* Size of the memory context header */
+ headerSize = offsetof(SlabContext, freelist) + freelistSize;
slab = (SlabContext *) malloc(headerSize);
if (slab == NULL)
@@ -270,12 +262,9 @@ SlabContextCreate(MemoryContext parent,
/* Finally, do the type-independent part of context creation */
MemoryContextCreate((MemoryContext) slab,
T_SlabContext,
- headerSize,
- nameOffset,
&SlabMethods,
parent,
- name,
- flags);
+ name);
return (MemoryContext) slab;
}
@@ -634,12 +623,13 @@ SlabIsEmpty(MemoryContext context)
* SlabStats
* Compute stats about memory consumption of a Slab context.
*
- * level: recursion level (0 at top level); used for print indentation.
- * print: true to print stats to stderr.
- * totals: if not NULL, add stats about this Slab into *totals.
+ * printfunc: if not NULL, pass a human-readable stats string to this.
+ * passthru: pass this pointer through to printfunc.
+ * totals: if not NULL, add stats about this context into *totals.
*/
static void
-SlabStats(MemoryContext context, int level, bool print,
+SlabStats(MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals)
{
SlabContext *slab = castNode(SlabContext, context);
@@ -667,14 +657,15 @@ SlabStats(MemoryContext context, int level, bool print,
}
}
- if (print)
+ if (printfunc)
{
- for (i = 0; i < level; i++)
- fprintf(stderr, " ");
- fprintf(stderr,
- "Slab: %s: %zu total in %zd blocks; %zu free (%zd chunks); %zu used\n",
- slab->header.name, totalspace, nblocks, freespace, freechunks,
- totalspace - freespace);
+ char stats_string[200];
+
+ snprintf(stats_string, sizeof(stats_string),
+ "%zu total in %zd blocks; %zu free (%zd chunks); %zu used",
+ totalspace, nblocks, freespace, freechunks,
+ totalspace - freespace);
+ printfunc(context, passthru, stats_string);
}
if (totals)
diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h
index ccb64f0..2a8d83f 100644
--- a/src/include/nodes/memnodes.h
+++ b/src/include/nodes/memnodes.h
@@ -39,18 +39,21 @@ typedef struct MemoryContextCounters
* A logical context in which memory allocations occur.
*
* MemoryContext itself is an abstract type that can have multiple
- * implementations, though for now we have only AllocSetContext.
+ * implementations.
* The function pointers in MemoryContextMethods define one specific
* implementation of MemoryContext --- they are a virtual function table
* in C++ terms.
*
* Node types that are actual implementations of memory contexts must
- * begin with the same fields as MemoryContext.
+ * begin with the same fields as MemoryContextData.
*
* Note: for largely historical reasons, typedef MemoryContext is a pointer
* to the context struct rather than the struct type itself.
*/
+typedef void (*MemoryStatsPrintFunc) (MemoryContext context, void *passthru,
+ const char *stats_string);
+
typedef struct MemoryContextMethods
{
void *(*alloc) (MemoryContext context, Size size);
@@ -61,7 +64,8 @@ typedef struct MemoryContextMethods
void (*delete_context) (MemoryContext context);
Size (*get_chunk_space) (MemoryContext context, void *pointer);
bool (*is_empty) (MemoryContext context);
- void (*stats) (MemoryContext context, int level, bool print,
+ void (*stats) (MemoryContext context,
+ MemoryStatsPrintFunc printfunc, void *passthru,
MemoryContextCounters *totals);
#ifdef MEMORY_CONTEXT_CHECKING
void (*check) (MemoryContext context);
@@ -81,6 +85,7 @@ typedef struct MemoryContextData
MemoryContext prevchild; /* previous child of same parent */
MemoryContext nextchild; /* next child of same parent */
const char *name; /* context name (just for debugging) */
+ const char *ident; /* context ID if any (just for debugging) */
MemoryContextCallback *reset_cbs; /* list of reset/delete callbacks */
} MemoryContextData;
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index 6e6c6f2..4f2ca26 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -76,6 +76,7 @@ extern void MemoryContextDelete(MemoryContext context);
extern void MemoryContextResetOnly(MemoryContext context);
extern void MemoryContextResetChildren(MemoryContext context);
extern void MemoryContextDeleteChildren(MemoryContext context);
+extern void MemoryContextSetIdentifier(MemoryContext context, const char *id);
extern void MemoryContextSetParent(MemoryContext context,
MemoryContext new_parent);
extern Size GetMemoryChunkSpace(void *pointer);
@@ -91,6 +92,10 @@ extern void MemoryContextCheck(MemoryContext context);
#endif
extern bool MemoryContextContains(MemoryContext context, void *pointer);
+/* Handy macro for copying and assigning context ID ... but note double eval */
+#define MemoryContextCopySetIdentifier(cxt, id) \
+ MemoryContextSetIdentifier(cxt, MemoryContextStrdup(cxt, id))
+
/*
* GetMemoryChunkContext
* Given a currently-allocated chunk, determine the context
@@ -133,11 +138,10 @@ GetMemoryChunkContext(void *pointer)
* specific creation routines, and noplace else.
*/
extern void MemoryContextCreate(MemoryContext node,
- NodeTag tag, Size size, Size nameoffset,
+ NodeTag tag,
const MemoryContextMethods *methods,
MemoryContext parent,
- const char *name,
- int flags);
+ const char *name);
/*
@@ -147,47 +151,38 @@ extern void MemoryContextCreate(MemoryContext node,
/* aset.c */
extern MemoryContext AllocSetContextCreateExtended(MemoryContext parent,
const char *name,
- int flags,
Size minContextSize,
Size initBlockSize,
Size maxBlockSize);
/*
- * This backwards compatibility macro only works for constant context names,
- * and you must specify block sizes with one of the abstraction macros below.
+ * This wrapper macro exists to check for non-constant strings used as context
+ * names; that's no longer supported. (Use MemoryContextSetIdentifier if you
+ * want to provide a variable identifier.) Note you must specify block sizes
+ * with one of the abstraction macros below.
*/
#ifdef HAVE__BUILTIN_CONSTANT_P
#define AllocSetContextCreate(parent, name, allocparams) \
(StaticAssertExpr(__builtin_constant_p(name), \
- "Use AllocSetContextCreateExtended with MEMCONTEXT_COPY_NAME for non-constant context names"), \
- AllocSetContextCreateExtended(parent, name, 0, allocparams))
+ "memory context names must be constant strings"), \
+ AllocSetContextCreateExtended(parent, name, allocparams))
#else
#define AllocSetContextCreate(parent, name, allocparams) \
- AllocSetContextCreateExtended(parent, name, 0, allocparams)
+ AllocSetContextCreateExtended(parent, name, allocparams)
#endif
/* slab.c */
extern MemoryContext SlabContextCreate(MemoryContext parent,
const char *name,
- int flags,
Size blockSize,
Size chunkSize);
/* generation.c */
extern MemoryContext GenerationContextCreate(MemoryContext parent,
const char *name,
- int flags,
Size blockSize);
/*
- * Flag option bits for FooContextCreate functions.
- * In future, some of these might be relevant to only some context types.
- *
- * COPY_NAME: FooContextCreate's name argument is not a constant string
- */
-#define MEMCONTEXT_COPY_NAME 0x0001 /* is passed name transient? */
-
-/*
* Recommended default alloc parameters, suitable for "ordinary" contexts
* that might hold quite a lot of data.
*/
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index d44089a..7c0d665 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -2782,10 +2782,9 @@ compile_plperl_function(Oid fn_oid, bool is_trigger, bool is_event_trigger)
/************************************************************
* Allocate a context that will hold all PG data for the procedure.
************************************************************/
- proc_cxt = AllocSetContextCreateExtended(TopMemoryContext,
- NameStr(procStruct->proname),
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ proc_cxt = AllocSetContextCreate(TopMemoryContext,
+ "PL/Perl function",
+ ALLOCSET_SMALL_SIZES);
/************************************************************
* Allocate and fill a new procedure description block.
@@ -2794,6 +2793,7 @@ compile_plperl_function(Oid fn_oid, bool is_trigger, bool is_event_trigger)
oldcontext = MemoryContextSwitchTo(proc_cxt);
prodesc = (plperl_proc_desc *) palloc0(sizeof(plperl_proc_desc));
prodesc->proname = pstrdup(NameStr(procStruct->proname));
+ MemoryContextSetIdentifier(proc_cxt, prodesc->proname);
prodesc->fn_cxt = proc_cxt;
prodesc->fn_refcount = 0;
prodesc->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index b1a0c1c..96c1622 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -342,11 +342,12 @@ do_compile(FunctionCallInfo fcinfo,
* per-function memory context, so it can be reclaimed easily.
*/
func_cxt = AllocSetContextCreate(TopMemoryContext,
- "PL/pgSQL function context",
+ "PL/pgSQL function",
ALLOCSET_DEFAULT_SIZES);
plpgsql_compile_tmp_cxt = MemoryContextSwitchTo(func_cxt);
function->fn_signature = format_procedure(fcinfo->flinfo->fn_oid);
+ MemoryContextSetIdentifier(func_cxt, function->fn_signature);
function->fn_oid = fcinfo->flinfo->fn_oid;
function->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
function->fn_tid = procTup->t_self;
@@ -2453,7 +2454,7 @@ plpgsql_HashTableInit(void)
memset(&ctl, 0, sizeof(ctl));
ctl.keysize = sizeof(PLpgSQL_func_hashkey);
ctl.entrysize = sizeof(plpgsql_HashEnt);
- plpgsql_HashTable = hash_create("PLpgSQL function cache",
+ plpgsql_HashTable = hash_create("PLpgSQL function hash",
FUNCS_PER_USER,
&ctl,
HASH_ELEM | HASH_BLOBS);
diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c
index b4c4dcd..16f1278 100644
--- a/src/pl/plpython/plpy_procedure.c
+++ b/src/pl/plpython/plpy_procedure.c
@@ -164,10 +164,9 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
}
/* Create long-lived context that all procedure info will live in */
- cxt = AllocSetContextCreateExtended(TopMemoryContext,
- procName,
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_DEFAULT_SIZES);
+ cxt = AllocSetContextCreate(TopMemoryContext,
+ "PL/Python function",
+ ALLOCSET_DEFAULT_SIZES);
oldcxt = MemoryContextSwitchTo(cxt);
@@ -183,6 +182,7 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
int i;
proc->proname = pstrdup(NameStr(procStruct->proname));
+ MemoryContextSetIdentifier(cxt, proc->proname);
proc->pyname = pstrdup(procName);
proc->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
proc->fn_tid = procTup->t_self;
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 865071b..558cabc 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -1479,12 +1479,10 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
/************************************************************
* Allocate a context that will hold all PG data for the procedure.
- * We use the internal proc name as the context name.
************************************************************/
- proc_cxt = AllocSetContextCreateExtended(TopMemoryContext,
- internal_proname,
- MEMCONTEXT_COPY_NAME,
- ALLOCSET_SMALL_SIZES);
+ proc_cxt = AllocSetContextCreate(TopMemoryContext,
+ "PL/Tcl function",
+ ALLOCSET_SMALL_SIZES);
/************************************************************
* Allocate and fill a new procedure description block.
@@ -1493,6 +1491,7 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
oldcontext = MemoryContextSwitchTo(proc_cxt);
prodesc = (pltcl_proc_desc *) palloc0(sizeof(pltcl_proc_desc));
prodesc->user_proname = pstrdup(NameStr(procStruct->proname));
+ MemoryContextSetIdentifier(proc_cxt, prodesc->user_proname);
prodesc->internal_proname = pstrdup(internal_proname);
prodesc->fn_cxt = proc_cxt;
prodesc->fn_refcount = 0;
Great stuff.
My only gripe is the pattern where the identifier needs to be
re-installed when resetting the context. I don't think we need to hold
push for that reason alone, but I bet we'll be revisiting that.
I suppose this infrastructure can be used to implement the idea in
/messages/by-id/CAMsr+YHii-BCC7ddpbb8fpCgzt0wMRt5GYZ0W_kD_Ft8rwWPiQ@mail.gmail.com
in some more acceptable manner. I'm not proposing it for now, just
parking the idea for a future patch.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
My only gripe is the pattern where the identifier needs to be
re-installed when resetting the context. I don't think we need to hold
push for that reason alone, but I bet we'll be revisiting that.
Yeah, that's slightly annoying; if I'd found more than one case of that,
I'd want a better answer. But it seems like contexts that are long-lived
enough to warrant labeling typically don't get reset during their
lifespans, so it shouldn't be a huge problem.
I considered having MemoryContextReset either Assert that context->ident
is NULL, or just forcibly reset it to NULL, thus preventing a dangling
pointer if someone gets this wrong. But that would lock out a perfectly
valid coding pattern where the identifier is in the parent context, so
I'm not convinced it's a good idea.
I suppose this infrastructure can be used to implement the idea in
/messages/by-id/CAMsr+YHii-BCC7ddpbb8fpCgzt0wMRt5GYZ0W_kD_Ft8rwWPiQ@mail.gmail.com
in some more acceptable manner. I'm not proposing it for now, just
parking the idea for a future patch.
Ah, I thought I remembered the callback idea from some previous
discussion, but I'd not located this one. I think I've got a nicer
API for the per-context-type stats functions than what Craig
proposes there, but we could imagine doing this API or something
close to it for MemoryContextStatsInternal. Or, as I mentioned
before, an external caller could just implement the scan over the
context tree for itself, and format the data however it wants.
regards, tom lane
On 3/27/18 12:47, Tom Lane wrote:
Here's an updated patch that adjusts the output format per discussion:
- context identifier at the end of the line, so it's easier to see the
numbers- identifiers truncated at 100 bytes, control characters replaced by
spacesAlso, I hacked things so that dynahash hash tables continue to print
the way they did before, since the hash table name is really what
you want to see there.Sample output is same test case as last time (dump at end of plpgsql.sql
regression test script).Barring objection I plan to push this shortly.
Cool.
How about this one as well:
diff --git a/src/backend/utils/mmgr/portalmem.c
b/src/backend/utils/mmgr/portalmem.c
index 75a6dde32b..c08dc260e2 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -200,6 +200,7 @@ CreatePortal(const char *name, bool allowDup, bool
dupSilent)
portal->portalContext = AllocSetContextCreate(TopPortalContext,
"PortalContext",
ALLOCSET_SMALL_SIZES);
+ MemoryContextCopySetIdentifier(portal->portalContext, name);
/* create a resource owner for the portal */
portal->resowner = ResourceOwnerCreate(CurTransactionResourceOwner,
The term CopySetIdentifier has confused me a bit. (What's a "set
identifier"?) Maybe use CopyAndSetIdentifier? (We similarly have
MemoryContextResetAndDeleteChildren.)
I'm also not clear why this doesn't undo the previous optimization that
preferred making the identifier a compile time-constant. Aren't we now
just going back to doing a pstrdup() every time?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
How about this one as well:
portal->portalContext = AllocSetContextCreate(TopPortalContext,
"PortalContext",
ALLOCSET_SMALL_SIZES);
+ MemoryContextCopySetIdentifier(portal->portalContext, name);
Seems reasonable, although I think if you were to delay setting the
name till the end of that function, you could point to portal->name
and avoid the extra pstrdup. Maybe that's useless microoptimization.
The term CopySetIdentifier has confused me a bit. (What's a "set
identifier"?) Maybe use CopyAndSetIdentifier? (We similarly have
MemoryContextResetAndDeleteChildren.)
No objection, do you want to make the change?
I'm also not clear why this doesn't undo the previous optimization that
preferred making the identifier a compile time-constant. Aren't we now
just going back to doing a pstrdup() every time?
Huh? It's not undoing that, it's doubling down on it; the "name" now
*has* to be a compile-time constant. Only for contexts that seem worthy
of carrying extra ID information, which is a small minority, do we bother
setting the ident field. Even for those, in the majority of cases we can
avoid an extra strcpy because the identity info is being carried somewhere
inside the context already.
regards, tom lane
On 3/27/18 19:55, Tom Lane wrote:
Seems reasonable, although I think if you were to delay setting the
name till the end of that function, you could point to portal->name
and avoid the extra pstrdup. Maybe that's useless microoptimization.
done
The term CopySetIdentifier has confused me a bit. (What's a "set
identifier"?) Maybe use CopyAndSetIdentifier? (We similarly have
MemoryContextResetAndDeleteChildren.)No objection, do you want to make the change?
and done
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services