From d22d80e33877cf1982b183a457e17f85683aff6b Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Tue, 28 Jan 2020 10:58:05 -0500 Subject: [PATCH v1] Teach MemoryContext infrastructure not to depend on Node. Previously, each type of MemoryContext was a node type. Now, there's a magic number that begins every MemoryContext, and then a "char" field that identifies the context type. This takes up no more room than before, due to alignment. This has a couple of advantages. First, MemoryContextIsValid() no longer needs to know about all context types, which might be useful for extensions. 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 | 4 +-- src/backend/utils/mmgr/generation.c | 2 +- src/backend/utils/mmgr/mcxt.c | 5 ++-- src/backend/utils/mmgr/slab.c | 18 ++++++------- src/include/nodes/memnodes.h | 42 ++++++++++++++++++++++++----- src/include/nodes/nodes.h | 8 ------ src/include/utils/memutils.h | 2 +- 7 files changed, 51 insertions(+), 30 deletions(-) diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index c0623f106d..122c7d7ffa 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -459,7 +459,7 @@ AllocSetContextCreateInternal(MemoryContext parent, /* Reinitialize its header, installing correct name and parent */ MemoryContextCreate((MemoryContext) set, - T_AllocSetContext, + MemoryContext_AllocSet, &AllocSetMethods, parent, name); @@ -550,7 +550,7 @@ AllocSetContextCreateInternal(MemoryContext parent, /* Finally, do the type-independent part of context creation */ MemoryContextCreate((MemoryContext) set, - T_AllocSetContext, + MemoryContext_AllocSet, &AllocSetMethods, parent, name); diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c index 56651d0693..341723bce9 100644 --- a/src/backend/utils/mmgr/generation.c +++ b/src/backend/utils/mmgr/generation.c @@ -263,7 +263,7 @@ GenerationContextCreate(MemoryContext parent, /* Finally, do the type-independent part of context creation */ MemoryContextCreate((MemoryContext) set, - T_GenerationContext, + MemoryContext_Generation, &GenerationMethods, parent, name); diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c index 9e24fec72d..7b5376316d 100644 --- a/src/backend/utils/mmgr/mcxt.c +++ b/src/backend/utils/mmgr/mcxt.c @@ -746,7 +746,7 @@ MemoryContextContains(MemoryContext context, void *pointer) */ void MemoryContextCreate(MemoryContext node, - NodeTag tag, + MemoryContextType type, const MemoryContextMethods *methods, MemoryContext parent, const char *name) @@ -755,7 +755,8 @@ MemoryContextCreate(MemoryContext node, Assert(CritSectionCount == 0); /* Initialize all standard fields of memory context header */ - node->type = tag; + node->magic = MCXT_MAGIC; + node->type = type; 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..0d8d33bbf2 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -278,7 +278,7 @@ SlabContextCreate(MemoryContext parent, /* Finally, do the type-independent part of context creation */ MemoryContextCreate((MemoryContext) slab, - T_SlabContext, + MemoryContext_Slab, &SlabMethods, parent, name); @@ -297,7 +297,7 @@ static void SlabReset(MemoryContext context) { int i; - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = MemoryContextCast(Slab, context); Assert(slab); @@ -353,7 +353,7 @@ SlabDelete(MemoryContext context) static void * SlabAlloc(MemoryContext context, Size size) { - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = MemoryContextCast(Slab, context); SlabBlock *block; SlabChunk *chunk; int idx; @@ -514,7 +514,7 @@ static void SlabFree(MemoryContext context, void *pointer) { int idx; - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = MemoryContextCast(Slab, context); SlabChunk *chunk = SlabPointerGetChunk(pointer); SlabBlock *block = chunk->block; @@ -603,7 +603,7 @@ SlabFree(MemoryContext context, void *pointer) static void * SlabRealloc(MemoryContext context, void *pointer, Size size) { - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = MemoryContextCast(Slab, context); Assert(slab); @@ -623,7 +623,7 @@ SlabRealloc(MemoryContext context, void *pointer, Size size) static Size SlabGetChunkSpace(MemoryContext context, void *pointer) { - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = MemoryContextCast(Slab, context); Assert(slab); @@ -637,7 +637,7 @@ SlabGetChunkSpace(MemoryContext context, void *pointer) static bool SlabIsEmpty(MemoryContext context) { - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = MemoryContextCast(Slab, context); Assert(slab); @@ -657,7 +657,7 @@ SlabStats(MemoryContext context, MemoryStatsPrintFunc printfunc, void *passthru, MemoryContextCounters *totals) { - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = MemoryContextCast(Slab, context); Size nblocks = 0; Size freechunks = 0; Size totalspace; @@ -717,7 +717,7 @@ static void SlabCheck(MemoryContext context) { int i; - SlabContext *slab = castNode(SlabContext, context); + SlabContext *slab = MemoryContextCast(Slab, 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..e82d5bd0bd 100644 --- a/src/include/nodes/memnodes.h +++ b/src/include/nodes/memnodes.h @@ -14,7 +14,18 @@ #ifndef MEMNODES_H #define MEMNODES_H -#include "nodes/nodes.h" +/* Magic number to help us identify memory contexts. */ +#define MCXT_MAGIC 0x56269b04 + +/* + * List of valid memory context types. + */ +typedef enum +{ + MemoryContext_AllocSet, + MemoryContext_Generation, + MemoryContext_Slab +} MemoryContextType; /* * MemoryContextCounters @@ -75,7 +86,8 @@ typedef struct MemoryContextMethods typedef struct MemoryContextData { - NodeTag type; /* identifies exact kind of context */ + int magic; /* always MCXT_MAGIC */ + char type; /* MemoryContextType for this context */ /* 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 +111,26 @@ 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; +} + +/* + * MemoryContextCast + * Cast a MemoryContext to a context of a given type, checking the type. + * + * NB: The static inline function avoids multiple evaluation. + */ +static inline MemoryContext +MemoryContextCastImpl(MemoryContextType type, MemoryContext context) +{ + Assert(context == NULL || context->type == type); + return context; +} +#define MemoryContextCast(_type_, context) \ + ((_type_##Context *) \ + MemoryContextCastImpl(MemoryContext_##_type_, context)) #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..bd306fec04 100644 --- a/src/include/utils/memutils.h +++ b/src/include/utils/memutils.h @@ -139,7 +139,7 @@ GetMemoryChunkContext(void *pointer) * specific creation routines, and noplace else. */ extern void MemoryContextCreate(MemoryContext node, - NodeTag tag, + MemoryContextType type, const MemoryContextMethods *methods, MemoryContext parent, const char *name); -- 2.17.2 (Apple Git-113)