From 9b6ffdceb3c6f592d3d1597b19f1240d4a78d848 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 23 Feb 2024 00:27:07 +1300 Subject: [PATCH v3 2/2] fixup! Optimize palloc() etc to allow sibling calls --- src/backend/utils/mmgr/mcxt.c | 67 +++++++++++++++++++++++++-- src/backend/utils/mmgr/slab.c | 11 ++--- src/include/nodes/memnodes.h | 38 +++++++++++++++ src/include/utils/memutils_internal.h | 7 +-- 4 files changed, 110 insertions(+), 13 deletions(-) diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c index 2947b2046b..c214d323c8 100644 --- a/src/backend/utils/mmgr/mcxt.c +++ b/src/backend/utils/mmgr/mcxt.c @@ -1023,6 +1023,12 @@ MemoryContextCreate(MemoryContext node, VALGRIND_CREATE_MEMPOOL(node, 0, false); } +/* + * MemoryContextAllocationFailure + * For use by MemoryContextMethods implementations to handle when malloc + * returns NULL. The bahavior is specific to whether MCXT_ALLOC_NO_OOM + * is in 'flags'. + */ void * MemoryContextAllocationFailure(MemoryContext context, Size size, int flags) { @@ -1038,6 +1044,11 @@ MemoryContextAllocationFailure(MemoryContext context, Size size, int flags) return NULL; } +/* + * MemoryContextSizeFailure + * For use by MemoryContextMethods implementations to handle invalid + * memory allocation request sizes. + */ void MemoryContextSizeFailure(MemoryContext context, Size size, int flags) { @@ -1061,6 +1072,16 @@ MemoryContextAlloc(MemoryContext context, Size size) context->isReset = false; + /* + * For efficiency reasons, we purposefully offload the handling of + * allocation failures to the MemoryContextMethods implementation as this + * allows these checks to be performed only when an actual malloc needs to + * be done to request more memory from the OS. Additionally, not having + * to execute any instructions after this call allows the compiler to use + * the tailcall optimization. If you're considering adding code after + * this call, consider making it the responsibility of the 'alloc' + * function instead. + */ ret = context->methods->alloc(context, size, 0); VALGRIND_MEMPOOL_ALLOC(context, ret, size); @@ -1192,9 +1213,19 @@ palloc(Size size) context->isReset = false; + /* + * For efficiency reasons, we purposefully offload the handling of + * allocation failures to the MemoryContextMethods implementation as this + * allows these checks to be performed only when an actual malloc needs to + * be done to request more memory from the OS. Additionally, not having + * to execute any instructions after this call allows the compiler to use + * the tailcall optimization. If you're considering adding code after + * this call, consider making it the responsibility of the 'alloc' + * function instead. + */ ret = context->methods->alloc(context, size, 0); - - Assert(ret != 0); + /* We expect OOM to be handled by the alloc function */ + Assert(ret != NULL); VALGRIND_MEMPOOL_ALLOC(context, ret, size); return ret; @@ -1410,10 +1441,20 @@ repalloc(void *pointer, Size size) /* isReset must be false already */ Assert(!context->isReset); + /* + * For efficiency reasons, we purposefully offload the handling of + * allocation failures to the MemoryContextMethods implementation as this + * allows these checks to be performed only when an actual malloc needs to + * be done to request more memory from the OS. Additionally, not having + * to execute any instructions after this call allows the compiler to use + * the tailcall optimization. If you're considering adding code after + * this call, consider making it the responsibility of the 'realloc' + * function instead. + */ ret = MCXT_METHOD(pointer, realloc) (pointer, size, 0); #ifdef USE_VALGRIND - if (ret != NULL && method != MCTX_ALIGNED_REDIRECT_ID) + if (method != MCTX_ALIGNED_REDIRECT_ID) VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size); #endif @@ -1438,6 +1479,16 @@ repalloc_extended(void *pointer, Size size, int flags) /* isReset must be false already */ Assert(!context->isReset); + /* + * For efficiency reasons, we purposefully offload the handling of + * allocation failures to the MemoryContextMethods implementation as this + * allows these checks to be performed only when an actual malloc needs to + * be done to request more memory from the OS. Additionally, not having + * to execute any instructions after this call allows the compiler to use + * the tailcall optimization. If you're considering adding code after + * this call, consider making it the responsibility of the 'realloc' + * function instead. + */ ret = MCXT_METHOD(pointer, realloc) (pointer, size, flags); if (unlikely(ret == NULL)) return NULL; @@ -1483,6 +1534,16 @@ MemoryContextAllocHuge(MemoryContext context, Size size) context->isReset = false; + /* + * For efficiency reasons, we purposefully offload the handling of + * allocation failures to the MemoryContextMethods implementation as this + * allows these checks to be performed only when an actual malloc needs to + * be done to request more memory from the OS. Additionally, not having + * to execute any instructions after this call allows the compiler to use + * the tailcall optimization. If you're considering adding code after + * this call, consider making it the responsibility of the 'alloc' + * function instead. + */ ret = context->methods->alloc(context, size, MCXT_ALLOC_HUGE); VALGRIND_MEMPOOL_ALLOC(context, ret, size); diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c index 1345c8d500..bc91446cb3 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -504,17 +504,14 @@ SlabAlloc(MemoryContext context, Size size, int flags) Assert(SlabIsValid(slab)); - /* - * XXX: Probably no need to check for huge allocations, we only support - * one size? Which could theoretically be huge, but that'd not make - * sense... - */ - /* sanity check that this is pointing to a valid blocklist */ Assert(slab->curBlocklistIndex >= 0); Assert(slab->curBlocklistIndex <= SlabBlocklistIndex(slab, slab->chunksPerBlock)); - /* make sure we only allow correct request size */ + /* + * Make sure we only allow correct request size. This doubles as the + * MemoryContextCheckSize check. + */ if (unlikely(size != slab->chunkSize)) elog(ERROR, "unexpected alloc chunk size %zu (expected %u)", size, slab->chunkSize); diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h index cc8cc58a0c..edc0257f36 100644 --- a/src/include/nodes/memnodes.h +++ b/src/include/nodes/memnodes.h @@ -57,20 +57,58 @@ typedef void (*MemoryStatsPrintFunc) (MemoryContext context, void *passthru, typedef struct MemoryContextMethods { + /* + * Function to handle memory allocation requests of 'size' to allocate + * memory into the given 'context'. The function must handle flags + * MCXT_ALLOC_HUGE and MCXT_ALLOC_NO_OOM. MCXT_ALLOC_ZERO is handled by + * the calling function. + */ void *(*alloc) (MemoryContext context, Size size, int flags); + /* call this free_p in case someone #define's free() */ void (*free_p) (void *pointer); + + /* + * Function to handle a size change request for an existing allocation. + * The implementation must handle flags MCXT_ALLOC_HUGE and + * MCXT_ALLOC_NO_OOM. MCXT_ALLOC_ZERO is handled by the calling function. + */ void *(*realloc) (void *pointer, Size size, int flags); + + /* + * Invalidate all previous allocations in the given memory context and + * prepare the context for a new set of allocations. Implementations may + * optionally free() excess memory back to the OS during this time. + */ void (*reset) (MemoryContext context); + + /* Free all memory consumed by the given MemoryContext. */ void (*delete_context) (MemoryContext context); + + /* Return the MemoryContext that the given pointer belongs to. */ MemoryContext (*get_chunk_context) (void *pointer); + + /* + * Return the number of bytes consumed by the given pointer within its + * memory context, including the overhead of alignment and chunk headers. + */ Size (*get_chunk_space) (void *pointer); + + /* + * Return true if the given MemoryContext has not had any allocations + * since it was created or last reset. + */ bool (*is_empty) (MemoryContext context); void (*stats) (MemoryContext context, MemoryStatsPrintFunc printfunc, void *passthru, MemoryContextCounters *totals, bool print_to_stderr); #ifdef MEMORY_CONTEXT_CHECKING + + /* + * Perform validation checks on the given context and raise any discovered + * anomalies as WARNINGs. + */ void (*check) (MemoryContext context); #endif } MemoryContextMethods; diff --git a/src/include/utils/memutils_internal.h b/src/include/utils/memutils_internal.h index e5cecd7122..ad1048fd82 100644 --- a/src/include/utils/memutils_internal.h +++ b/src/include/utils/memutils_internal.h @@ -133,9 +133,11 @@ extern void MemoryContextCreate(MemoryContext node, MemoryContext parent, const char *name); -extern void *MemoryContextAllocationFailure(MemoryContext context, Size size, int flags); +extern void *MemoryContextAllocationFailure(MemoryContext context, Size size, + int flags); -extern void MemoryContextSizeFailure(MemoryContext context, Size size, int flags) pg_attribute_noreturn(); +extern void MemoryContextSizeFailure(MemoryContext context, Size size, + int flags) pg_attribute_noreturn(); static inline void MemoryContextCheckSize(MemoryContext context, Size size, int flags) @@ -147,5 +149,4 @@ MemoryContextCheckSize(MemoryContext context, Size size, int flags) } } - #endif /* MEMUTILS_INTERNAL_H */ -- 2.40.1