On Mon, 8 Apr 2024 at 00:37, David Rowley <dgrowle...@gmail.com> wrote: > I've now pushed all 3 patches. Thank you for all the reviews on > these and for the extra MemoryContextMethodID bit, Matthias.
I realised earlier today when working on [1] that bump makes a pretty brain-dead move when adding dedicated blocks to the blocks list. The problem is that I opted to not have a current block field in BumpContext and just rely on the head pointer of the blocks list to be the "current" block. The head block is the block we look at to see if we've any space left when new allocations come in. The problem there is when adding a dedicated block in BumpAllocLarge(), the code adds this to the head of the blocks list so that when a new allocation comes in that's normal-sized, the block at the top of the list is full and we have to create a new block for the allocation. The attached fixes this by pushing these large/dedicated blocks to the *tail* of the blocks list. This means the partially filled block remains at the head and is available for any new allocation which will fit. This behaviour is evident by the regression test change that I added earlier today when working on [1]. The 2nd and smaller allocation in that text goes onto the keeper block rather than a new block. I plan to push this tomorrow. David [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=bea97cd02ebb347ab469b78673c2b33a72109669
From 39a60420a83e5a059de50869c0981be9732909ab Mon Sep 17 00:00:00 2001 From: David Rowley <dgrow...@gmail.com> Date: Tue, 16 Apr 2024 22:26:00 +1200 Subject: [PATCH v1] Push dedicated BumpBlocks to the tail of the blocks list BumpContext relies on using the head block from its 'blocks' field to use as the current block to allocate new chunks to. When we receive an allocation request larger than allocChunkLimit, we place these chunks on a new dedicated block and push this block onto the head of the 'blocks' list. This behavior caused the previous bump block to no longer be available for new normal-sized (non-large) allocations and would result in wasting space on blocks when a large request arrived before the current block was full. Here adjust the code to push these dedicated blocks onto the *tail* of the blocks list so that the head block remains intact and available to be used by normal allocation request sizes. Discussion: https://postgr.es/m/caaphdvp9___r-ayjj0nz6gd3mecgwgz0_6zptwpwj+zqhtm...@mail.gmail.com --- src/backend/utils/mmgr/bump.c | 8 ++++++-- src/test/regress/expected/sysviews.out | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/backend/utils/mmgr/bump.c b/src/backend/utils/mmgr/bump.c index 449bd29344..83798e2245 100644 --- a/src/backend/utils/mmgr/bump.c +++ b/src/backend/utils/mmgr/bump.c @@ -342,8 +342,12 @@ BumpAllocLarge(MemoryContext context, Size size, int flags) randomize_mem((char *) MemoryChunkGetPointer(chunk), size); #endif - /* add the block to the list of allocated blocks */ - dlist_push_head(&set->blocks, &block->node); + /* + * Add the block to the tail of allocated blocks list. The current block + * is left at the head of the list as it may still have space for + * non-large allocations. + */ + dlist_push_tail(&set->blocks, &block->node); #ifdef MEMORY_CONTEXT_CHECKING /* Ensure any padding bytes are marked NOACCESS. */ diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out index 634dc8d8b8..2f3eb4e7f1 100644 --- a/src/test/regress/expected/sysviews.out +++ b/src/test/regress/expected/sysviews.out @@ -47,7 +47,7 @@ select name, parent, total_bytes > 0, total_nblocks, free_bytes > 0, free_chunks from pg_backend_memory_contexts where name = 'Caller tuples'; name | parent | ?column? | total_nblocks | ?column? | free_chunks ---------------+----------------+----------+---------------+----------+------------- - Caller tuples | TupleSort sort | t | 3 | t | 0 + Caller tuples | TupleSort sort | t | 2 | t | 0 (1 row) rollback; -- 2.40.1
