v1-0004-Demote-MemoryContextContains-to-MEMORY_CONTEXT_CH.patch

From 28f287cdd32f4d3bdde0fd803c9357646e904a17 Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
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