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: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Rowley <dgrowleyml@gmail.com>,
Tomas Vondra <tomas.vondra@enterprisedb.com>, Andres Freund <andres@anarazel.de>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2024-04-05T13:29:59Z
Lists: pgsql-hackers
Attachments
- repalloc-performance-test-function.patch.txt (text/plain)
- image.png (image/png)
- v5-0002-Introduce-a-bump-memory-allocator.patch (application/octet-stream) patch v5-0002
- v5-0001-Add-bitspace-for-more-memory-context-types-in-Mem.patch (application/octet-stream) patch v5-0001
- v5-0003-Use-bump-memory-context-for-tuplesorts.patch (application/octet-stream) patch v5-0003
On Thu, 4 Apr 2024 at 22:43, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Matthias van de Meent <boekewurm+postgres@gmail.com> writes: > > On Mon, 25 Mar 2024 at 22:44, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> Basically, I'm not happy with consuming the last reasonably-available > >> pattern for a memory context type that has little claim to being the > >> Last Context Type We Will Ever Want. Rather than making a further > >> dent in our ability to detect corrupted chunks, we should do something > >> towards restoring the expansibility that existed in the original > >> design. Then we can add bump contexts and whatever else we want. > > > So, would something like the attached make enough IDs available so > > that we can add the bump context anyway? > > > It extends memory context IDs to 5 bits (32 values), of which > > - 8 have glibc's malloc pattern of 001/010; > > - 1 is unused memory's 00000 > > - 1 is wipe_mem's 11111 > > - 4 are used by existing contexts (Aset/Generation/Slab/AlignedRedirect) > > - 18 are newly available. > > This seems like it would solve the problem for a good long time > to come; and if we ever need more IDs, we could steal one more bit > by requiring the offset to the block header to be a multiple of 8. > (Really, we could just about do that today at little or no cost ... > machines with MAXALIGN less than 8 are very thin on the ground.) Hmm, it seems like a decent idea, but I didn't want to deal with the repercussions of that this late in the cycle when these 2 bits were still relatively easy to get hold of. > The only objection I can think of is that perhaps this would slow > things down a tad by requiring more complicated shifting/masking. > I wonder if we could redo the performance checks that were done > on the way to accepting the current design. I didn't do very extensive testing, but the light performance tests that I did with the palloc performance benchmark patch & script shared above indicate didn't measure an observable negative effect. An adapted version of the test that uses repalloc() to check performance differences in MCXT_METHOD() doesn't show a significant performance difference from master either. That test case is attached as repalloc-performance-test-function.patch.txt. The full set of patches would then accumulate to the attached v5 of the patchset. 0001 is an update of my patch from yesterday, in which I update MemoryContextMethodID infrastructure for more IDs, and use a new naming scheme for unused/reserved IDs. 0002 and 0003 are David's patches, with minor changes to work with 0001 (rebasing, and I moved the location around to keep function declaration in order with memctx ids) Kind regards, Matthias van de Meent
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