public inbox for [email protected]
help / color / mirror / Atom feedFrom: David Rowley <[email protected]>
To: Andres Freund <[email protected]>
Cc: [email protected]
Cc: Robert Haas <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Tomas Vondra <[email protected]>
Subject: Re: Avoid stack frame setup in performance critical routines using tail calls
Date: Thu, 29 Feb 2024 00:29:17 +1300
Message-ID: <CAApHDvpHVSJqqb4B4OZLixr=CotKq-eKkbwZqvZVo_biYvUvQA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<CAApHDvrZmQ6RvJ1KDj_v30FmTmAgegtmf=Woxx7HYM0g05wQHw@mail.gmail.com>
<CAApHDvp9dA8KSEr7oE2dQn-tG6faH2Mtc3qnFgNghwNvBoq8Nw@mail.gmail.com>
<CAApHDvqHUJTtAha=wcLBAXkz6PNkMAFzsPzTHXig+Q3TUrQcug@mail.gmail.com>
<[email protected]>
On Fri, 23 Feb 2024 at 11:53, Andres Freund <[email protected]> wrote:
>
> On 2024-02-23 00:46:26 +1300, David Rowley wrote:
> > In absence of anyone else looking at this, I think it's ready to go.
> > If anyone is following along and wants to review or test it, please do
> > so soon.
>
> Makes sense!
I pushed the 0001 and 0002 patches today.
I switched over to working on doing what you did in 0002 for
generation.c and slab.c.
See the attached patch which runs the same test as in [1] (aset.c is
just there for comparisons between slab and generation)
The attached includes some additional tuning to generation.c:
1) Changed GenerationFree() to not free() the current block when it
becomes empty. The code now just marks it as empty and reuses it.
Saves free()/malloc() cycle. Also means we can get rid of a NULL check
in GenerationAlloc().
2) Removed code in GenerationAlloc() which I felt was trying too hard
to fill the keeper, free and current block. The changes I made here
do mean that once the keeper block becomes empty, it won't be used
again until the context is reset and gets a new allocation. I don't
see this as a big issue as the keeper block is small anyway.
generation.c is now ~30% faster for the 8-byte test.
David
[1] https://postgr.es/m/CAApHDvqss7-a9c51nj+f9xyAr15wjLB6teHsxPe-NwLCNqiJbg@mail.gmail.com
From 2f15c7ef04ebe5af539cfa2e3e05f2d491d4fc97 Mon Sep 17 00:00:00 2001
From: David Rowley <[email protected]>
Date: Wed, 28 Feb 2024 17:08:58 +1300
Subject: [PATCH v1] Optimize GenerationAlloc() and SlabAlloc()
In a similar effort to 413c18401, separate out the hot and cold paths in
GenerationAlloc() and SlabAlloc() to avoid having to setup the stack frame
for the hot path.
This additionally adjusts how we adjust the GenerationContext's
freeblock. freeblock, when set is now always empty and we only switch
to using it when the current allocation request finds the current block
does not have enough space and the freeblock is large enough to
accomodate the allocation.
This commit also adjusts GenerationFree() so that if we pfree a pointer
that's the current generation block, we now mark that block as empty and
keep it as the current block. Previously we free'd that block and set
the current block to NULL. Doing that meant we needed a special case in
GenerationAlloc to check if GenerationContext.block was NULL.
Author: David Rowley
---
src/backend/utils/mmgr/aset.c | 5 +-
src/backend/utils/mmgr/generation.c | 389 ++++++++++++++++------------
src/backend/utils/mmgr/slab.c | 230 +++++++++-------
3 files changed, 368 insertions(+), 256 deletions(-)
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 0cfee52274..e5eb7a1f39 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -943,8 +943,9 @@ AllocSetAllocFromNewBlock(MemoryContext context, Size size, int flags,
/*
* AllocSetAlloc
- * Returns pointer to allocated memory of given size or NULL if
- * request could not be completed; memory is added to the set.
+ * Returns a pointer to allocated memory of given size or raises an ERROR
+ * on allocation failure, or returns NULL when flags contains
+ * MCXT_ALLOC_NO_OOM.
*
* No request may exceed:
* MAXALIGN_DOWN(SIZE_MAX) - ALLOC_BLOCKHDRSZ - ALLOC_CHUNKHDRSZ
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index ae4a7c999e..8e9289c659 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -69,8 +69,8 @@ typedef struct GenerationContext
GenerationBlock *block; /* current (most recently allocated) block, or
* NULL if we've just freed the most recent
* block */
- GenerationBlock *freeblock; /* pointer to a block that's being recycled,
- * or NULL if there's no such block. */
+ GenerationBlock *freeblock; /* pointer to empty block that's being
+ * recycled, or NULL if there's no such block. */
dlist_head blocks; /* list of blocks */
} GenerationContext;
@@ -331,28 +331,23 @@ GenerationDelete(MemoryContext context)
}
/*
- * GenerationAlloc
- * Returns pointer to allocated memory of given size or NULL if
- * request could not be completed; memory is added to the set.
- *
- * No request may exceed:
- * MAXALIGN_DOWN(SIZE_MAX) - Generation_BLOCKHDRSZ - Generation_CHUNKHDRSZ
- * All callers use a much-lower limit.
+ * Helper for GenerationAlloc() that allocates an entire block for the chunk.
*
- * Note: when using valgrind, it doesn't matter how the returned allocation
- * is marked, as mcxt.c will set it to UNDEFINED. In some paths we will
- * return space that is marked NOACCESS - GenerationRealloc has to beware!
+ * GenerationAlloc()'s comment explains why this is separate.
*/
-void *
-GenerationAlloc(MemoryContext context, Size size, int flags)
+pg_noinline
+static void *
+GenerationAllocLarge(MemoryContext context, Size size, int flags)
{
GenerationContext *set = (GenerationContext *) context;
GenerationBlock *block;
MemoryChunk *chunk;
Size chunk_size;
Size required_size;
+ Size blksize;
- Assert(GenerationIsValid(set));
+ /* validate 'size' is within the limits for the given 'flags' */
+ MemoryContextCheckSize(context, size, flags);
#ifdef MEMORY_CONTEXT_CHECKING
/* ensure there's always space for the sentinel byte */
@@ -361,141 +356,66 @@ GenerationAlloc(MemoryContext context, Size size, int flags)
chunk_size = MAXALIGN(size);
#endif
required_size = chunk_size + Generation_CHUNKHDRSZ;
+ blksize = required_size + Generation_BLOCKHDRSZ;
- /* is it an over-sized chunk? if yes, allocate special block */
- if (chunk_size > set->allocChunkLimit)
- {
- Size blksize = required_size + Generation_BLOCKHDRSZ;
-
- /* only check size in paths where the limits could be hit */
- MemoryContextCheckSize((MemoryContext) set, size, flags);
-
- block = (GenerationBlock *) malloc(blksize);
- if (block == NULL)
- return MemoryContextAllocationFailure(context, size, flags);
+ block = (GenerationBlock *) malloc(blksize);
+ if (block == NULL)
+ return MemoryContextAllocationFailure(context, size, flags);
- context->mem_allocated += blksize;
+ context->mem_allocated += blksize;
- /* block with a single (used) chunk */
- block->context = set;
- block->blksize = blksize;
- block->nchunks = 1;
- block->nfree = 0;
+ /* block with a single (used) chunk */
+ block->context = set;
+ block->blksize = blksize;
+ block->nchunks = 1;
+ block->nfree = 0;
- /* the block is completely full */
- block->freeptr = block->endptr = ((char *) block) + blksize;
+ /* the block is completely full */
+ block->freeptr = block->endptr = ((char *) block) + blksize;
- chunk = (MemoryChunk *) (((char *) block) + Generation_BLOCKHDRSZ);
+ chunk = (MemoryChunk *) (((char *) block) + Generation_BLOCKHDRSZ);
- /* mark the MemoryChunk as externally managed */
- MemoryChunkSetHdrMaskExternal(chunk, MCTX_GENERATION_ID);
+ /* mark the MemoryChunk as externally managed */
+ MemoryChunkSetHdrMaskExternal(chunk, MCTX_GENERATION_ID);
#ifdef MEMORY_CONTEXT_CHECKING
- chunk->requested_size = size;
- /* set mark to catch clobber of "unused" space */
- Assert(size < chunk_size);
- set_sentinel(MemoryChunkGetPointer(chunk), size);
+ chunk->requested_size = size;
+ /* set mark to catch clobber of "unused" space */
+ Assert(size < chunk_size);
+ set_sentinel(MemoryChunkGetPointer(chunk), size);
#endif
#ifdef RANDOMIZE_ALLOCATED_MEMORY
- /* fill the allocated space with junk */
- randomize_mem((char *) MemoryChunkGetPointer(chunk), size);
+ /* fill the allocated space with junk */
+ randomize_mem((char *) MemoryChunkGetPointer(chunk), size);
#endif
- /* add the block to the list of allocated blocks */
- dlist_push_head(&set->blocks, &block->node);
-
- /* Ensure any padding bytes are marked NOACCESS. */
- VALGRIND_MAKE_MEM_NOACCESS((char *) MemoryChunkGetPointer(chunk) + size,
- chunk_size - size);
-
- /* Disallow access to the chunk header. */
- VALGRIND_MAKE_MEM_NOACCESS(chunk, Generation_CHUNKHDRSZ);
-
- return MemoryChunkGetPointer(chunk);
- }
-
- /*
- * Not an oversized chunk. We try to first make use of the current block,
- * but if there's not enough space in it, instead of allocating a new
- * block, we look to see if the freeblock is empty and has enough space.
- * If not, we'll also try the same using the keeper block. The keeper
- * block may have become empty and we have no other way to reuse it again
- * if we don't try to use it explicitly here.
- *
- * We don't want to start filling the freeblock before the current block
- * is full, otherwise we may cause fragmentation in FIFO type workloads.
- * We only switch to using the freeblock or keeper block if those blocks
- * are completely empty. If we didn't do that we could end up fragmenting
- * consecutive allocations over multiple blocks which would be a problem
- * that would compound over time.
- */
- block = set->block;
-
- if (block == NULL ||
- GenerationBlockFreeBytes(block) < required_size)
- {
- Size blksize;
- GenerationBlock *freeblock = set->freeblock;
-
- if (freeblock != NULL &&
- GenerationBlockIsEmpty(freeblock) &&
- GenerationBlockFreeBytes(freeblock) >= required_size)
- {
- block = freeblock;
-
- /*
- * Zero out the freeblock as we'll set this to the current block
- * below
- */
- set->freeblock = NULL;
- }
- else if (GenerationBlockIsEmpty(KeeperBlock(set)) &&
- GenerationBlockFreeBytes(KeeperBlock(set)) >= required_size)
- {
- block = KeeperBlock(set);
- }
- else
- {
- /*
- * 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;
-
- /* we'll need a block hdr too, so add that to the required size */
- required_size += Generation_BLOCKHDRSZ;
-
- /* round the size up to the next power of 2 */
- if (blksize < required_size)
- blksize = pg_nextpower2_size_t(required_size);
-
- block = (GenerationBlock *) malloc(blksize);
-
- if (block == NULL)
- return MemoryContextAllocationFailure(context, size, flags);
-
- context->mem_allocated += blksize;
+ /* add the block to the list of allocated blocks */
+ dlist_push_head(&set->blocks, &block->node);
- /* initialize the new block */
- GenerationBlockInit(set, block, blksize);
+ /* Ensure any padding bytes are marked NOACCESS. */
+ VALGRIND_MAKE_MEM_NOACCESS((char *) MemoryChunkGetPointer(chunk) + size,
+ chunk_size - size);
- /* add it to the doubly-linked list of blocks */
- dlist_push_head(&set->blocks, &block->node);
+ /* Disallow access to the chunk header. */
+ VALGRIND_MAKE_MEM_NOACCESS(chunk, Generation_CHUNKHDRSZ);
- /* Zero out the freeblock in case it's become full */
- set->freeblock = NULL;
- }
+ return MemoryChunkGetPointer(chunk);
+}
- /* and also use it as the current allocation block */
- set->block = block;
- }
+/*
+ * Small helper for allocating a new chunk from a chunk, to avoid duplicating
+ * the code between GenerationAlloc() and GenerationAllocFromNewBlock().
+ */
+static inline void *
+GenerationAllocChunkFromBlock(MemoryContext context, GenerationBlock *block,
+ Size size, Size chunk_size)
+{
+ MemoryChunk *chunk = (MemoryChunk *) (block->freeptr);
- /* we're supposed to have a block with enough free space now */
+ /* validate we've been given a block with enough free space */
Assert(block != NULL);
- Assert((block->endptr - block->freeptr) >= Generation_CHUNKHDRSZ + chunk_size);
+ Assert((block->endptr - block->freeptr) >=
+ Generation_CHUNKHDRSZ + chunk_size);
chunk = (MemoryChunk *) block->freeptr;
@@ -529,6 +449,155 @@ GenerationAlloc(MemoryContext context, Size size, int flags)
return MemoryChunkGetPointer(chunk);
}
+/*
+ * Helper for GenerationAlloc() that allocates a new block and returns a chunk
+ * allocated from it.
+ *
+ * GenerationAlloc()'s comment explains why this is separate.
+ */
+pg_noinline
+static void *
+GenerationAllocFromNewBlock(MemoryContext context, Size size, int flags,
+ Size chunk_size)
+{
+ GenerationContext *set = (GenerationContext *) context;
+ GenerationBlock *block;
+ Size blksize;
+ Size required_size;
+
+ /*
+ * 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;
+
+ /* we'll need space for the chunk, chunk hdr and block hdr */
+ required_size = chunk_size + Generation_CHUNKHDRSZ + Generation_BLOCKHDRSZ;
+
+ /* round the size up to the next power of 2 */
+ if (blksize < required_size)
+ blksize = pg_nextpower2_size_t(required_size);
+
+ block = (GenerationBlock *) malloc(blksize);
+
+ if (block == NULL)
+ return MemoryContextAllocationFailure(context, size, flags);
+
+ context->mem_allocated += blksize;
+
+ /* initialize the new block */
+ GenerationBlockInit(set, block, blksize);
+
+ /* add it to the doubly-linked list of blocks */
+ dlist_push_head(&set->blocks, &block->node);
+
+ /* make this the current block */
+ set->block = block;
+
+ return GenerationAllocChunkFromBlock(context, block, size, chunk_size);
+}
+
+/*
+ * GenerationAlloc
+ * Returns a pointer to allocated memory of given size or raises an ERROR
+ * on allocation failure, or returns NULL when flags contains
+ * MCXT_ALLOC_NO_OOM.
+ *
+ * No request may exceed:
+ * MAXALIGN_DOWN(SIZE_MAX) - Generation_BLOCKHDRSZ - Generation_CHUNKHDRSZ
+ * All callers use a much-lower limit.
+ *
+ * Note: when using valgrind, it doesn't matter how the returned allocation
+ * is marked, as mcxt.c will set it to UNDEFINED. In some paths we will
+ * return space that is marked NOACCESS - GenerationRealloc has to beware!
+ *
+ * This function should only contain the most common code paths. Everything
+ * else should be in pg_noinline helper functions, thus avoiding the overheads
+ * creating a stack frame for the common cases. Allocating memory is often a
+ * bottleneck in many workloads, so avoiding stack frame setup is worthwhile.
+ * Helper functions should always directly return the newly allocated memory
+ * so that we can just return that address directly as a tail call.
+ */
+void *
+GenerationAlloc(MemoryContext context, Size size, int flags)
+{
+ GenerationContext *set = (GenerationContext *) context;
+ GenerationBlock *block;
+ Size chunk_size;
+ Size required_size;
+
+ Assert(GenerationIsValid(set));
+
+#ifdef MEMORY_CONTEXT_CHECKING
+ /* ensure there's always space for the sentinel byte */
+ chunk_size = MAXALIGN(size + 1);
+#else
+ chunk_size = MAXALIGN(size);
+#endif
+
+ /*
+ * If requested size exceeds maximum for chunks we hand the the request
+ * off to GenerationAllocLarge().
+ */
+ if (chunk_size > set->allocChunkLimit)
+ return GenerationAllocLarge(context, size, flags);
+
+ required_size = chunk_size + Generation_CHUNKHDRSZ;
+
+ /*
+ * Not an oversized chunk. We try to first make use of the current block,
+ * but if there's not enough space in it, instead of allocating a new
+ * block, we look to see if the empty freeblock has enough space. We
+ * don't try reusing the keeper block. If it's become empty we'll reuse
+ * that again only if the context is reset.
+ *
+ * We only try reusing the freeblock if we've no space for this allocation
+ * on the current block. When a freeblock exists, we'll switch to it once
+ * the first time we can't fit an allocation in the current block. We
+ * avoid ping-ponging between the two as we need to be careful not to
+ * fragment differently sized consecutive allocations between several
+ * blocks. Going between the two could cause fragmentation for FIFO
+ * workloads, which generation is meant to be good at.
+ */
+ block = set->block;
+
+ if (unlikely(GenerationBlockFreeBytes(block) < required_size))
+ {
+ GenerationBlock *freeblock = set->freeblock;
+
+ /* freeblock, if set, must be empty */
+ Assert(freeblock == NULL || GenerationBlockIsEmpty(freeblock));
+
+ /* check if we have a freeblock and if it's big enough */
+ if (freeblock != NULL &&
+ GenerationBlockFreeBytes(freeblock) >= required_size)
+ {
+ /* make the freeblock the current block */
+ set->freeblock = NULL;
+ set->block = freeblock;
+
+ return GenerationAllocChunkFromBlock(context,
+ freeblock,
+ size,
+ chunk_size);
+ }
+ else
+ {
+ /*
+ * No freeblock, or it's not big enough for this allocation. Make
+ * a new block.
+ */
+ return GenerationAllocFromNewBlock(context, size, flags, chunk_size);
+ }
+ }
+
+ /* The current block has space, so just allocate chunk there. */
+ return GenerationAllocChunkFromBlock(context, block, size, chunk_size);
+}
+
/*
* GenerationBlockInit
* Initializes 'block' assuming 'blksize'. Does not update the context's
@@ -621,8 +690,8 @@ GenerationBlockFree(GenerationContext *set, GenerationBlock *block)
/*
* GenerationFree
- * Update number of chunks in the block, and if all chunks in the block
- * are now free then discard the block.
+ * Update number of chunks in the block, and consider freeing the block
+ * if it's become empty.
*/
void
GenerationFree(void *pointer)
@@ -694,43 +763,37 @@ GenerationFree(void *pointer)
Assert(block->nfree <= block->nchunks);
/* If there are still allocated chunks in the block, we're done. */
- if (block->nfree < block->nchunks)
+ if (likely(block->nfree < block->nchunks))
return;
set = block->context;
- /* Don't try to free the keeper block, just mark it empty */
- if (IsKeeperBlock(set, block))
- {
- GenerationBlockMarkEmpty(block);
- return;
- }
-
- /*
- * If there is no freeblock set or if this is the freeblock then instead
- * of freeing this memory, we keep it around so that new allocations have
- * the option of recycling it.
+ /*-----------------------
+ * The block this allocation was on has now become completely empty of
+ * chunks. In the general case, we can now return the memory for this
+ * block back to malloc. However, there are cases where we don't want to
+ * do that:
+ *
+ * 1) If it's the keeper block. This block is malloc'd with the context
+ * and can't be free'd without freeing the context itself.
+ * 2) If it's the current block. We could free this, but doing so would
+ * leave us nothing to set the current block to, so we just mark the
+ * block as empty so new allocations can reuse it again.
+ * 3) If we have no "freeblock" set, then to avoid new allocations from
+ * having to malloc a new block again, we save a single block to
+ * avoid that. This is especially useful for FIFO workloads as it
+ * avoids continual free/malloc cycles.
*/
- if (set->freeblock == NULL || set->freeblock == block)
+ if (IsKeeperBlock(set, block) || set->block == block)
+ GenerationBlockMarkEmpty(block); /* case 1 and 2 */
+ else if (set->freeblock == NULL)
{
- /* XXX should we only recycle maxBlockSize sized blocks? */
- set->freeblock = block;
+ /* case 3 */
GenerationBlockMarkEmpty(block);
- return;
+ set->freeblock = block;
}
-
- /* Also make sure the block is not marked as the current block. */
- if (set->block == block)
- set->block = NULL;
-
- /*
- * 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);
-
- set->header.mem_allocated -= block->blksize;
- free(block);
+ else
+ GenerationBlockFree(set, block); /* Otherwise, free it */
}
/*
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index bc91446cb3..c5f985d5ea 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -490,10 +490,139 @@ SlabDelete(MemoryContext context)
free(context);
}
+/*
+ * Small helper for allocating a new chunk from a chunk, to avoid duplicating
+ * the code between SlabAlloc() and SlabAllocFromNewBlock().
+ */
+static inline void *
+SlabAllocSetupNewChunk(MemoryContext context, SlabBlock *block,
+ MemoryChunk *chunk, Size size)
+{
+ SlabContext *slab = (SlabContext *) context;
+
+ /*
+ * Check that the chunk pointer is actually somewhere on the block and is
+ * aligned as expected.
+ */
+ Assert(chunk >= SlabBlockGetChunk(slab, block, 0));
+ Assert(chunk <= SlabBlockGetChunk(slab, block, slab->chunksPerBlock - 1));
+ Assert(SlabChunkMod(slab, block, chunk) == 0);
+
+ /* Prepare to initialize the chunk header. */
+ VALGRIND_MAKE_MEM_UNDEFINED(chunk, Slab_CHUNKHDRSZ);
+
+ MemoryChunkSetHdrMask(chunk, block, MAXALIGN(slab->chunkSize), MCTX_SLAB_ID);
+
+#ifdef MEMORY_CONTEXT_CHECKING
+ /* slab mark to catch clobber of "unused" space */
+ Assert(slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ));
+ set_sentinel(MemoryChunkGetPointer(chunk), size);
+ VALGRIND_MAKE_MEM_NOACCESS(((char *) chunk) + Slab_CHUNKHDRSZ +
+ slab->chunkSize,
+ slab->fullChunkSize -
+ (slab->chunkSize + Slab_CHUNKHDRSZ));
+#endif
+
+#ifdef RANDOMIZE_ALLOCATED_MEMORY
+ /* fill the allocated space with junk */
+ randomize_mem((char *) MemoryChunkGetPointer(chunk), size);
+#endif
+
+ /* Disallow access to the chunk header. */
+ VALGRIND_MAKE_MEM_NOACCESS(chunk, Slab_CHUNKHDRSZ);
+
+ return MemoryChunkGetPointer(chunk);
+}
+
+pg_noinline
+static void *
+SlabAllocFromNewBlock(MemoryContext context, Size size, int flags)
+{
+ SlabContext *slab = (SlabContext *) context;
+ SlabBlock *block;
+ MemoryChunk *chunk;
+ dlist_head *blocklist;
+ int blocklist_idx;
+
+ /* to save allocating a new one, first check the empty blocks list */
+ if (dclist_count(&slab->emptyblocks) > 0)
+ {
+ dlist_node *node = dclist_pop_head_node(&slab->emptyblocks);
+
+ block = dlist_container(SlabBlock, node, node);
+
+ /*
+ * SlabFree() should have left this block in a valid state with all
+ * chunks free. Ensure that's the case.
+ */
+ Assert(block->nfree == slab->chunksPerBlock);
+
+ /* fetch the next chunk from this block */
+ chunk = SlabGetNextFreeChunk(slab, block);
+ }
+ else
+ {
+ block = (SlabBlock *) malloc(slab->blockSize);
+
+ if (unlikely(block == NULL))
+ return MemoryContextAllocationFailure(context, size, flags);
+
+ block->slab = slab;
+ context->mem_allocated += slab->blockSize;
+
+ /* use the first chunk in the new block */
+ chunk = SlabBlockGetChunk(slab, block, 0);
+
+ block->nfree = slab->chunksPerBlock - 1;
+ block->unused = SlabBlockGetChunk(slab, block, 1);
+ block->freehead = NULL;
+ block->nunused = slab->chunksPerBlock - 1;
+ }
+
+ /* find the blocklist element for storing blocks with 1 used chunk */
+ blocklist_idx = SlabBlocklistIndex(slab, block->nfree);
+ blocklist = &slab->blocklist[blocklist_idx];
+
+ /* this better be empty. We just added a block thinking it was */
+ Assert(dlist_is_empty(blocklist));
+
+ dlist_push_head(blocklist, &block->node);
+
+ slab->curBlocklistIndex = blocklist_idx;
+
+ return SlabAllocSetupNewChunk(context, block, chunk, size);
+}
+
+/*
+ * SlabAllocInvalidSize
+ * Handle raising an ERROR for an invalid size request. We don't do this
+ * in slab alloc as calling the elog functions would force the compiler
+ * to setup the stack frame in SlabAlloc. For performance reasons, we
+ * want to avoid that.
+ */
+pg_noinline
+static void
+pg_attribute_noreturn()
+SlabAllocInvalidSize(MemoryContext context, Size size)
+{
+ SlabContext *slab = (SlabContext *) context;
+
+ elog(ERROR, "unexpected alloc chunk size %zu (expected %u)", size,
+ slab->chunkSize);
+}
+
/*
* SlabAlloc
- * Returns a pointer to allocated memory of given size or NULL if
- * request could not be completed; memory is added to the slab.
+ * Returns a pointer to allocated memory of given size or raises an ERROR
+ * on allocation failure, or returns NULL when flags contains
+ * MCXT_ALLOC_NO_OOM.
+ *
+ * This function should only contain the most common code paths. Everything
+ * else should be in pg_noinline helper functions, thus avoiding the overheads
+ * creating a stack frame for the common cases. Allocating memory is often a
+ * bottleneck in many workloads, so avoiding stack frame setup is worthwhile.
+ * Helper functions should always directly return the newly allocated memory
+ * so that we can just return that address directly as a tail call.
*/
void *
SlabAlloc(MemoryContext context, Size size, int flags)
@@ -513,66 +642,16 @@ SlabAlloc(MemoryContext context, Size size, int flags)
* MemoryContextCheckSize check.
*/
if (unlikely(size != slab->chunkSize))
- elog(ERROR, "unexpected alloc chunk size %zu (expected %u)",
- size, slab->chunkSize);
+ SlabAllocInvalidSize(context, size);
- /*
- * Handle the case when there are no partially filled blocks available.
- * SlabFree() will have updated the curBlocklistIndex setting it to zero
- * to indicate that it has freed the final block. Also later in
- * SlabAlloc() we will set the curBlocklistIndex to zero if we end up
- * filling the final block.
- */
if (unlikely(slab->curBlocklistIndex == 0))
{
- dlist_head *blocklist;
- int blocklist_idx;
-
- /* to save allocating a new one, first check the empty blocks list */
- if (dclist_count(&slab->emptyblocks) > 0)
- {
- dlist_node *node = dclist_pop_head_node(&slab->emptyblocks);
-
- block = dlist_container(SlabBlock, node, node);
-
- /*
- * SlabFree() should have left this block in a valid state with
- * all chunks free. Ensure that's the case.
- */
- Assert(block->nfree == slab->chunksPerBlock);
-
- /* fetch the next chunk from this block */
- chunk = SlabGetNextFreeChunk(slab, block);
- }
- else
- {
- block = (SlabBlock *) malloc(slab->blockSize);
-
- if (unlikely(block == NULL))
- return MemoryContextAllocationFailure(context, size, flags);
-
- block->slab = slab;
- context->mem_allocated += slab->blockSize;
-
- /* use the first chunk in the new block */
- chunk = SlabBlockGetChunk(slab, block, 0);
-
- block->nfree = slab->chunksPerBlock - 1;
- block->unused = SlabBlockGetChunk(slab, block, 1);
- block->freehead = NULL;
- block->nunused = slab->chunksPerBlock - 1;
- }
-
- /* find the blocklist element for storing blocks with 1 used chunk */
- blocklist_idx = SlabBlocklistIndex(slab, block->nfree);
- blocklist = &slab->blocklist[blocklist_idx];
-
- /* this better be empty. We just added a block thinking it was */
- Assert(dlist_is_empty(blocklist));
-
- dlist_push_head(blocklist, &block->node);
-
- slab->curBlocklistIndex = blocklist_idx;
+ /*
+ * Handle the case when there are no partially filled blocks
+ * available. This happens either when the last allocation took the
+ * last chunk in the block, or when SlabFree() free'd the final block.
+ */
+ return SlabAllocFromNewBlock(context, size, flags);
}
else
{
@@ -609,38 +688,7 @@ SlabAlloc(MemoryContext context, Size size, int flags)
}
}
- /*
- * Check that the chunk pointer is actually somewhere on the block and is
- * aligned as expected.
- */
- Assert(chunk >= SlabBlockGetChunk(slab, block, 0));
- Assert(chunk <= SlabBlockGetChunk(slab, block, slab->chunksPerBlock - 1));
- Assert(SlabChunkMod(slab, block, chunk) == 0);
-
- /* Prepare to initialize the chunk header. */
- VALGRIND_MAKE_MEM_UNDEFINED(chunk, Slab_CHUNKHDRSZ);
-
- MemoryChunkSetHdrMask(chunk, block, MAXALIGN(slab->chunkSize),
- MCTX_SLAB_ID);
-#ifdef MEMORY_CONTEXT_CHECKING
- /* slab mark to catch clobber of "unused" space */
- Assert(slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ));
- set_sentinel(MemoryChunkGetPointer(chunk), size);
- VALGRIND_MAKE_MEM_NOACCESS(((char *) chunk) +
- Slab_CHUNKHDRSZ + slab->chunkSize,
- slab->fullChunkSize -
- (slab->chunkSize + Slab_CHUNKHDRSZ));
-#endif
-
-#ifdef RANDOMIZE_ALLOCATED_MEMORY
- /* fill the allocated space with junk */
- randomize_mem((char *) MemoryChunkGetPointer(chunk), size);
-#endif
-
- /* Disallow access to the chunk header. */
- VALGRIND_MAKE_MEM_NOACCESS(chunk, Slab_CHUNKHDRSZ);
-
- return MemoryChunkGetPointer(chunk);
+ return SlabAllocSetupNewChunk(context, block, chunk, size);
}
/*
--
2.40.1
Attachments:
[text/plain] v1-0001-Optimize-GenerationAlloc-and-SlabAlloc.patch (26.5K, ../CAApHDvpHVSJqqb4B4OZLixr=CotKq-eKkbwZqvZVo_biYvUvQA@mail.gmail.com/2-v1-0001-Optimize-GenerationAlloc-and-SlabAlloc.patch)
download | inline diff:
From 2f15c7ef04ebe5af539cfa2e3e05f2d491d4fc97 Mon Sep 17 00:00:00 2001
From: David Rowley <[email protected]>
Date: Wed, 28 Feb 2024 17:08:58 +1300
Subject: [PATCH v1] Optimize GenerationAlloc() and SlabAlloc()
In a similar effort to 413c18401, separate out the hot and cold paths in
GenerationAlloc() and SlabAlloc() to avoid having to setup the stack frame
for the hot path.
This additionally adjusts how we adjust the GenerationContext's
freeblock. freeblock, when set is now always empty and we only switch
to using it when the current allocation request finds the current block
does not have enough space and the freeblock is large enough to
accomodate the allocation.
This commit also adjusts GenerationFree() so that if we pfree a pointer
that's the current generation block, we now mark that block as empty and
keep it as the current block. Previously we free'd that block and set
the current block to NULL. Doing that meant we needed a special case in
GenerationAlloc to check if GenerationContext.block was NULL.
Author: David Rowley
---
src/backend/utils/mmgr/aset.c | 5 +-
src/backend/utils/mmgr/generation.c | 389 ++++++++++++++++------------
src/backend/utils/mmgr/slab.c | 230 +++++++++-------
3 files changed, 368 insertions(+), 256 deletions(-)
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 0cfee52274..e5eb7a1f39 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -943,8 +943,9 @@ AllocSetAllocFromNewBlock(MemoryContext context, Size size, int flags,
/*
* AllocSetAlloc
- * Returns pointer to allocated memory of given size or NULL if
- * request could not be completed; memory is added to the set.
+ * Returns a pointer to allocated memory of given size or raises an ERROR
+ * on allocation failure, or returns NULL when flags contains
+ * MCXT_ALLOC_NO_OOM.
*
* No request may exceed:
* MAXALIGN_DOWN(SIZE_MAX) - ALLOC_BLOCKHDRSZ - ALLOC_CHUNKHDRSZ
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index ae4a7c999e..8e9289c659 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -69,8 +69,8 @@ typedef struct GenerationContext
GenerationBlock *block; /* current (most recently allocated) block, or
* NULL if we've just freed the most recent
* block */
- GenerationBlock *freeblock; /* pointer to a block that's being recycled,
- * or NULL if there's no such block. */
+ GenerationBlock *freeblock; /* pointer to empty block that's being
+ * recycled, or NULL if there's no such block. */
dlist_head blocks; /* list of blocks */
} GenerationContext;
@@ -331,28 +331,23 @@ GenerationDelete(MemoryContext context)
}
/*
- * GenerationAlloc
- * Returns pointer to allocated memory of given size or NULL if
- * request could not be completed; memory is added to the set.
- *
- * No request may exceed:
- * MAXALIGN_DOWN(SIZE_MAX) - Generation_BLOCKHDRSZ - Generation_CHUNKHDRSZ
- * All callers use a much-lower limit.
+ * Helper for GenerationAlloc() that allocates an entire block for the chunk.
*
- * Note: when using valgrind, it doesn't matter how the returned allocation
- * is marked, as mcxt.c will set it to UNDEFINED. In some paths we will
- * return space that is marked NOACCESS - GenerationRealloc has to beware!
+ * GenerationAlloc()'s comment explains why this is separate.
*/
-void *
-GenerationAlloc(MemoryContext context, Size size, int flags)
+pg_noinline
+static void *
+GenerationAllocLarge(MemoryContext context, Size size, int flags)
{
GenerationContext *set = (GenerationContext *) context;
GenerationBlock *block;
MemoryChunk *chunk;
Size chunk_size;
Size required_size;
+ Size blksize;
- Assert(GenerationIsValid(set));
+ /* validate 'size' is within the limits for the given 'flags' */
+ MemoryContextCheckSize(context, size, flags);
#ifdef MEMORY_CONTEXT_CHECKING
/* ensure there's always space for the sentinel byte */
@@ -361,141 +356,66 @@ GenerationAlloc(MemoryContext context, Size size, int flags)
chunk_size = MAXALIGN(size);
#endif
required_size = chunk_size + Generation_CHUNKHDRSZ;
+ blksize = required_size + Generation_BLOCKHDRSZ;
- /* is it an over-sized chunk? if yes, allocate special block */
- if (chunk_size > set->allocChunkLimit)
- {
- Size blksize = required_size + Generation_BLOCKHDRSZ;
-
- /* only check size in paths where the limits could be hit */
- MemoryContextCheckSize((MemoryContext) set, size, flags);
-
- block = (GenerationBlock *) malloc(blksize);
- if (block == NULL)
- return MemoryContextAllocationFailure(context, size, flags);
+ block = (GenerationBlock *) malloc(blksize);
+ if (block == NULL)
+ return MemoryContextAllocationFailure(context, size, flags);
- context->mem_allocated += blksize;
+ context->mem_allocated += blksize;
- /* block with a single (used) chunk */
- block->context = set;
- block->blksize = blksize;
- block->nchunks = 1;
- block->nfree = 0;
+ /* block with a single (used) chunk */
+ block->context = set;
+ block->blksize = blksize;
+ block->nchunks = 1;
+ block->nfree = 0;
- /* the block is completely full */
- block->freeptr = block->endptr = ((char *) block) + blksize;
+ /* the block is completely full */
+ block->freeptr = block->endptr = ((char *) block) + blksize;
- chunk = (MemoryChunk *) (((char *) block) + Generation_BLOCKHDRSZ);
+ chunk = (MemoryChunk *) (((char *) block) + Generation_BLOCKHDRSZ);
- /* mark the MemoryChunk as externally managed */
- MemoryChunkSetHdrMaskExternal(chunk, MCTX_GENERATION_ID);
+ /* mark the MemoryChunk as externally managed */
+ MemoryChunkSetHdrMaskExternal(chunk, MCTX_GENERATION_ID);
#ifdef MEMORY_CONTEXT_CHECKING
- chunk->requested_size = size;
- /* set mark to catch clobber of "unused" space */
- Assert(size < chunk_size);
- set_sentinel(MemoryChunkGetPointer(chunk), size);
+ chunk->requested_size = size;
+ /* set mark to catch clobber of "unused" space */
+ Assert(size < chunk_size);
+ set_sentinel(MemoryChunkGetPointer(chunk), size);
#endif
#ifdef RANDOMIZE_ALLOCATED_MEMORY
- /* fill the allocated space with junk */
- randomize_mem((char *) MemoryChunkGetPointer(chunk), size);
+ /* fill the allocated space with junk */
+ randomize_mem((char *) MemoryChunkGetPointer(chunk), size);
#endif
- /* add the block to the list of allocated blocks */
- dlist_push_head(&set->blocks, &block->node);
-
- /* Ensure any padding bytes are marked NOACCESS. */
- VALGRIND_MAKE_MEM_NOACCESS((char *) MemoryChunkGetPointer(chunk) + size,
- chunk_size - size);
-
- /* Disallow access to the chunk header. */
- VALGRIND_MAKE_MEM_NOACCESS(chunk, Generation_CHUNKHDRSZ);
-
- return MemoryChunkGetPointer(chunk);
- }
-
- /*
- * Not an oversized chunk. We try to first make use of the current block,
- * but if there's not enough space in it, instead of allocating a new
- * block, we look to see if the freeblock is empty and has enough space.
- * If not, we'll also try the same using the keeper block. The keeper
- * block may have become empty and we have no other way to reuse it again
- * if we don't try to use it explicitly here.
- *
- * We don't want to start filling the freeblock before the current block
- * is full, otherwise we may cause fragmentation in FIFO type workloads.
- * We only switch to using the freeblock or keeper block if those blocks
- * are completely empty. If we didn't do that we could end up fragmenting
- * consecutive allocations over multiple blocks which would be a problem
- * that would compound over time.
- */
- block = set->block;
-
- if (block == NULL ||
- GenerationBlockFreeBytes(block) < required_size)
- {
- Size blksize;
- GenerationBlock *freeblock = set->freeblock;
-
- if (freeblock != NULL &&
- GenerationBlockIsEmpty(freeblock) &&
- GenerationBlockFreeBytes(freeblock) >= required_size)
- {
- block = freeblock;
-
- /*
- * Zero out the freeblock as we'll set this to the current block
- * below
- */
- set->freeblock = NULL;
- }
- else if (GenerationBlockIsEmpty(KeeperBlock(set)) &&
- GenerationBlockFreeBytes(KeeperBlock(set)) >= required_size)
- {
- block = KeeperBlock(set);
- }
- else
- {
- /*
- * 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;
-
- /* we'll need a block hdr too, so add that to the required size */
- required_size += Generation_BLOCKHDRSZ;
-
- /* round the size up to the next power of 2 */
- if (blksize < required_size)
- blksize = pg_nextpower2_size_t(required_size);
-
- block = (GenerationBlock *) malloc(blksize);
-
- if (block == NULL)
- return MemoryContextAllocationFailure(context, size, flags);
-
- context->mem_allocated += blksize;
+ /* add the block to the list of allocated blocks */
+ dlist_push_head(&set->blocks, &block->node);
- /* initialize the new block */
- GenerationBlockInit(set, block, blksize);
+ /* Ensure any padding bytes are marked NOACCESS. */
+ VALGRIND_MAKE_MEM_NOACCESS((char *) MemoryChunkGetPointer(chunk) + size,
+ chunk_size - size);
- /* add it to the doubly-linked list of blocks */
- dlist_push_head(&set->blocks, &block->node);
+ /* Disallow access to the chunk header. */
+ VALGRIND_MAKE_MEM_NOACCESS(chunk, Generation_CHUNKHDRSZ);
- /* Zero out the freeblock in case it's become full */
- set->freeblock = NULL;
- }
+ return MemoryChunkGetPointer(chunk);
+}
- /* and also use it as the current allocation block */
- set->block = block;
- }
+/*
+ * Small helper for allocating a new chunk from a chunk, to avoid duplicating
+ * the code between GenerationAlloc() and GenerationAllocFromNewBlock().
+ */
+static inline void *
+GenerationAllocChunkFromBlock(MemoryContext context, GenerationBlock *block,
+ Size size, Size chunk_size)
+{
+ MemoryChunk *chunk = (MemoryChunk *) (block->freeptr);
- /* we're supposed to have a block with enough free space now */
+ /* validate we've been given a block with enough free space */
Assert(block != NULL);
- Assert((block->endptr - block->freeptr) >= Generation_CHUNKHDRSZ + chunk_size);
+ Assert((block->endptr - block->freeptr) >=
+ Generation_CHUNKHDRSZ + chunk_size);
chunk = (MemoryChunk *) block->freeptr;
@@ -529,6 +449,155 @@ GenerationAlloc(MemoryContext context, Size size, int flags)
return MemoryChunkGetPointer(chunk);
}
+/*
+ * Helper for GenerationAlloc() that allocates a new block and returns a chunk
+ * allocated from it.
+ *
+ * GenerationAlloc()'s comment explains why this is separate.
+ */
+pg_noinline
+static void *
+GenerationAllocFromNewBlock(MemoryContext context, Size size, int flags,
+ Size chunk_size)
+{
+ GenerationContext *set = (GenerationContext *) context;
+ GenerationBlock *block;
+ Size blksize;
+ Size required_size;
+
+ /*
+ * 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;
+
+ /* we'll need space for the chunk, chunk hdr and block hdr */
+ required_size = chunk_size + Generation_CHUNKHDRSZ + Generation_BLOCKHDRSZ;
+
+ /* round the size up to the next power of 2 */
+ if (blksize < required_size)
+ blksize = pg_nextpower2_size_t(required_size);
+
+ block = (GenerationBlock *) malloc(blksize);
+
+ if (block == NULL)
+ return MemoryContextAllocationFailure(context, size, flags);
+
+ context->mem_allocated += blksize;
+
+ /* initialize the new block */
+ GenerationBlockInit(set, block, blksize);
+
+ /* add it to the doubly-linked list of blocks */
+ dlist_push_head(&set->blocks, &block->node);
+
+ /* make this the current block */
+ set->block = block;
+
+ return GenerationAllocChunkFromBlock(context, block, size, chunk_size);
+}
+
+/*
+ * GenerationAlloc
+ * Returns a pointer to allocated memory of given size or raises an ERROR
+ * on allocation failure, or returns NULL when flags contains
+ * MCXT_ALLOC_NO_OOM.
+ *
+ * No request may exceed:
+ * MAXALIGN_DOWN(SIZE_MAX) - Generation_BLOCKHDRSZ - Generation_CHUNKHDRSZ
+ * All callers use a much-lower limit.
+ *
+ * Note: when using valgrind, it doesn't matter how the returned allocation
+ * is marked, as mcxt.c will set it to UNDEFINED. In some paths we will
+ * return space that is marked NOACCESS - GenerationRealloc has to beware!
+ *
+ * This function should only contain the most common code paths. Everything
+ * else should be in pg_noinline helper functions, thus avoiding the overheads
+ * creating a stack frame for the common cases. Allocating memory is often a
+ * bottleneck in many workloads, so avoiding stack frame setup is worthwhile.
+ * Helper functions should always directly return the newly allocated memory
+ * so that we can just return that address directly as a tail call.
+ */
+void *
+GenerationAlloc(MemoryContext context, Size size, int flags)
+{
+ GenerationContext *set = (GenerationContext *) context;
+ GenerationBlock *block;
+ Size chunk_size;
+ Size required_size;
+
+ Assert(GenerationIsValid(set));
+
+#ifdef MEMORY_CONTEXT_CHECKING
+ /* ensure there's always space for the sentinel byte */
+ chunk_size = MAXALIGN(size + 1);
+#else
+ chunk_size = MAXALIGN(size);
+#endif
+
+ /*
+ * If requested size exceeds maximum for chunks we hand the the request
+ * off to GenerationAllocLarge().
+ */
+ if (chunk_size > set->allocChunkLimit)
+ return GenerationAllocLarge(context, size, flags);
+
+ required_size = chunk_size + Generation_CHUNKHDRSZ;
+
+ /*
+ * Not an oversized chunk. We try to first make use of the current block,
+ * but if there's not enough space in it, instead of allocating a new
+ * block, we look to see if the empty freeblock has enough space. We
+ * don't try reusing the keeper block. If it's become empty we'll reuse
+ * that again only if the context is reset.
+ *
+ * We only try reusing the freeblock if we've no space for this allocation
+ * on the current block. When a freeblock exists, we'll switch to it once
+ * the first time we can't fit an allocation in the current block. We
+ * avoid ping-ponging between the two as we need to be careful not to
+ * fragment differently sized consecutive allocations between several
+ * blocks. Going between the two could cause fragmentation for FIFO
+ * workloads, which generation is meant to be good at.
+ */
+ block = set->block;
+
+ if (unlikely(GenerationBlockFreeBytes(block) < required_size))
+ {
+ GenerationBlock *freeblock = set->freeblock;
+
+ /* freeblock, if set, must be empty */
+ Assert(freeblock == NULL || GenerationBlockIsEmpty(freeblock));
+
+ /* check if we have a freeblock and if it's big enough */
+ if (freeblock != NULL &&
+ GenerationBlockFreeBytes(freeblock) >= required_size)
+ {
+ /* make the freeblock the current block */
+ set->freeblock = NULL;
+ set->block = freeblock;
+
+ return GenerationAllocChunkFromBlock(context,
+ freeblock,
+ size,
+ chunk_size);
+ }
+ else
+ {
+ /*
+ * No freeblock, or it's not big enough for this allocation. Make
+ * a new block.
+ */
+ return GenerationAllocFromNewBlock(context, size, flags, chunk_size);
+ }
+ }
+
+ /* The current block has space, so just allocate chunk there. */
+ return GenerationAllocChunkFromBlock(context, block, size, chunk_size);
+}
+
/*
* GenerationBlockInit
* Initializes 'block' assuming 'blksize'. Does not update the context's
@@ -621,8 +690,8 @@ GenerationBlockFree(GenerationContext *set, GenerationBlock *block)
/*
* GenerationFree
- * Update number of chunks in the block, and if all chunks in the block
- * are now free then discard the block.
+ * Update number of chunks in the block, and consider freeing the block
+ * if it's become empty.
*/
void
GenerationFree(void *pointer)
@@ -694,43 +763,37 @@ GenerationFree(void *pointer)
Assert(block->nfree <= block->nchunks);
/* If there are still allocated chunks in the block, we're done. */
- if (block->nfree < block->nchunks)
+ if (likely(block->nfree < block->nchunks))
return;
set = block->context;
- /* Don't try to free the keeper block, just mark it empty */
- if (IsKeeperBlock(set, block))
- {
- GenerationBlockMarkEmpty(block);
- return;
- }
-
- /*
- * If there is no freeblock set or if this is the freeblock then instead
- * of freeing this memory, we keep it around so that new allocations have
- * the option of recycling it.
+ /*-----------------------
+ * The block this allocation was on has now become completely empty of
+ * chunks. In the general case, we can now return the memory for this
+ * block back to malloc. However, there are cases where we don't want to
+ * do that:
+ *
+ * 1) If it's the keeper block. This block is malloc'd with the context
+ * and can't be free'd without freeing the context itself.
+ * 2) If it's the current block. We could free this, but doing so would
+ * leave us nothing to set the current block to, so we just mark the
+ * block as empty so new allocations can reuse it again.
+ * 3) If we have no "freeblock" set, then to avoid new allocations from
+ * having to malloc a new block again, we save a single block to
+ * avoid that. This is especially useful for FIFO workloads as it
+ * avoids continual free/malloc cycles.
*/
- if (set->freeblock == NULL || set->freeblock == block)
+ if (IsKeeperBlock(set, block) || set->block == block)
+ GenerationBlockMarkEmpty(block); /* case 1 and 2 */
+ else if (set->freeblock == NULL)
{
- /* XXX should we only recycle maxBlockSize sized blocks? */
- set->freeblock = block;
+ /* case 3 */
GenerationBlockMarkEmpty(block);
- return;
+ set->freeblock = block;
}
-
- /* Also make sure the block is not marked as the current block. */
- if (set->block == block)
- set->block = NULL;
-
- /*
- * 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);
-
- set->header.mem_allocated -= block->blksize;
- free(block);
+ else
+ GenerationBlockFree(set, block); /* Otherwise, free it */
}
/*
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index bc91446cb3..c5f985d5ea 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -490,10 +490,139 @@ SlabDelete(MemoryContext context)
free(context);
}
+/*
+ * Small helper for allocating a new chunk from a chunk, to avoid duplicating
+ * the code between SlabAlloc() and SlabAllocFromNewBlock().
+ */
+static inline void *
+SlabAllocSetupNewChunk(MemoryContext context, SlabBlock *block,
+ MemoryChunk *chunk, Size size)
+{
+ SlabContext *slab = (SlabContext *) context;
+
+ /*
+ * Check that the chunk pointer is actually somewhere on the block and is
+ * aligned as expected.
+ */
+ Assert(chunk >= SlabBlockGetChunk(slab, block, 0));
+ Assert(chunk <= SlabBlockGetChunk(slab, block, slab->chunksPerBlock - 1));
+ Assert(SlabChunkMod(slab, block, chunk) == 0);
+
+ /* Prepare to initialize the chunk header. */
+ VALGRIND_MAKE_MEM_UNDEFINED(chunk, Slab_CHUNKHDRSZ);
+
+ MemoryChunkSetHdrMask(chunk, block, MAXALIGN(slab->chunkSize), MCTX_SLAB_ID);
+
+#ifdef MEMORY_CONTEXT_CHECKING
+ /* slab mark to catch clobber of "unused" space */
+ Assert(slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ));
+ set_sentinel(MemoryChunkGetPointer(chunk), size);
+ VALGRIND_MAKE_MEM_NOACCESS(((char *) chunk) + Slab_CHUNKHDRSZ +
+ slab->chunkSize,
+ slab->fullChunkSize -
+ (slab->chunkSize + Slab_CHUNKHDRSZ));
+#endif
+
+#ifdef RANDOMIZE_ALLOCATED_MEMORY
+ /* fill the allocated space with junk */
+ randomize_mem((char *) MemoryChunkGetPointer(chunk), size);
+#endif
+
+ /* Disallow access to the chunk header. */
+ VALGRIND_MAKE_MEM_NOACCESS(chunk, Slab_CHUNKHDRSZ);
+
+ return MemoryChunkGetPointer(chunk);
+}
+
+pg_noinline
+static void *
+SlabAllocFromNewBlock(MemoryContext context, Size size, int flags)
+{
+ SlabContext *slab = (SlabContext *) context;
+ SlabBlock *block;
+ MemoryChunk *chunk;
+ dlist_head *blocklist;
+ int blocklist_idx;
+
+ /* to save allocating a new one, first check the empty blocks list */
+ if (dclist_count(&slab->emptyblocks) > 0)
+ {
+ dlist_node *node = dclist_pop_head_node(&slab->emptyblocks);
+
+ block = dlist_container(SlabBlock, node, node);
+
+ /*
+ * SlabFree() should have left this block in a valid state with all
+ * chunks free. Ensure that's the case.
+ */
+ Assert(block->nfree == slab->chunksPerBlock);
+
+ /* fetch the next chunk from this block */
+ chunk = SlabGetNextFreeChunk(slab, block);
+ }
+ else
+ {
+ block = (SlabBlock *) malloc(slab->blockSize);
+
+ if (unlikely(block == NULL))
+ return MemoryContextAllocationFailure(context, size, flags);
+
+ block->slab = slab;
+ context->mem_allocated += slab->blockSize;
+
+ /* use the first chunk in the new block */
+ chunk = SlabBlockGetChunk(slab, block, 0);
+
+ block->nfree = slab->chunksPerBlock - 1;
+ block->unused = SlabBlockGetChunk(slab, block, 1);
+ block->freehead = NULL;
+ block->nunused = slab->chunksPerBlock - 1;
+ }
+
+ /* find the blocklist element for storing blocks with 1 used chunk */
+ blocklist_idx = SlabBlocklistIndex(slab, block->nfree);
+ blocklist = &slab->blocklist[blocklist_idx];
+
+ /* this better be empty. We just added a block thinking it was */
+ Assert(dlist_is_empty(blocklist));
+
+ dlist_push_head(blocklist, &block->node);
+
+ slab->curBlocklistIndex = blocklist_idx;
+
+ return SlabAllocSetupNewChunk(context, block, chunk, size);
+}
+
+/*
+ * SlabAllocInvalidSize
+ * Handle raising an ERROR for an invalid size request. We don't do this
+ * in slab alloc as calling the elog functions would force the compiler
+ * to setup the stack frame in SlabAlloc. For performance reasons, we
+ * want to avoid that.
+ */
+pg_noinline
+static void
+pg_attribute_noreturn()
+SlabAllocInvalidSize(MemoryContext context, Size size)
+{
+ SlabContext *slab = (SlabContext *) context;
+
+ elog(ERROR, "unexpected alloc chunk size %zu (expected %u)", size,
+ slab->chunkSize);
+}
+
/*
* SlabAlloc
- * Returns a pointer to allocated memory of given size or NULL if
- * request could not be completed; memory is added to the slab.
+ * Returns a pointer to allocated memory of given size or raises an ERROR
+ * on allocation failure, or returns NULL when flags contains
+ * MCXT_ALLOC_NO_OOM.
+ *
+ * This function should only contain the most common code paths. Everything
+ * else should be in pg_noinline helper functions, thus avoiding the overheads
+ * creating a stack frame for the common cases. Allocating memory is often a
+ * bottleneck in many workloads, so avoiding stack frame setup is worthwhile.
+ * Helper functions should always directly return the newly allocated memory
+ * so that we can just return that address directly as a tail call.
*/
void *
SlabAlloc(MemoryContext context, Size size, int flags)
@@ -513,66 +642,16 @@ SlabAlloc(MemoryContext context, Size size, int flags)
* MemoryContextCheckSize check.
*/
if (unlikely(size != slab->chunkSize))
- elog(ERROR, "unexpected alloc chunk size %zu (expected %u)",
- size, slab->chunkSize);
+ SlabAllocInvalidSize(context, size);
- /*
- * Handle the case when there are no partially filled blocks available.
- * SlabFree() will have updated the curBlocklistIndex setting it to zero
- * to indicate that it has freed the final block. Also later in
- * SlabAlloc() we will set the curBlocklistIndex to zero if we end up
- * filling the final block.
- */
if (unlikely(slab->curBlocklistIndex == 0))
{
- dlist_head *blocklist;
- int blocklist_idx;
-
- /* to save allocating a new one, first check the empty blocks list */
- if (dclist_count(&slab->emptyblocks) > 0)
- {
- dlist_node *node = dclist_pop_head_node(&slab->emptyblocks);
-
- block = dlist_container(SlabBlock, node, node);
-
- /*
- * SlabFree() should have left this block in a valid state with
- * all chunks free. Ensure that's the case.
- */
- Assert(block->nfree == slab->chunksPerBlock);
-
- /* fetch the next chunk from this block */
- chunk = SlabGetNextFreeChunk(slab, block);
- }
- else
- {
- block = (SlabBlock *) malloc(slab->blockSize);
-
- if (unlikely(block == NULL))
- return MemoryContextAllocationFailure(context, size, flags);
-
- block->slab = slab;
- context->mem_allocated += slab->blockSize;
-
- /* use the first chunk in the new block */
- chunk = SlabBlockGetChunk(slab, block, 0);
-
- block->nfree = slab->chunksPerBlock - 1;
- block->unused = SlabBlockGetChunk(slab, block, 1);
- block->freehead = NULL;
- block->nunused = slab->chunksPerBlock - 1;
- }
-
- /* find the blocklist element for storing blocks with 1 used chunk */
- blocklist_idx = SlabBlocklistIndex(slab, block->nfree);
- blocklist = &slab->blocklist[blocklist_idx];
-
- /* this better be empty. We just added a block thinking it was */
- Assert(dlist_is_empty(blocklist));
-
- dlist_push_head(blocklist, &block->node);
-
- slab->curBlocklistIndex = blocklist_idx;
+ /*
+ * Handle the case when there are no partially filled blocks
+ * available. This happens either when the last allocation took the
+ * last chunk in the block, or when SlabFree() free'd the final block.
+ */
+ return SlabAllocFromNewBlock(context, size, flags);
}
else
{
@@ -609,38 +688,7 @@ SlabAlloc(MemoryContext context, Size size, int flags)
}
}
- /*
- * Check that the chunk pointer is actually somewhere on the block and is
- * aligned as expected.
- */
- Assert(chunk >= SlabBlockGetChunk(slab, block, 0));
- Assert(chunk <= SlabBlockGetChunk(slab, block, slab->chunksPerBlock - 1));
- Assert(SlabChunkMod(slab, block, chunk) == 0);
-
- /* Prepare to initialize the chunk header. */
- VALGRIND_MAKE_MEM_UNDEFINED(chunk, Slab_CHUNKHDRSZ);
-
- MemoryChunkSetHdrMask(chunk, block, MAXALIGN(slab->chunkSize),
- MCTX_SLAB_ID);
-#ifdef MEMORY_CONTEXT_CHECKING
- /* slab mark to catch clobber of "unused" space */
- Assert(slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ));
- set_sentinel(MemoryChunkGetPointer(chunk), size);
- VALGRIND_MAKE_MEM_NOACCESS(((char *) chunk) +
- Slab_CHUNKHDRSZ + slab->chunkSize,
- slab->fullChunkSize -
- (slab->chunkSize + Slab_CHUNKHDRSZ));
-#endif
-
-#ifdef RANDOMIZE_ALLOCATED_MEMORY
- /* fill the allocated space with junk */
- randomize_mem((char *) MemoryChunkGetPointer(chunk), size);
-#endif
-
- /* Disallow access to the chunk header. */
- VALGRIND_MAKE_MEM_NOACCESS(chunk, Slab_CHUNKHDRSZ);
-
- return MemoryChunkGetPointer(chunk);
+ return SlabAllocSetupNewChunk(context, block, chunk, size);
}
/*
--
2.40.1
[image/png] slab_generation_benchmark_results.png (94.8K, ../CAApHDvpHVSJqqb4B4OZLixr=CotKq-eKkbwZqvZVo_biYvUvQA@mail.gmail.com/3-slab_generation_benchmark_results.png)
download | view image
view thread (12+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Avoid stack frame setup in performance critical routines using tail calls
In-Reply-To: <CAApHDvpHVSJqqb4B4OZLixr=CotKq-eKkbwZqvZVo_biYvUvQA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox