Hi,
I spent a bit of time hacking on the Generation context, adding the two
improvements discussed in this thread:
1) internal handling of block sizes, similar to what AllocSet does (it
pretty much just copies parts of it)
2) keeper block (we keep one empry block instead of freeing it)
3) I've also added allocChunkLimit, which makes it look a bit more like
AllocSet (instead of using just blockSize/8, which does not work too
well with dynamic blockSize)
I haven't done any extensive tests on it, but it does pass check-world
with asserts etc. I haven't touched the comments, those need updating.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
>From 6ca0ae674964157c1f2d2ae446cb415c9be509b6 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Fri, 30 Jul 2021 23:53:52 +0200
Subject: [PATCH 1/3] Generation: grow blocks
---
src/backend/access/gist/gistvacuum.c | 2 +-
.../replication/logical/reorderbuffer.c | 2 +-
src/backend/utils/mmgr/generation.c | 53 ++++++++++++++-----
src/include/utils/memutils.h | 4 +-
4 files changed, 44 insertions(+), 17 deletions(-)
diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c
index 0663193531..1818ed06fc 100644
--- a/src/backend/access/gist/gistvacuum.c
+++ b/src/backend/access/gist/gistvacuum.c
@@ -161,7 +161,7 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
*/
vstate.page_set_context = GenerationContextCreate(CurrentMemoryContext,
"GiST VACUUM page set context",
- 16 * 1024);
+ ALLOCSET_DEFAULT_SIZES);
oldctx = MemoryContextSwitchTo(vstate.page_set_context);
vstate.internal_page_set = intset_create();
vstate.empty_leaf_set = intset_create();
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index f99e45f22d..89018dc99d 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -336,7 +336,7 @@ ReorderBufferAllocate(void)
buffer->tup_context = GenerationContextCreate(new_ctx,
"Tuples",
- SLAB_LARGE_BLOCK_SIZE);
+ ALLOCSET_DEFAULT_SIZES);
hash_ctl.keysize = sizeof(TransactionId);
hash_ctl.entrysize = sizeof(ReorderBufferTXNByIdEnt);
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index 584cd614da..771a2525ca 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -60,7 +60,9 @@ typedef struct GenerationContext
MemoryContextData header; /* Standard memory-context fields */
/* Generational context parameters */
- Size blockSize; /* standard block size */
+ Size initBlockSize; /* initial block size */
+ Size maxBlockSize; /* maximum block size */
+ Size nextBlockSize; /* next block size to allocate */
GenerationBlock *block; /* current (most recently allocated) block */
dlist_head blocks; /* list of blocks */
@@ -196,7 +198,9 @@ static const MemoryContextMethods GenerationMethods = {
MemoryContext
GenerationContextCreate(MemoryContext parent,
const char *name,
- Size blockSize)
+ Size minContextSize,
+ Size initBlockSize,
+ Size maxBlockSize)
{
GenerationContext *set;
@@ -208,16 +212,20 @@ GenerationContextCreate(MemoryContext parent,
"padding calculation in GenerationChunk is wrong");
/*
- * First, validate allocation parameters. (If we're going to throw an
- * error, we should do so before the context is created, not after.) We
- * somewhat arbitrarily enforce a minimum 1K block size, mostly because
- * that's what AllocSet does.
+ * First, validate allocation parameters. Once these were regular runtime
+ * test and elog's, but in practice Asserts seem sufficient because nobody
+ * varies their parameters at runtime. We somewhat arbitrarily enforce a
+ * minimum 1K block size.
*/
- if (blockSize != MAXALIGN(blockSize) ||
- blockSize < 1024 ||
- !AllocHugeSizeIsValid(blockSize))
- elog(ERROR, "invalid blockSize for memory context: %zu",
- blockSize);
+ Assert(initBlockSize == MAXALIGN(initBlockSize) &&
+ initBlockSize >= 1024);
+ Assert(maxBlockSize == MAXALIGN(maxBlockSize) &&
+ maxBlockSize >= initBlockSize &&
+ AllocHugeSizeIsValid(maxBlockSize)); /* must be safe to double */
+ Assert(minContextSize == 0 ||
+ (minContextSize == MAXALIGN(minContextSize) &&
+ minContextSize >= 1024 &&
+ minContextSize <= maxBlockSize));
/*
* Allocate the context header. Unlike aset.c, we never try to combine
@@ -242,7 +250,9 @@ GenerationContextCreate(MemoryContext parent,
*/
/* Fill in GenerationContext-specific header fields */
- set->blockSize = blockSize;
+ set->initBlockSize = initBlockSize;
+ set->maxBlockSize = maxBlockSize;
+ set->nextBlockSize = initBlockSize;
set->block = NULL;
dlist_init(&set->blocks);
@@ -293,6 +303,9 @@ GenerationReset(MemoryContext context)
set->block = NULL;
+ /* Reset block size allocation sequence, too */
+ set->nextBlockSize = set->initBlockSize;
+
Assert(dlist_is_empty(&set->blocks));
}
@@ -329,9 +342,12 @@ GenerationAlloc(MemoryContext context, Size size)
GenerationBlock *block;
GenerationChunk *chunk;
Size chunk_size = MAXALIGN(size);
+ Size blockSize;
+
+ blockSize = (set->block) ? set->block->blksize : set->nextBlockSize;
/* is it an over-sized chunk? if yes, allocate special block */
- if (chunk_size > set->blockSize / 8)
+ if (chunk_size > (blockSize / 8))
{
Size blksize = chunk_size + Generation_BLOCKHDRSZ + Generation_CHUNKHDRSZ;
@@ -387,7 +403,16 @@ GenerationAlloc(MemoryContext context, Size size)
if ((block == NULL) ||
(block->endptr - block->freeptr) < Generation_CHUNKHDRSZ + chunk_size)
{
- Size blksize = set->blockSize;
+ Size blksize;
+
+ /*
+ * The first such block has size initBlockSize, and we double the
+ * space in each succeeding block, but not more than maxBlockSize.
+ */
+ blksize = set->nextBlockSize;
+ set->nextBlockSize <<= 1;
+ if (set->nextBlockSize > set->maxBlockSize)
+ set->nextBlockSize = set->maxBlockSize;
block = (GenerationBlock *) malloc(blksize);
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index ff872274d4..514c0bf75b 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -183,7 +183,9 @@ extern MemoryContext SlabContextCreate(MemoryContext parent,
/* generation.c */
extern MemoryContext GenerationContextCreate(MemoryContext parent,
const char *name,
- Size blockSize);
+ Size minContextSize,
+ Size initBlockSize,
+ Size maxBlockSize);
/*
* Recommended default alloc parameters, suitable for "ordinary" contexts
--
2.31.1
>From 952bd3b480f2a76bb802a6e0433b14dde5f3f461 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Sat, 31 Jul 2021 02:54:36 +0200
Subject: [PATCH 2/3] Generation: keeper block
---
src/backend/utils/mmgr/generation.c | 61 +++++++++++++++++++++++++----
1 file changed, 54 insertions(+), 7 deletions(-)
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index 771a2525ca..6c90416e27 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -65,6 +65,7 @@ typedef struct GenerationContext
Size nextBlockSize; /* next block size to allocate */
GenerationBlock *block; /* current (most recently allocated) block */
+ GenerationBlock *keeper; /* keeper block */
dlist_head blocks; /* list of blocks */
} GenerationContext;
@@ -254,6 +255,7 @@ GenerationContextCreate(MemoryContext parent,
set->maxBlockSize = maxBlockSize;
set->nextBlockSize = initBlockSize;
set->block = NULL;
+ set->keeper = NULL;
dlist_init(&set->blocks);
/* Finally, do the type-independent part of context creation */
@@ -302,6 +304,7 @@ GenerationReset(MemoryContext context)
}
set->block = NULL;
+ set->keeper = NULL;
/* Reset block size allocation sequence, too */
set->nextBlockSize = set->initBlockSize;
@@ -344,7 +347,7 @@ GenerationAlloc(MemoryContext context, Size size)
Size chunk_size = MAXALIGN(size);
Size blockSize;
- blockSize = (set->block) ? set->block->blksize : set->nextBlockSize;
+ blockSize = set->initBlockSize;
/* is it an over-sized chunk? if yes, allocate special block */
if (chunk_size > (blockSize / 8))
@@ -400,6 +403,26 @@ GenerationAlloc(MemoryContext context, Size size)
*/
block = set->block;
+ /*
+ * If we can't use the current block, and we have a keeper block with
+ * enough free space in it, use it as the block.
+ *
+ * XXX We don't want to do this when there's not enough free space
+ * (although the keeper block should be empty, so not sure if checking
+ * the space in the keeper block is necessary).
+ */
+ if (((block == NULL) ||
+ (block->endptr - block->freeptr) < Generation_CHUNKHDRSZ + chunk_size) &&
+ (set->keeper != NULL) &&
+ (set->keeper->endptr - set->keeper->freeptr) < Generation_CHUNKHDRSZ + chunk_size)
+ {
+ block = set->keeper;
+ set->keeper = NULL;
+
+ /* keeper block was not counted as allocated, so add it back */
+ context->mem_allocated += block->blksize;
+ }
+
if ((block == NULL) ||
(block->endptr - block->freeptr) < Generation_CHUNKHDRSZ + chunk_size)
{
@@ -524,16 +547,34 @@ GenerationFree(MemoryContext context, void *pointer)
if (block->nfree < block->nchunks)
return;
+ /* Also make sure the block is not marked as the current block. */
+ if (set->block == block)
+ set->block = NULL;
+
+ /* Keep the block for reuse, if we don't have one already. */
+ if (!set->keeper && block->blksize <= set->maxBlockSize)
+ {
+ /* reset the pointers before we use it as keeper block */
+ block->freeptr = ((char *) block) + Generation_BLOCKHDRSZ;
+ block->endptr = ((char *) block) + block->blksize;
+
+ block->nfree = 0;
+ block->nchunks = 0;
+
+ set->keeper = block;
+
+ /* keeper block is not counted as allocated */
+ context->mem_allocated -= block->blksize;
+
+ return;
+ }
+
/*
* The block is empty, so let's get rid of it. First remove it from the
* list of blocks, then return it to malloc().
*/
dlist_delete(&block->node);
- /* Also make sure the block is not marked as the current block. */
- if (set->block == block)
- set->block = NULL;
-
context->mem_allocated -= block->blksize;
free(block);
}
@@ -770,13 +811,19 @@ GenerationCheck(MemoryContext context)
nchunks;
char *ptr;
- total_allocated += block->blksize;
+ /*
+ * The keeper block in the list of blocks, but we don't consider it
+ * as allocated in memory accounting. So don't include it in the sum.
+ */
+ if (block != gen->keeper)
+ total_allocated += block->blksize;
/*
* nfree > nchunks is surely wrong, and we don't expect to see
* equality either, because such a block should have gotten freed.
*/
- if (block->nfree >= block->nchunks)
+ if ((block->nfree > block->nchunks) &&
+ ((block != gen->keeper) && (block->nfree == block->nchunks)))
elog(WARNING, "problem in Generation %s: number of free chunks %d in block %p exceeds %d allocated",
name, block->nfree, block, block->nchunks);
--
2.31.1
>From ad420a220e67480cbb94a4b60845423ea34c31c1 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Sat, 31 Jul 2021 03:20:56 +0200
Subject: [PATCH 3/3] Generation: allocChunkLimit
---
src/backend/utils/mmgr/generation.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index 6c90416e27..2465598762 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -46,6 +46,8 @@
#define Generation_BLOCKHDRSZ MAXALIGN(sizeof(GenerationBlock))
#define Generation_CHUNKHDRSZ sizeof(GenerationChunk)
+#define Generation_CHUNK_FRACTION 8
+
typedef struct GenerationBlock GenerationBlock; /* forward reference */
typedef struct GenerationChunk GenerationChunk;
@@ -63,6 +65,7 @@ typedef struct GenerationContext
Size initBlockSize; /* initial block size */
Size maxBlockSize; /* maximum block size */
Size nextBlockSize; /* next block size to allocate */
+ Size allocChunkLimit; /* effective chunk size limit */
GenerationBlock *block; /* current (most recently allocated) block */
GenerationBlock *keeper; /* keeper block */
@@ -254,6 +257,17 @@ GenerationContextCreate(MemoryContext parent,
set->initBlockSize = initBlockSize;
set->maxBlockSize = maxBlockSize;
set->nextBlockSize = initBlockSize;
+
+ /*
+ * Compute the allocation chunk size limit for this context.
+ *
+ * Follows similar ideas as AllocSet, see aset.c for details ...
+ */
+ set->allocChunkLimit = 1;
+ while ((Size) (set->allocChunkLimit + Generation_CHUNKHDRSZ) >
+ (Size) ((Size) (maxBlockSize - Generation_BLOCKHDRSZ) / Generation_CHUNK_FRACTION))
+ set->allocChunkLimit >>= 1;
+
set->block = NULL;
set->keeper = NULL;
dlist_init(&set->blocks);
@@ -345,12 +359,9 @@ GenerationAlloc(MemoryContext context, Size size)
GenerationBlock *block;
GenerationChunk *chunk;
Size chunk_size = MAXALIGN(size);
- Size blockSize;
-
- blockSize = set->initBlockSize;
/* is it an over-sized chunk? if yes, allocate special block */
- if (chunk_size > (blockSize / 8))
+ if (chunk_size > set->allocChunkLimit)
{
Size blksize = chunk_size + Generation_BLOCKHDRSZ + Generation_CHUNKHDRSZ;
--
2.31.1
