Skip to content

Commit 442accc

Browse files
tglsfdctglsfdc
committed
Allow memory contexts to have both fixed and variable ident strings.
Originally, we treated memory context names as potentially variable in all cases, and therefore always copied them into the context header. Commit 9fa6f00 rethought this a little bit and invented a distinction between fixed and variable names, skipping the copy step for the former. But we can make things both simpler and more useful by instead allowing there to be two parts to a context's identification, a fixed "name" and an optional, variable "ident". The name supplied in the context create call is now required to be a compile-time-constant string in all cases, as it is never copied but just pointed to. The "ident" string, if wanted, is supplied later. This is needed because typically we want the ident to be stored inside the context so that it's cleaned up automatically on context deletion; that means it has to be copied into the context before we can set the pointer. The cost of this approach is basically just an additional pointer field in struct MemoryContextData, which isn't much overhead, and is bought back entirely in the AllocSet case by not needing a headerSize field anymore, since we no longer have to cope with variable header length. In addition, we can simplify the internal interfaces for memory context creation still further, saving a few cycles there. And it's no longer true that a custom identifier disqualifies a context from participating in aset.c's freelist scheme, so possibly there's some win on that end. All the places that were using non-compile-time-constant context names are adjusted to put the variable info into the "ident" instead. This allows more effective identification of those contexts in many cases; for example, subsidary contexts of relcache entries are now identified by both type (e.g. "index info") and relname, where before you got only one or the other. Contexts associated with PL function cache entries are now identified more fully and uniformly, too. I also arranged for plancache contexts to use the query source string as their identifier. This is basically free for CachedPlanSources, as they contained a copy of that string already. We pay an extra pstrdup to do it for CachedPlans. That could perhaps be avoided, but it would make things more fragile (since the CachedPlanSource is sometimes destroyed first). I suspect future improvements in error reporting will require CachedPlans to have a copy of that string anyway, so it's not clear that it's worth moving mountains to avoid it now. This also changes the APIs for context statistics routines so that the context-specific routines no longer assume that output goes straight to stderr, nor do they know all details of the output format. This is useful immediately to reduce code duplication, and it also allows for external code to do something with stats output that's different from printing to stderr. The reason for pushing this now rather than waiting for v12 is that it rethinks some of the API changes made by commit 9fa6f00. Seems better for extension authors to endure just one round of API changes not two. Discussion: https://postgr.es/m/CAB=Je-FdtmFZ9y9REHD7VsSrnCkiBhsA4mdsLKSPauwXtQBeNA@mail.gmail.com
1 parent c203d6c commit 442accc

File tree

20 files changed

+263
-210
lines changed

20 files changed

+263
-210
lines changed

‎src/backend/access/transam/xact.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -999,7 +999,6 @@ AtStart_Memory(void)
999999
TransactionAbortContext =
10001000
AllocSetContextCreateExtended(TopMemoryContext,
10011001
"TransactionAbortContext",
1002-
0,
10031002
32 * 1024,
10041003
32 * 1024,
10051004
32 * 1024);

‎src/backend/catalog/partition.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -525,10 +525,11 @@ RelationBuildPartitionDesc(Relation rel)
525525
}
526526

527527
/* Now build the actual relcache partition descriptor */
528-
rel->rd_pdcxt = AllocSetContextCreateExtended(CacheMemoryContext,
529-
RelationGetRelationName(rel),
530-
MEMCONTEXT_COPY_NAME,
531-
ALLOCSET_DEFAULT_SIZES);
528+
rel->rd_pdcxt = AllocSetContextCreate(CacheMemoryContext,
529+
"partition descriptor",
530+
ALLOCSET_DEFAULT_SIZES);
531+
MemoryContextCopySetIdentifier(rel->rd_pdcxt, RelationGetRelationName(rel));
532+
532533
oldcxt = MemoryContextSwitchTo(rel->rd_pdcxt);
533534

534535
result = (PartitionDescData *) palloc0(sizeof(PartitionDescData));

‎src/backend/commands/policy.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,9 @@ RelationBuildRowSecurity(Relation relation)
214214
SysScanDesc sscan;
215215
HeapTuple tuple;
216216

217+
MemoryContextCopySetIdentifier(rscxt,
218+
RelationGetRelationName(relation));
219+
217220
rsdesc = MemoryContextAllocZero(rscxt, sizeof(RowSecurityDesc));
218221
rsdesc->rscxt = rscxt;
219222

‎src/backend/executor/functions.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,7 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
612612
* must be a child of whatever context holds the FmgrInfo.
613613
*/
614614
fcontext = AllocSetContextCreate(finfo->fn_mcxt,
615-
"SQL function data",
615+
"SQL function",
616616
ALLOCSET_DEFAULT_SIZES);
617617

618618
oldcontext = MemoryContextSwitchTo(fcontext);
@@ -635,9 +635,11 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
635635
procedureStruct = (Form_pg_proc) GETSTRUCT(procedureTuple);
636636

637637
/*
638-
* copy function name immediately for use by error reporting callback
638+
* copy function name immediately for use by error reporting callback, and
639+
* for use as memory context identifier
639640
*/
640641
fcache->fname = pstrdup(NameStr(procedureStruct->proname));
642+
MemoryContextSetIdentifier(fcontext, fcache->fname);
641643

642644
/*
643645
* get the result type from the procedure tuple, and check for polymorphic

‎src/backend/replication/logical/reorderbuffer.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,19 +243,16 @@ ReorderBufferAllocate(void)
243243

244244
buffer->change_context = SlabContextCreate(new_ctx,
245245
"Change",
246-
0,
247246
SLAB_DEFAULT_BLOCK_SIZE,
248247
sizeof(ReorderBufferChange));
249248

250249
buffer->txn_context = SlabContextCreate(new_ctx,
251250
"TXN",
252-
0,
253251
SLAB_DEFAULT_BLOCK_SIZE,
254252
sizeof(ReorderBufferTXN));
255253

256254
buffer->tup_context = GenerationContextCreate(new_ctx,
257255
"Tuples",
258-
0,
259256
SLAB_LARGE_BLOCK_SIZE);
260257

261258
hash_ctl.keysize = sizeof(TransactionId);

‎src/backend/statistics/extended_stats.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
7474
MemoryContext cxt;
7575
MemoryContext oldcxt;
7676

77-
cxt = AllocSetContextCreate(CurrentMemoryContext, "stats ext",
77+
cxt = AllocSetContextCreate(CurrentMemoryContext,
78+
"BuildRelationExtStatistics",
7879
ALLOCSET_DEFAULT_SIZES);
7980
oldcxt = MemoryContextSwitchTo(cxt);
8081

‎src/backend/utils/cache/plancache.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ CreateCachedPlan(RawStmt *raw_parse_tree,
180180
plansource->magic = CACHEDPLANSOURCE_MAGIC;
181181
plansource->raw_parse_tree = copyObject(raw_parse_tree);
182182
plansource->query_string = pstrdup(query_string);
183+
MemoryContextSetIdentifier(source_context, plansource->query_string);
183184
plansource->commandTag = commandTag;
184185
plansource->param_types = NULL;
185186
plansource->num_params = 0;
@@ -951,6 +952,7 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
951952
plan_context = AllocSetContextCreate(CurrentMemoryContext,
952953
"CachedPlan",
953954
ALLOCSET_START_SMALL_SIZES);
955+
MemoryContextCopySetIdentifier(plan_context, plansource->query_string);
954956

955957
/*
956958
* Copy plan into the new context.
@@ -1346,6 +1348,7 @@ CopyCachedPlan(CachedPlanSource *plansource)
13461348
newsource->magic = CACHEDPLANSOURCE_MAGIC;
13471349
newsource->raw_parse_tree = copyObject(plansource->raw_parse_tree);
13481350
newsource->query_string = pstrdup(plansource->query_string);
1351+
MemoryContextSetIdentifier(source_context, newsource->query_string);
13491352
newsource->commandTag = plansource->commandTag;
13501353
if (plansource->num_params > 0)
13511354
{

‎src/backend/utils/cache/relcache.c

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -675,11 +675,12 @@ RelationBuildRuleLock(Relation relation)
675675
/*
676676
* Make the private context. Assume it'll not contain much data.
677677
*/
678-
rulescxt = AllocSetContextCreateExtended(CacheMemoryContext,
679-
RelationGetRelationName(relation),
680-
MEMCONTEXT_COPY_NAME,
681-
ALLOCSET_SMALL_SIZES);
678+
rulescxt = AllocSetContextCreate(CacheMemoryContext,
679+
"relation rules",
680+
ALLOCSET_SMALL_SIZES);
682681
relation->rd_rulescxt = rulescxt;
682+
MemoryContextCopySetIdentifier(rulescxt,
683+
RelationGetRelationName(relation));
683684

684685
/*
685686
* allocate an array to hold the rewrite rules (the array is extended if
@@ -852,10 +853,11 @@ RelationBuildPartitionKey(Relation relation)
852853
if (!HeapTupleIsValid(tuple))
853854
return;
854855

855-
partkeycxt = AllocSetContextCreateExtended(CurTransactionContext,
856-
RelationGetRelationName(relation),
857-
MEMCONTEXT_COPY_NAME,
858-
ALLOCSET_SMALL_SIZES);
856+
partkeycxt = AllocSetContextCreate(CurTransactionContext,
857+
"partition key",
858+
ALLOCSET_SMALL_SIZES);
859+
MemoryContextCopySetIdentifier(partkeycxt,
860+
RelationGetRelationName(relation));
859861

860862
key = (PartitionKey) MemoryContextAllocZero(partkeycxt,
861863
sizeof(PartitionKeyData));
@@ -1533,11 +1535,12 @@ RelationInitIndexAccessInfo(Relation relation)
15331535
* a context, and not just a couple of pallocs, is so that we won't leak
15341536
* any subsidiary info attached to fmgr lookup records.
15351537
*/
1536-
indexcxt = AllocSetContextCreateExtended(CacheMemoryContext,
1537-
RelationGetRelationName(relation),
1538-
MEMCONTEXT_COPY_NAME,
1539-
ALLOCSET_SMALL_SIZES);
1538+
indexcxt = AllocSetContextCreate(CacheMemoryContext,
1539+
"index info",
1540+
ALLOCSET_SMALL_SIZES);
15401541
relation->rd_indexcxt = indexcxt;
1542+
MemoryContextCopySetIdentifier(indexcxt,
1543+
RelationGetRelationName(relation));
15411544

15421545
/*
15431546
* Now we can fetch the index AM's API struct
@@ -5603,12 +5606,12 @@ load_relcache_init_file(bool shared)
56035606
* prepare index info context --- parameters should match
56045607
* RelationInitIndexAccessInfo
56055608
*/
5606-
indexcxt =
5607-
AllocSetContextCreateExtended(CacheMemoryContext,
5608-
RelationGetRelationName(rel),
5609-
MEMCONTEXT_COPY_NAME,
5610-
ALLOCSET_SMALL_SIZES);
5609+
indexcxt = AllocSetContextCreate(CacheMemoryContext,
5610+
"index info",
5611+
ALLOCSET_SMALL_SIZES);
56115612
rel->rd_indexcxt = indexcxt;
5613+
MemoryContextCopySetIdentifier(indexcxt,
5614+
RelationGetRelationName(rel));
56125615

56135616
/*
56145617
* Now we can fetch the index AM's API struct. (We can't store

‎src/backend/utils/cache/ts_cache.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -294,16 +294,19 @@ lookup_ts_dictionary_cache(Oid dictId)
294294
Assert(!found); /* it wasn't there a moment ago */
295295

296296
/* Create private memory context the first time through */
297-
saveCtx = AllocSetContextCreateExtended(CacheMemoryContext,
298-
NameStr(dict->dictname),
299-
MEMCONTEXT_COPY_NAME,
300-
ALLOCSET_SMALL_SIZES);
297+
saveCtx = AllocSetContextCreate(CacheMemoryContext,
298+
"TS dictionary",
299+
ALLOCSET_SMALL_SIZES);
300+
MemoryContextCopySetIdentifier(saveCtx, NameStr(dict->dictname));
301301
}
302302
else
303303
{
304304
/* Clear the existing entry's private context */
305305
saveCtx = entry->dictCtx;
306-
MemoryContextResetAndDeleteChildren(saveCtx);
306+
/* Don't let context's ident pointer dangle while we reset it */
307+
MemoryContextSetIdentifier(saveCtx, NULL);
308+
MemoryContextReset(saveCtx);
309+
MemoryContextCopySetIdentifier(saveCtx, NameStr(dict->dictname));
307310
}
308311

309312
MemSet(entry, 0, sizeof(TSDictionaryCacheEntry));

‎src/backend/utils/hash/dynahash.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -340,11 +340,9 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
340340
CurrentDynaHashCxt = info->hcxt;
341341
else
342342
CurrentDynaHashCxt = TopMemoryContext;
343-
CurrentDynaHashCxt =
344-
AllocSetContextCreateExtended(CurrentDynaHashCxt,
345-
tabname,
346-
MEMCONTEXT_COPY_NAME,
347-
ALLOCSET_DEFAULT_SIZES);
343+
CurrentDynaHashCxt = AllocSetContextCreate(CurrentDynaHashCxt,
344+
"dynahash",
345+
ALLOCSET_DEFAULT_SIZES);
348346
}
349347

350348
/* 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)
354352
hashp->tabname = (char *) (hashp + 1);
355353
strcpy(hashp->tabname, tabname);
356354

355+
/* If we have a private context, label it with hashtable's name */
356+
if (!(flags & HASH_SHARED_MEM))
357+
MemoryContextSetIdentifier(CurrentDynaHashCxt, hashp->tabname);
358+
357359
/*
358360
* Select the appropriate hash function (see comments at head of file).
359361
*/

0 commit comments

Comments
 (0)