From 28f287cdd32f4d3bdde0fd803c9357646e904a17 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 9 Sep 2022 16:19:13 +1200 Subject: [PATCH v1 4/4] Demote MemoryContextContains to MEMORY_CONTEXT_CHECKING builds only Making MemoryContextContains work like it did prior to c6e0fe1f2 required making it perform a loop over each block that the MemoryContext had allocated and check if the pointer is in the address-space range of that block. This was far too expensive to be used as a general-purpose function. Here we instead opt to use GetMemoryChunkContext(). Previously we couldn't use that function as it only works when given a pointer as it was returned by palloc and co. For some use cases of MemoryContextContains they could receive a pointer into a tuple (as was the case for lead/lag window functions, and a pointer to a field in an internal aggregate state, as was the case for int2int4_sum(). --- src/backend/executor/nodeAgg.c | 6 ++---- src/backend/executor/nodeWindowAgg.c | 3 +-- src/backend/utils/mmgr/aset.c | 2 ++ src/backend/utils/mmgr/generation.c | 3 +++ src/backend/utils/mmgr/mcxt.c | 10 ++++++---- src/backend/utils/mmgr/slab.c | 2 ++ src/include/nodes/memnodes.h | 2 +- src/include/utils/memutils.h | 2 +- src/include/utils/memutils_internal.h | 6 +++--- 9 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 933c304901..323ae94938 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1122,8 +1122,7 @@ finalize_aggregate(AggState *aggstate, * If result is pass-by-ref, make sure it is in the right context. */ if (!peragg->resulttypeByVal && !*resultIsNull && - !MemoryContextContains(CurrentMemoryContext, - DatumGetPointer(*resultVal))) + GetMemoryChunkContext(DatumGetPointer(*resultVal)) != CurrentMemoryContext) *resultVal = datumCopy(*resultVal, peragg->resulttypeByVal, peragg->resulttypeLen); @@ -1184,8 +1183,7 @@ finalize_partialaggregate(AggState *aggstate, /* If result is pass-by-ref, make sure it is in the right context. */ if (!peragg->resulttypeByVal && !*resultIsNull && - !MemoryContextContains(CurrentMemoryContext, - DatumGetPointer(*resultVal))) + GetMemoryChunkContext(DatumGetPointer(*resultVal)) != CurrentMemoryContext) *resultVal = datumCopy(*resultVal, peragg->resulttypeByVal, peragg->resulttypeLen); diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index ba119046f4..2820044cd0 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -639,8 +639,7 @@ finalize_windowaggregate(WindowAggState *winstate, * If result is pass-by-ref, make sure it is in the right context. */ if (!peraggstate->resulttypeByVal && !*isnull && - !MemoryContextContains(CurrentMemoryContext, - DatumGetPointer(*result))) + GetMemoryChunkContext(DatumGetPointer(*result)) != CurrentMemoryContext) *result = datumCopy(*result, peraggstate->resulttypeByVal, peraggstate->resulttypeLen); diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index b435624d8f..3a6f12be32 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -1333,6 +1333,7 @@ AllocSetGetChunkContext(void *pointer) return &set->header; } +#ifdef MEMORY_CONTEXT_CHECKING /* * AllocSetContains * Attempt to determine if 'pointer' is memory which was allocated by @@ -1374,6 +1375,7 @@ AllocSetContains(MemoryContext context, void *pointer) } return false; } +#endif /* MEMORY_CONTEXT_CHECKING */ /* * AllocSetGetChunkSpace diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c index f6a3a01912..9c0d04de08 100644 --- a/src/backend/utils/mmgr/generation.c +++ b/src/backend/utils/mmgr/generation.c @@ -848,6 +848,8 @@ GenerationGetChunkContext(void *pointer) return &block->context->header; } + +#ifdef MEMORY_CONTEXT_CHECKING /* * GenerationContains * Attempt to determine if 'pointer' is memory which was allocated by @@ -892,6 +894,7 @@ GenerationContains(MemoryContext context, void *pointer) } return false; } +#endif /* MEMORY_CONTEXT_CHECKING */ /* * GenerationGetChunkSpace diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c index 3615a83d53..1097c1129c 100644 --- a/src/backend/utils/mmgr/mcxt.c +++ b/src/backend/utils/mmgr/mcxt.c @@ -44,12 +44,12 @@ static const MemoryContextMethods mcxt_methods[] = { [MCTX_ASET_ID].reset = AllocSetReset, [MCTX_ASET_ID].delete_context = AllocSetDelete, [MCTX_ASET_ID].get_chunk_context = AllocSetGetChunkContext, - [MCTX_ASET_ID].contains = AllocSetContains, [MCTX_ASET_ID].get_chunk_space = AllocSetGetChunkSpace, [MCTX_ASET_ID].is_empty = AllocSetIsEmpty, [MCTX_ASET_ID].stats = AllocSetStats, #ifdef MEMORY_CONTEXT_CHECKING [MCTX_ASET_ID].check = AllocSetCheck, + [MCTX_ASET_ID].contains = AllocSetContains, #endif /* generation.c */ @@ -59,12 +59,12 @@ static const MemoryContextMethods mcxt_methods[] = { [MCTX_GENERATION_ID].reset = GenerationReset, [MCTX_GENERATION_ID].delete_context = GenerationDelete, [MCTX_GENERATION_ID].get_chunk_context = GenerationGetChunkContext, - [MCTX_GENERATION_ID].contains = GenerationContains, [MCTX_GENERATION_ID].get_chunk_space = GenerationGetChunkSpace, [MCTX_GENERATION_ID].is_empty = GenerationIsEmpty, [MCTX_GENERATION_ID].stats = GenerationStats, #ifdef MEMORY_CONTEXT_CHECKING [MCTX_GENERATION_ID].check = GenerationCheck, + [MCTX_GENERATION_ID].contains = GenerationContains, #endif /* slab.c */ @@ -74,12 +74,12 @@ static const MemoryContextMethods mcxt_methods[] = { [MCTX_SLAB_ID].reset = SlabReset, [MCTX_SLAB_ID].delete_context = SlabDelete, [MCTX_SLAB_ID].get_chunk_context = SlabGetChunkContext, - [MCTX_SLAB_ID].contains = SlabContains, [MCTX_SLAB_ID].get_chunk_space = SlabGetChunkSpace, [MCTX_SLAB_ID].is_empty = SlabIsEmpty, [MCTX_SLAB_ID].stats = SlabStats #ifdef MEMORY_CONTEXT_CHECKING - ,[MCTX_SLAB_ID].check = SlabCheck + ,[MCTX_SLAB_ID].check = SlabCheck, + [MCTX_SLAB_ID].contains = SlabContains #endif }; @@ -816,6 +816,7 @@ MemoryContextCheck(MemoryContext context) } #endif +#ifdef MEMORY_CONTEXT_CHECKING /* * MemoryContextContains * Detect whether an allocated chunk of memory belongs to a given @@ -843,6 +844,7 @@ MemoryContextContains(MemoryContext context, void *pointer) return context->methods->contains(context, pointer); } +#endif /* MEMORY_CONTEXT_CHECKING */ /* * MemoryContextCreate diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c index b5c801eb01..e46f3611ac 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -567,6 +567,7 @@ SlabGetChunkContext(void *pointer) return &slab->header; } +#ifdef MEMORY_CONTEXT_CHECKING /* * SlabContains * Attempt to determine if 'pointer' is memory which was allocated by @@ -610,6 +611,7 @@ SlabContains(MemoryContext context, void *pointer) } return false; } +#endif /* MEMORY_CONTEXT_CHECKING */ /* * SlabGetChunkSpace diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h index 6a8d4f4e4e..6929cc8739 100644 --- a/src/include/nodes/memnodes.h +++ b/src/include/nodes/memnodes.h @@ -64,7 +64,6 @@ typedef struct MemoryContextMethods void (*reset) (MemoryContext context); void (*delete_context) (MemoryContext context); MemoryContext (*get_chunk_context) (void *pointer); - bool (*contains) (MemoryContext context, void *pointer); Size (*get_chunk_space) (void *pointer); bool (*is_empty) (MemoryContext context); void (*stats) (MemoryContext context, @@ -73,6 +72,7 @@ typedef struct MemoryContextMethods bool print_to_stderr); #ifdef MEMORY_CONTEXT_CHECKING void (*check) (MemoryContext context); + bool (*contains) (MemoryContext context, void *pointer); #endif } MemoryContextMethods; diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h index 52bc41ec53..d7144d7307 100644 --- a/src/include/utils/memutils.h +++ b/src/include/utils/memutils.h @@ -95,8 +95,8 @@ extern void MemoryContextAllowInCriticalSection(MemoryContext context, #ifdef MEMORY_CONTEXT_CHECKING extern void MemoryContextCheck(MemoryContext context); -#endif extern bool MemoryContextContains(MemoryContext context, void *pointer); +#endif /* Handy macro for copying and assigning context ID ... but note double eval */ #define MemoryContextCopyAndSetIdentifier(cxt, id) \ diff --git a/src/include/utils/memutils_internal.h b/src/include/utils/memutils_internal.h index ebc1d60393..b767982896 100644 --- a/src/include/utils/memutils_internal.h +++ b/src/include/utils/memutils_internal.h @@ -25,7 +25,6 @@ extern void *AllocSetRealloc(void *pointer, Size size); extern void AllocSetReset(MemoryContext context); extern void AllocSetDelete(MemoryContext context); extern MemoryContext AllocSetGetChunkContext(void *pointer); -extern bool AllocSetContains(MemoryContext context, void *pointer); extern Size AllocSetGetChunkSpace(void *pointer); extern bool AllocSetIsEmpty(MemoryContext context); extern void AllocSetStats(MemoryContext context, @@ -34,6 +33,7 @@ extern void AllocSetStats(MemoryContext context, bool print_to_stderr); #ifdef MEMORY_CONTEXT_CHECKING extern void AllocSetCheck(MemoryContext context); +extern bool AllocSetContains(MemoryContext context, void *pointer); #endif /* These functions implement the MemoryContext API for Generation context. */ @@ -43,7 +43,6 @@ extern void *GenerationRealloc(void *pointer, Size size); extern void GenerationReset(MemoryContext context); extern void GenerationDelete(MemoryContext context); extern MemoryContext GenerationGetChunkContext(void *pointer); -extern bool GenerationContains(MemoryContext context, void *pointer); extern Size GenerationGetChunkSpace(void *pointer); extern bool GenerationIsEmpty(MemoryContext context); extern void GenerationStats(MemoryContext context, @@ -52,6 +51,7 @@ extern void GenerationStats(MemoryContext context, bool print_to_stderr); #ifdef MEMORY_CONTEXT_CHECKING extern void GenerationCheck(MemoryContext context); +extern bool GenerationContains(MemoryContext context, void *pointer); #endif @@ -62,7 +62,6 @@ extern void *SlabRealloc(void *pointer, Size size); extern void SlabReset(MemoryContext context); extern void SlabDelete(MemoryContext context); extern MemoryContext SlabGetChunkContext(void *pointer); -extern bool SlabContains(MemoryContext context, void *pointer); extern Size SlabGetChunkSpace(void *pointer); extern bool SlabIsEmpty(MemoryContext context); extern void SlabStats(MemoryContext context, @@ -71,6 +70,7 @@ extern void SlabStats(MemoryContext context, bool print_to_stderr); #ifdef MEMORY_CONTEXT_CHECKING extern void SlabCheck(MemoryContext context); +extern bool SlabContains(MemoryContext context, void *pointer); #endif /* -- 2.34.1