From eed0152b35ff807b8ca17bd633760aa1cbdbcc89 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Tue, 28 Jan 2020 10:58:05 -0500 Subject: [PATCH v2] Teach MemoryContext infrastructure not to depend on Node. Previously, each type of MemoryContext was a separate type of node. Now, there's a single magic number for all types, and the specific type of context is indicated only by the "methods" pointer. This has a couple of advantages. First, it means that extensions are free to create new memory context tyeps. Second, it means that nodes/memnodes.h no longer needs to depend on nodes/nodes.h, which could be useful if we someday want to allow the use of this infrastructure in frontend code (and reducing dependencies is not a bad thing, anyway). Along the way, convert MemoryContextIsValid() from a macro to an inline function, as per newer practice, and thereby remove the multiple evaluation hazard. Discussion: http://postgr.es/m/CAOP8fzbsfUtka3Lo02guuAWEKFTej6AcsEVQsLmYmBgoMOuRWA@mail.gmail.com --- src/backend/utils/mmgr/aset.c | 2 -- src/backend/utils/mmgr/generation.c | 1 - src/backend/utils/mmgr/mcxt.c | 4 +--- src/backend/utils/mmgr/slab.c | 25 ++++++++++++++++--------- src/include/nodes/memnodes.h | 15 ++++++++------- src/include/nodes/nodes.h | 8 -------- src/include/utils/memutils.h | 1 - 7 files changed, 25 insertions(+), 31 deletions(-) diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index c0623f106d..f447f187a3 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -459,7 +459,6 @@ AllocSetContextCreateInternal(MemoryContext parent, /* Reinitialize its header, installing correct name and parent */ MemoryContextCreate((MemoryContext) set, - T_AllocSetContext, &AllocSetMethods, parent, name); @@ -550,7 +549,6 @@ AllocSetContextCreateInternal(MemoryContext parent, /* Finally, do the type-independent part of context creation */ MemoryContextCreate((MemoryContext) set, - T_AllocSetContext, &AllocSetMethods, parent, name); diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c index 56651d0693..4328e5b228 100644 --- a/src/backend/utils/mmgr/generation.c +++ b/src/backend/utils/mmgr/generation.c @@ -263,7 +263,6 @@ GenerationContextCreate(MemoryContext parent, /* Finally, do the type-independent part of context creation */ MemoryContextCreate((MemoryContext) set, - T_GenerationContext, &GenerationMethods, parent, name); diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c index 9e24fec72d..c8a8f7d90e 100644 --- a/src/backend/utils/mmgr/mcxt.c +++ b/src/backend/utils/mmgr/mcxt.c @@ -736,7 +736,6 @@ MemoryContextContains(MemoryContext context, void *pointer) * sure the node is left in a state that delete will handle. * * node: the as-yet-uninitialized common part of the context header node. - * tag: NodeTag code identifying the memory context type. * 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 (must be statically allocated). @@ -746,7 +745,6 @@ MemoryContextContains(MemoryContext context, void *pointer) */ void MemoryContextCreate(MemoryContext node, - NodeTag tag, const MemoryContextMethods *methods, MemoryContext parent, const char *name) @@ -755,7 +753,7 @@ MemoryContextCreate(MemoryContext node, Assert(CritSectionCount == 0); /* Initialize all standard fields of memory context header */ - node->type = tag; + node->magic = MCXT_MAGIC; node->isReset = true; node->methods = methods; node->parent = parent; diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c index c928476c47..6e2d0a3cab 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -173,6 +173,14 @@ static const MemoryContextMethods SlabMethods = { #define SlabAllocInfo(_cxt, _chunk) #endif +/* Cast MemoryContext to SlabContext * after appropriate sanity checks. */ +static inline SlabContext * +SlabContextFromMemoryContext(MemoryContext context) +{ + Assert(context->magic == MCXT_MAGIC); + Assert(context->methods == &SlabMethods); + return (SlabContext *) context; +} /* * SlabContextCreate @@ -278,7 +286,6 @@ SlabContextCreate(MemoryContext parent, /* Finally, do the type-independent part of context creation */ MemoryContextCreate((MemoryContext) slab, - T_SlabContext, &SlabMethods, parent, name); @@ -297,7 +304,7 @@ static void SlabReset(MemoryContext context) { int i; - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = SlabContextFromMemoryContext(context); Assert(slab); @@ -353,7 +360,7 @@ SlabDelete(MemoryContext context) static void * SlabAlloc(MemoryContext context, Size size) { - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = SlabContextFromMemoryContext(context); SlabBlock *block; SlabChunk *chunk; int idx; @@ -514,7 +521,7 @@ static void SlabFree(MemoryContext context, void *pointer) { int idx; - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = SlabContextFromMemoryContext(context); SlabChunk *chunk = SlabPointerGetChunk(pointer); SlabBlock *block = chunk->block; @@ -603,7 +610,7 @@ SlabFree(MemoryContext context, void *pointer) static void * SlabRealloc(MemoryContext context, void *pointer, Size size) { - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = SlabContextFromMemoryContext(context); Assert(slab); @@ -623,7 +630,7 @@ SlabRealloc(MemoryContext context, void *pointer, Size size) static Size SlabGetChunkSpace(MemoryContext context, void *pointer) { - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = SlabContextFromMemoryContext(context); Assert(slab); @@ -637,7 +644,7 @@ SlabGetChunkSpace(MemoryContext context, void *pointer) static bool SlabIsEmpty(MemoryContext context) { - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = SlabContextFromMemoryContext(context); Assert(slab); @@ -657,7 +664,7 @@ SlabStats(MemoryContext context, MemoryStatsPrintFunc printfunc, void *passthru, MemoryContextCounters *totals) { - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = SlabContextFromMemoryContext(context); Size nblocks = 0; Size freechunks = 0; Size totalspace; @@ -717,7 +724,7 @@ static void SlabCheck(MemoryContext context) { int i; - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = SlabContextFromMemoryContext(context); const char *name = slab->header.name; Assert(slab); diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h index c9f2bbcb36..00298f3d63 100644 --- a/src/include/nodes/memnodes.h +++ b/src/include/nodes/memnodes.h @@ -14,7 +14,8 @@ #ifndef MEMNODES_H #define MEMNODES_H -#include "nodes/nodes.h" +/* Magic number to help us identify memory contexts. */ +#define MCXT_MAGIC 0x56269b04 /* * MemoryContextCounters @@ -75,7 +76,7 @@ typedef struct MemoryContextMethods typedef struct MemoryContextData { - NodeTag type; /* identifies exact kind of context */ + int magic; /* always MCXT_MAGIC */ /* these two fields are placed here to minimize alignment wastage: */ bool isReset; /* T = no space alloced since last reset */ bool allowInCritSection; /* allow palloc in critical section */ @@ -99,10 +100,10 @@ typedef struct MemoryContextData * * Add new context types to the set accepted by this macro. */ -#define MemoryContextIsValid(context) \ - ((context) != NULL && \ - (IsA((context), AllocSetContext) || \ - IsA((context), SlabContext) || \ - IsA((context), GenerationContext))) +static inline bool +MemoryContextIsValid(MemoryContext context) +{ + return context != NULL && context->magic == MCXT_MAGIC; +} #endif /* MEMNODES_H */ diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h index baced7eec0..bd98f7afe2 100644 --- a/src/include/nodes/nodes.h +++ b/src/include/nodes/nodes.h @@ -273,14 +273,6 @@ typedef enum NodeTag T_GroupingSetData, T_StatisticExtInfo, - /* - * TAGS FOR MEMORY NODES (memnodes.h) - */ - T_MemoryContext, - T_AllocSetContext, - T_SlabContext, - T_GenerationContext, - /* * TAGS FOR VALUE NODES (value.h) */ diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h index 909bc2e988..52e5a3f47f 100644 --- a/src/include/utils/memutils.h +++ b/src/include/utils/memutils.h @@ -139,7 +139,6 @@ GetMemoryChunkContext(void *pointer) * specific creation routines, and noplace else. */ extern void MemoryContextCreate(MemoryContext node, - NodeTag tag, const MemoryContextMethods *methods, MemoryContext parent, const char *name); -- 2.17.2 (Apple Git-113)