Subject: Re: HEAPDEBUGALL is broken
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <645c0646-4218-d4c3-409a-a7003a0c108d@2ndquadrant.com>
 <3070eb7d-e06d-31a0-5b68-4ae088fd146c@gmail.com>
 <cffb7d94-e7f4-6d45-bd0b-7cca9e2796fb@2ndquadrant.com>
From: Alexander Lakhin <exclusion@gmail.com>
Message-ID: <8936216c-3492-3f6e-634b-d638fddc5f91@gmail.com>
Date: Wed, 22 Apr 2020 07:00:00 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.4.1
MIME-Version: 1.0
In-Reply-To: <cffb7d94-e7f4-6d45-bd0b-7cca9e2796fb@2ndquadrant.com>
Content-Type: multipart/mixed;
 boundary="------------5B968AA60B5C7D33B230ADE2"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------5B968AA60B5C7D33B230ADE2
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit

21.04.2020 21:01, Peter Eisentraut wrote:
> On 2020-04-19 22:00, Alexander Lakhin wrote:
>> To the point, I've tried to use HAVE_ALLOCINFO on master today and it
>> failed too:
>
> Do you have a proposed patch?
>
As this is broken at least since the invention of the generational
allocator (2017-11-23, a4ccc1ce), I believe than no one uses this (and
slab is broken too). Nonetheless, HAVE_ALLOCINFO in aset.c is still
working, so it could be leaved alone, though the output too chatty for
general use (`make check` produces postmaster log of size 3.8GB). I
think someone would still need to insert some extra conditions to use
that or find another way to debug memory allocations.

So I would just remove this debug macro. The proposed patch is attached.

Best regards,
Alexander

--------------5B968AA60B5C7D33B230ADE2
Content-Type: text/x-patch; charset=UTF-8;
 name="remove-allocinfo.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="remove-allocinfo.patch"

diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index c0623f106d2..dd8d7a33a4f 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -50,9 +50,6 @@
 #include "utils/memdebug.h"
 #include "utils/memutils.h"
 
-/* Define this to detail debug alloc information */
-/* #define HAVE_ALLOCINFO */
-
 /*--------------------
  * Chunk freelist k holds chunks of size 1 << (k + ALLOC_MINBITS),
  * for k = 0 .. ALLOCSET_NUM_FREELISTS-1.
@@ -298,21 +295,6 @@ static const MemoryContextMethods AllocSetMethods = {
 #endif
 };
 
-/* ----------
- * Debug macros
- * ----------
- */
-#ifdef HAVE_ALLOCINFO
-#define AllocFreeInfo(_cxt, _chunk) \
-			fprintf(stderr, "AllocFree: %s: %p, %zu\n", \
-				(_cxt)->header.name, (_chunk), (_chunk)->size)
-#define AllocAllocInfo(_cxt, _chunk) \
-			fprintf(stderr, "AllocAlloc: %s: %p, %zu\n", \
-				(_cxt)->header.name, (_chunk), (_chunk)->size)
-#else
-#define AllocFreeInfo(_cxt, _chunk)
-#define AllocAllocInfo(_cxt, _chunk)
-#endif
 
 /* ----------
  * AllocSetFreeIndex -
@@ -796,8 +778,6 @@ AllocSetAlloc(MemoryContext context, Size size)
 			set->blocks = block;
 		}
 
-		AllocAllocInfo(set, chunk);
-
 		/* Ensure any padding bytes are marked NOACCESS. */
 		VALGRIND_MAKE_MEM_NOACCESS((char *) AllocChunkGetPointer(chunk) + size,
 								   chunk_size - size);
@@ -835,8 +815,6 @@ AllocSetAlloc(MemoryContext context, Size size)
 		randomize_mem((char *) AllocChunkGetPointer(chunk), size);
 #endif
 
-		AllocAllocInfo(set, chunk);
-
 		/* Ensure any padding bytes are marked NOACCESS. */
 		VALGRIND_MAKE_MEM_NOACCESS((char *) AllocChunkGetPointer(chunk) + size,
 								   chunk->size - size);
@@ -996,8 +974,6 @@ AllocSetAlloc(MemoryContext context, Size size)
 	randomize_mem((char *) AllocChunkGetPointer(chunk), size);
 #endif
 
-	AllocAllocInfo(set, chunk);
-
 	/* Ensure any padding bytes are marked NOACCESS. */
 	VALGRIND_MAKE_MEM_NOACCESS((char *) AllocChunkGetPointer(chunk) + size,
 							   chunk_size - size);
@@ -1021,8 +997,6 @@ AllocSetFree(MemoryContext context, void *pointer)
 	/* Allow access to private part of chunk header. */
 	VALGRIND_MAKE_MEM_DEFINED(chunk, ALLOCCHUNK_PRIVATE_LEN);
 
-	AllocFreeInfo(set, chunk);
-
 #ifdef MEMORY_CONTEXT_CHECKING
 	/* Test for someone scribbling on unused space in chunk */
 	if (chunk->requested_size < chunk->size)
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index 56651d06931..af52616e575 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -178,22 +178,6 @@ static const MemoryContextMethods GenerationMethods = {
 #endif
 };
 
-/* ----------
- * Debug macros
- * ----------
- */
-#ifdef HAVE_ALLOCINFO
-#define GenerationFreeInfo(_cxt, _chunk) \
-			fprintf(stderr, "GenerationFree: %s: %p, %lu\n", \
-				(_cxt)->name, (_chunk), (_chunk)->size)
-#define GenerationAllocInfo(_cxt, _chunk) \
-			fprintf(stderr, "GenerationAlloc: %s: %p, %lu\n", \
-				(_cxt)->name, (_chunk), (_chunk)->size)
-#else
-#define GenerationFreeInfo(_cxt, _chunk)
-#define GenerationAllocInfo(_cxt, _chunk)
-#endif
-
 
 /*
  * Public routines
@@ -383,8 +367,6 @@ GenerationAlloc(MemoryContext context, Size size)
 		/* add the block to the list of allocated blocks */
 		dlist_push_head(&set->blocks, &block->node);
 
-		GenerationAllocInfo(set, chunk);
-
 		/* Ensure any padding bytes are marked NOACCESS. */
 		VALGRIND_MAKE_MEM_NOACCESS((char *) GenerationChunkGetPointer(chunk) + size,
 								   chunk_size - size);
@@ -460,8 +442,6 @@ GenerationAlloc(MemoryContext context, Size size)
 	randomize_mem((char *) GenerationChunkGetPointer(chunk), size);
 #endif
 
-	GenerationAllocInfo(set, chunk);
-
 	/* Ensure any padding bytes are marked NOACCESS. */
 	VALGRIND_MAKE_MEM_NOACCESS((char *) GenerationChunkGetPointer(chunk) + size,
 							   chunk_size - size);
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index c928476c479..e9e962d7674 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -157,22 +157,6 @@ static const MemoryContextMethods SlabMethods = {
 #endif
 };
 
-/* ----------
- * Debug macros
- * ----------
- */
-#ifdef HAVE_ALLOCINFO
-#define SlabFreeInfo(_cxt, _chunk) \
-			fprintf(stderr, "SlabFree: %s: %p, %zu\n", \
-				(_cxt)->header.name, (_chunk), (_chunk)->header.size)
-#define SlabAllocInfo(_cxt, _chunk) \
-			fprintf(stderr, "SlabAlloc: %s: %p, %zu\n", \
-				(_cxt)->header.name, (_chunk), (_chunk)->header.size)
-#else
-#define SlabFreeInfo(_cxt, _chunk)
-#define SlabAllocInfo(_cxt, _chunk)
-#endif
-
 
 /*
  * SlabContextCreate
@@ -499,8 +483,6 @@ SlabAlloc(MemoryContext context, Size size)
 	randomize_mem((char *) SlabChunkGetPointer(chunk), size);
 #endif
 
-	SlabAllocInfo(slab, chunk);
-
 	Assert(slab->nblocks * slab->blockSize == context->mem_allocated);
 
 	return SlabChunkGetPointer(chunk);
@@ -518,8 +500,6 @@ SlabFree(MemoryContext context, void *pointer)
 	SlabChunk  *chunk = SlabPointerGetChunk(pointer);
 	SlabBlock  *block = chunk->block;
 
-	SlabFreeInfo(slab, chunk);
-
 #ifdef MEMORY_CONTEXT_CHECKING
 	/* Test for someone scribbling on unused space in chunk */
 	if (slab->chunkSize < (slab->fullChunkSize - sizeof(SlabChunk)))

--------------5B968AA60B5C7D33B230ADE2--