Re: Add bump memory context type and use it for tuplesorts
Matthias van de Meent <boekewurm+postgres@gmail.com>
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2024-02-20T11:02:41Z
Lists: pgsql-hackers
On Tue, 20 Feb 2024 at 11:02, David Rowley <dgrowleyml@gmail.com> wrote: > On Fri, 26 Jan 2024 at 01:29, Matthias van de Meent > <boekewurm+postgres@gmail.com> wrote: > > >> + allocSize = MAXALIGN(sizeof(BumpContext)) + Bump_BLOCKHDRSZ + > > >> + if (minContextSize != 0) > > >> + allocSize = Max(allocSize, minContextSize); > > >> + else > > >> + allocSize = Max(allocSize, initBlockSize); > > > > Shouldn't this be the following, considering the meaning of "initBlockSize"? > > No, we want to make the blocksize exactly initBlockSize if we can. Not > initBlockSize plus all the header stuff. We do it that way for all > the other contexts and I agree that it's a good idea as it keeps the > malloc request sizes powers of 2. One part of the reason of my comment was that initBlockSize was ignored in favour of minContextSize if that was configured, regardless of the value of initBlockSize. Is it deliberately ignored when minContextSize is set? > > >> + * BumpFree > > >> + * Unsupported. > > >> [...] > > >> + * BumpRealloc > > >> + * Unsupported. > > > > Rather than the error, can't we make this a no-op (potentially > > optionally, or in a different memory context?) > > Unfortunately not. There are no MemoryChunks on bump chunks so we've > no way to determine the context type a given pointer belongs to. I've > left the MemoryChunk on there for debug builds so we can get the > ERRORs to allow us to fix the broken code that is pfreeing these > chunks. > > > I understand that allowing pfree/repalloc in bump contexts requires > > each allocation to have a MemoryChunk prefix in overhead, but I think > > it's still a valid use case to have a very low overhead allocator with > > no-op deallocator (except context reset). Do you have performance > > comparison results between with and without the overhead of > > MemoryChunk? > > Oh right, you've taken this into account. I was hoping not to have > the headers otherwise the only gains we see over using generation.c is > that of the allocation function being faster. > > [...] The smaller the tuple, the > more that will be noticeable as the chunk header is a larger portion > of the overall allocation with those. I see. Thanks for the explanation. Kind regards, Matthias van de Meent Neon (https://neon.tech)
Commits
-
Update mmgr's README to mention BumpContext
- 58cf2e120e8a 17.0 landed
-
Push dedicated BumpBlocks to the tail of the blocks list
- 6d2fd66b9908 17.0 landed
-
Improve test coverage in bump.c
- bea97cd02ebb 17.0 cited
-
Fix incorrect KeeperBlock macro in bump.c
- 705ec0565371 17.0 landed
-
Use bump memory context for tuplesorts
- 6ed83d5fa55c 17.0 landed
-
Introduce a bump memory allocator
- 29f6a959cfd8 17.0 landed
-
Enlarge bit-space for MemoryContextMethodID
- 0ba8b75e7ea6 17.0 landed