Subject: Re: pgsql: Generational memory allocator
To: Simon Riggs <simon@2ndquadrant.com>, Andres Freund <andres@anarazel.de>
Cc: pgsql-committers <pgsql-committers@postgresql.org>
References: <E1eHa4J-0006hI-Q8@gemulon.postgresql.org>
 <20171123001649.y2nf7422iay6cw3v@alap3.anarazel.de>
 <CANP8+jKoT=Pg5W_3FcKiDf+FMrJhFYacxt_g5E0=tkmb5s8M6g@mail.gmail.com>
From: Tomas Vondra <tv@fuzzy.cz>
Message-ID: <c903f275-2150-fa52-64bf-dca7b53ebf8d@fuzzy.cz>
Date: Thu, 23 Nov 2017 16:16:08 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.3.0
MIME-Version: 1.0
In-Reply-To: 
 <CANP8+jKoT=Pg5W_3FcKiDf+FMrJhFYacxt_g5E0=tkmb5s8M6g@mail.gmail.com>
Content-Type: multipart/mixed;
 boundary="------------CCF850BE5769E8B5375FF91B"
Content-Language: en-US
Precedence: bulk

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


On 11/23/2017 10:57 AM, Simon Riggs wrote:
> On 23 November 2017 at 11:16, Andres Freund <andres@anarazel.de> wrote:
>> Hi,
>>
>> On 2017-11-22 18:48:19 +0000, Simon Riggs wrote:
>>> Generational memory allocator
>>>
>>> Add new style of memory allocator, known as Generational
>>> appropriate for use in cases where memory is allocated
>>> and then freed in roughly oldest first order (FIFO).
>>>
>>> Use new allocator for logical decoding’s reorderbuffer
>>> to significantly reduce memory usage and improve performance.
>>
>> Looks like it's not quite valgrind clean:
>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2017-11-22%2022%3A30%3A01
> 
> It doesn't report anything useful that would allow me to fix this.
> 
> Please enable some info reporting on the animal.
> 

I agree it would be useful to get the valgrind log from the animal, but 
I guess you'd need to get valgrind working to fix the issue anyway.

Anyway, the attached patch fixes the issues for me - strictly speaking 
the Assert is not needed, but it doesn't hurt.

regards
Tomas

--------------CCF850BE5769E8B5375FF91B
Content-Type: text/x-patch;
 name="genalloc-valgrind-fix.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="genalloc-valgrind-fix.patch"

diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index cdff20f..3e3cacd 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -402,9 +402,14 @@ GenerationAlloc(MemoryContext context, Size size)
 
 	chunk = (GenerationChunk *) block->freeptr;
 
+	/* Prepare to initialize the chunk header. */
+	VALGRIND_MAKE_MEM_UNDEFINED(chunk, Generation_CHUNKHDRSZ);
+
 	block->nchunks += 1;
 	block->freeptr += (Generation_CHUNKHDRSZ + chunk_size);
 
+	Assert(block->freeptr <= block->endptr);
+
 	chunk->block = block;
 
 	chunk->context = set;

--------------CCF850BE5769E8B5375FF91B--