On 8/31/22 23:46, David Rowley wrote:
> On Thu, 1 Sept 2022 at 08:53, Tomas Vondra
> <tomas.von...@enterprisedb.com> wrote:
>> So the raw size (what we asked for) is ~23.5GB, but in practice we
>> allocate ~28.8GB because of the pow-of-2 logic. And by adding the extra
>> 1B we end up allocating 31.5GB. That doesn't seem like a huge increase,
>> and it's far from the +60% you got.
>>
>> I wonder where does the difference come - I did make installcheck too,
>> so how come you get 10/16GB, and I get 28/31GB? My patch is attached,
>> maybe I did something silly.
> 
> The reason my reported results were lower is because I ignored size >
> allocChunkLimit allocations. These are not raised to the next power of
> 2, so I didn't think they should be included.

If I differentiate the large chunks allocated separately (v2 patch
attached), I get this:

        f        |    t     |  count   |       s1 |       s2 |       s3
-----------------+----------+----------+----------+----------+----------
 AllocSetAlloc   | normal   | 60714914 |     4982 |     6288 |     8185
 AllocSetAlloc   | separate |   824390 |    18245 |    18245 |    18251
 AllocSetRelloc  | normal   |   182070 |      763 |      826 |     1423
 GenerationAlloc | normal   |  2118115 |       68 |       90 |      102
 GenerationAlloc | separate |       28 |        0 |        0 |        0
(5 rows)

Where s1 is the sum of requested sizes, s2 is the sum of allocated
chunks, and s3 is chunks allocated with 1B sentinel.

Focusing on the aset, vast majority of allocations (60M out of 64M) is
small enough to use power-of-2 logic, and we go from 6.3GB to 8.2GB, so
~30%. Not great, not terrible.

For the large allocations, there's almost no increase - in the last
query I used the power-of-2 logic in the calculations, but that was
incorrect, of course.


> 
> I'm not sure why you're seeing only a 3GB additional overhead. I
> noticed a logic error in my query where I was checking
> maxaligned_size=pow2_size and doubling that to give sentinel space.
> That really should have been "case size=pow2_size then pow2_size * 2
> else pow2_size end", However, after adjusting the query, it does not
> seem to change the results much:
> 
> postgres=# select
> postgres-# round(sum(pow2_Size)::numeric/1024/1024/1024,3) as pow2_size,
> postgres-# round(sum(case when size=pow2_size then pow2_size*2 else
> pow2_size end)::numeric/1024/1024/1024,3) as method1,
> postgres-# round(sum(case when size=pow2_size then pow2_size+8 else
> pow2_size end)::numeric/1024/1024/1024,3) as method2
> postgres-# from memstats
> postgres-# where pow2_size > 0;
>  pow2_size | method1 | method2
> -----------+---------+---------
>     10.269 |  16.322 |  10.476
> 
> I've attached the crude patch I came up with for this.  For some
> reason it was crashing on Linux, but it ran ok on Windows, so I used
> the results from that instead.  Maybe that accounts for some
> differences as e.g sizeof(long) == 4 on 64-bit windows. I'd be
> surprised if that accounted for so many GBs though.
> 

I tried to use that patch, but "make installcheck" never completes for
me, for some reason. It seems to get stuck in infinite_recurse.sql, but
I haven't looked into the details.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index b6eeb8abab..b215940062 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -674,6 +674,15 @@ AllocSetDelete(MemoryContext context)
 	free(set);
 }
 
+static Size pow2_size(Size size)
+{
+	Size s = 1;
+	while (s < size)
+		s *= 2;
+
+	return s;
+}
+
 /*
  * AllocSetAlloc
  *		Returns pointer to allocated memory of given size or NULL if
@@ -703,9 +712,12 @@ AllocSetAlloc(MemoryContext context, Size size)
 	 * If requested size exceeds maximum for chunks, allocate an entire block
 	 * for this request.
 	 */
-	if (size > set->allocChunkLimit)
+	if (size + 1 > set->allocChunkLimit)
 	{
-		chunk_size = MAXALIGN(size);
+		fprintf(stderr, "AllocSetAlloc separate %ld %ld %ld\n", size, MAXALIGN(size), MAXALIGN(size+1));
+		fflush(stderr);
+
+		chunk_size = MAXALIGN(size+1);
 		blksize = chunk_size + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ;
 		block = (AllocBlock) malloc(blksize);
 		if (block == NULL)
@@ -726,6 +738,11 @@ AllocSetAlloc(MemoryContext context, Size size)
 		/* set mark to catch clobber of "unused" space */
 		if (size < chunk_size)
 			set_sentinel(MemoryChunkGetPointer(chunk), size);
+		else
+		{
+			fprintf(stderr, "sentinel not added\n");
+			fflush(stderr);
+		}
 #endif
 #ifdef RANDOMIZE_ALLOCATED_MEMORY
 		/* fill the allocated space with junk */
@@ -761,13 +778,16 @@ AllocSetAlloc(MemoryContext context, Size size)
 		return MemoryChunkGetPointer(chunk);
 	}
 
+	fprintf(stderr, "AllocSetAlloc normal %ld %ld %ld\n", size, pow2_size(size), pow2_size(size+1));
+	fflush(stderr);
+
 	/*
 	 * Request is small enough to be treated as a chunk.  Look in the
 	 * corresponding free list to see if there is a free chunk we could reuse.
 	 * If one is found, remove it from the free list, make it again a member
 	 * of the alloc set and return its data address.
 	 */
-	fidx = AllocSetFreeIndex(size);
+	fidx = AllocSetFreeIndex(size+1);
 	chunk = set->freelist[fidx];
 	if (chunk != NULL)
 	{
@@ -785,6 +805,11 @@ AllocSetAlloc(MemoryContext context, Size size)
 		/* set mark to catch clobber of "unused" space */
 		if (size < GetChunkSizeFromFreeListIdx(fidx))
 			set_sentinel(MemoryChunkGetPointer(chunk), size);
+		else
+		{
+			fprintf(stderr, "sentinel not added\n");
+			fflush(stderr);
+		}
 #endif
 #ifdef RANDOMIZE_ALLOCATED_MEMORY
 		/* fill the allocated space with junk */
@@ -951,6 +976,11 @@ AllocSetAlloc(MemoryContext context, Size size)
 	/* set mark to catch clobber of "unused" space */
 	if (size < chunk_size)
 		set_sentinel(MemoryChunkGetPointer(chunk), size);
+	else
+	{
+		fprintf(stderr, "sentinel not added\n");
+		fflush(stderr);
+	}
 #endif
 #ifdef RANDOMIZE_ALLOCATED_MEMORY
 	/* fill the allocated space with junk */
@@ -994,7 +1024,7 @@ AllocSetFree(void *pointer)
 			/* Test for someone scribbling on unused space in chunk */
 			if (chunk->requested_size < chunk_size)
 				if (!sentinel_ok(pointer, chunk->requested_size))
-					elog(WARNING, "detected write past chunk end in %s %p",
+					elog(ERROR, "detected write past chunk end in %s %p",
 						 set->header.name, chunk);
 		}
 #endif
@@ -1034,7 +1064,7 @@ AllocSetFree(void *pointer)
 		/* Test for someone scribbling on unused space in chunk */
 		if (chunk->requested_size < GetChunkSizeFromFreeListIdx(fidx))
 			if (!sentinel_ok(pointer, chunk->requested_size))
-				elog(WARNING, "detected write past chunk end in %s %p",
+				elog(ERROR, "detected write past chunk end in %s %p",
 					 set->header.name, chunk);
 #endif
 
@@ -1078,6 +1108,9 @@ AllocSetRealloc(void *pointer, Size size)
 	MemoryChunk *chunk = PointerGetMemoryChunk(pointer);
 	Size		oldsize;
 
+	fprintf(stderr, "AllocSetRelloc normal %ld %ld %ld\n", size, pow2_size(size), pow2_size(size+1));
+	fflush(stderr);
+
 	/* Allow access to private part of chunk header. */
 	VALGRIND_MAKE_MEM_DEFINED(chunk, ALLOCCHUNK_PRIVATE_LEN);
 
@@ -1100,7 +1133,7 @@ AllocSetRealloc(void *pointer, Size size)
 		/* Test for someone scribbling on unused space in chunk */
 		if (chunk->requested_size < oldsize)
 			if (!sentinel_ok(pointer, chunk->requested_size))
-				elog(WARNING, "detected write past chunk end in %s %p",
+				elog(ERROR, "detected write past chunk end in %s %p",
 					 set->header.name, chunk);
 #endif
 
@@ -1111,7 +1144,7 @@ AllocSetRealloc(void *pointer, Size size)
 		if (block->freeptr != block->endptr)
 			elog(ERROR, "could not find block containing chunk %p", chunk);
 
-		chksize = MAXALIGN(size);
+		chksize = MAXALIGN(size+1);
 
 		/* Do the realloc */
 		blksize = chksize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ;
@@ -1164,6 +1197,11 @@ AllocSetRealloc(void *pointer, Size size)
 		/* set mark to catch clobber of "unused" space */
 		if (size < chksize)
 			set_sentinel(pointer, size);
+		else
+		{
+			fprintf(stderr, "sentinel not added\n");
+			fflush(stderr);
+		}
 #else							/* !MEMORY_CONTEXT_CHECKING */
 
 		/*
@@ -1191,7 +1229,7 @@ AllocSetRealloc(void *pointer, Size size)
 	/* Test for someone scribbling on unused space in chunk */
 	if (chunk->requested_size < oldsize)
 		if (!sentinel_ok(pointer, chunk->requested_size))
-			elog(WARNING, "detected write past chunk end in %s %p",
+			elog(ERROR, "detected write past chunk end in %s %p",
 				 set->header.name, chunk);
 #endif
 
@@ -1200,7 +1238,7 @@ AllocSetRealloc(void *pointer, Size size)
 	 * allocated area already is >= the new size.  (In particular, we will
 	 * fall out here if the requested size is a decrease.)
 	 */
-	if (oldsize >= size)
+	if (oldsize >= size + 1)
 	{
 #ifdef MEMORY_CONTEXT_CHECKING
 		Size		oldrequest = chunk->requested_size;
@@ -1228,6 +1266,11 @@ AllocSetRealloc(void *pointer, Size size)
 		/* set mark to catch clobber of "unused" space */
 		if (size < oldsize)
 			set_sentinel(pointer, size);
+		else
+		{
+			fprintf(stderr, "sentinel not added\n");
+			fflush(stderr);
+		}
 #else							/* !MEMORY_CONTEXT_CHECKING */
 
 		/*
@@ -1532,7 +1575,7 @@ AllocSetCheck(MemoryContext context)
 			 */
 			if (dsize != InvalidAllocSize && dsize < chsize &&
 				!sentinel_ok(chunk, ALLOC_CHUNKHDRSZ + dsize))
-				elog(WARNING, "problem in alloc set %s: detected write past chunk end in block %p, chunk %p",
+				elog(ERROR, "problem in alloc set %s: detected write past chunk end in block %p, chunk %p",
 					 name, block, chunk);
 
 			/*
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index b39894ec94..0029edb31e 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -323,6 +323,15 @@ GenerationDelete(MemoryContext context)
 	free(context);
 }
 
+static Size pow2_size(Size size)
+{
+	Size s = 1;
+	while (s < size)
+		s *= 2;
+
+	return s;
+}
+
 /*
  * GenerationAlloc
  *		Returns pointer to allocated memory of given size or NULL if
@@ -342,7 +351,7 @@ GenerationAlloc(MemoryContext context, Size size)
 	GenerationContext *set = (GenerationContext *) context;
 	GenerationBlock *block;
 	MemoryChunk *chunk;
-	Size		chunk_size = MAXALIGN(size);
+	Size		chunk_size = MAXALIGN(size+1);
 	Size		required_size = chunk_size + Generation_CHUNKHDRSZ;
 
 	/* is it an over-sized chunk? if yes, allocate special block */
@@ -350,6 +359,9 @@ GenerationAlloc(MemoryContext context, Size size)
 	{
 		Size		blksize = required_size + Generation_BLOCKHDRSZ;
 
+		fprintf(stderr, "GenerationAlloc separate %ld %ld %ld\n", size, MAXALIGN(size), MAXALIGN(size+1));
+		fflush(stderr);
+
 		block = (GenerationBlock *) malloc(blksize);
 		if (block == NULL)
 			return NULL;
@@ -375,6 +387,11 @@ GenerationAlloc(MemoryContext context, Size size)
 		/* set mark to catch clobber of "unused" space */
 		if (size < chunk_size)
 			set_sentinel(MemoryChunkGetPointer(chunk), size);
+		else
+		{
+			fprintf(stderr, "sentinel not added\n");
+			fflush(stderr);
+		}
 #endif
 #ifdef RANDOMIZE_ALLOCATED_MEMORY
 		/* fill the allocated space with junk */
@@ -394,6 +411,9 @@ GenerationAlloc(MemoryContext context, Size size)
 		return MemoryChunkGetPointer(chunk);
 	}
 
+	fprintf(stderr, "GenerationAlloc normal %ld %ld %ld\n", size, pow2_size(size), pow2_size(size+1));
+	fflush(stderr);
+
 	/*
 	 * Not an oversized chunk.  We try to first make use of the current block,
 	 * but if there's not enough space in it, instead of allocating a new
@@ -493,6 +513,11 @@ GenerationAlloc(MemoryContext context, Size size)
 	/* set mark to catch clobber of "unused" space */
 	if (size < chunk_size)
 		set_sentinel(MemoryChunkGetPointer(chunk), size);
+	else
+	{
+		fprintf(stderr, "sentinel not added\n");
+		fflush(stderr);
+	}
 #endif
 #ifdef RANDOMIZE_ALLOCATED_MEMORY
 	/* fill the allocated space with junk */
@@ -709,6 +734,9 @@ GenerationRealloc(void *pointer, Size size)
 	GenerationPointer newPointer;
 	Size		oldsize;
 
+	fprintf(stderr, "GenerationRealloc normal %ld %ld %ld\n", size, MAXALIGN(size), MAXALIGN(size+1));
+	fflush(stderr);
+
 	/* Allow access to private part of chunk header. */
 	VALGRIND_MAKE_MEM_DEFINED(chunk, GENERATIONCHUNK_PRIVATE_LEN);
 
@@ -771,6 +799,11 @@ GenerationRealloc(void *pointer, Size size)
 		/* set mark to catch clobber of "unused" space */
 		if (size < oldsize)
 			set_sentinel(pointer, size);
+		else
+		{
+			fprintf(stderr, "sentinel not added\n");
+			fflush(stderr);
+		}
 #else							/* !MEMORY_CONTEXT_CHECKING */
 
 		/*

Reply via email to