From a6b086302cc36adb33eb74c3d9bc7116695dc55a Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Mon, 20 Oct 2025 22:22:28 +0100 Subject: [PATCH] [Feature] Improve memory pool destructors and allocation strategies This commit introduces several improvements to the memory pool subsystem: 1. Priority-based destructors using binary heap: - Replace linked list with min-heap for deterministic destructor ordering - Add rspamd_mempool_add_destructor_priority() for priority control - Maintain backward compatibility with existing rspamd_mempool_add_destructor() - Destructors now execute in priority order (lowest first) 2. Destructor statistics and preallocation: - Track destructor count per allocation point in entry statistics - Preallocate heap based on historical usage patterns - Adaptive sizing with configurable maximum (128 destructors) 3. Pool type differentiation: - Add RSPAMD_MEMPOOL_LONG_LIVED flag for configuration/global data - Add RSPAMD_MEMPOOL_SHORT_LIVED flag for task/temporary data - Optimize page sizes: 16KB minimum for long-lived, 4KB for short-lived - Provide convenience macros: rspamd_mempool_new_long_lived() and rspamd_mempool_new_short_lived() 4. Heap utility enhancements: - Add rspamd_min_heap_size() to query heap element count - Enable better integration with pool statistics Benefits: - Controlled resource cleanup order prevents use-after-free scenarios - Reduced memory fragmentation for long-lived pools - Better performance for frequently created/destroyed short-lived pools - Automatic adaptation to actual usage patterns --- src/libutil/heap.c | 7 ++ src/libutil/heap.h | 7 ++ src/libutil/mem_pool.c | 177 +++++++++++++++++++++++++------- src/libutil/mem_pool.h | 21 +++- src/libutil/mem_pool_internal.h | 23 +++-- 5 files changed, 187 insertions(+), 48 deletions(-) diff --git a/src/libutil/heap.c b/src/libutil/heap.c index dcff58b400..23219f528d 100644 --- a/src/libutil/heap.c +++ b/src/libutil/heap.c @@ -195,3 +195,10 @@ rspamd_min_heap_index(struct rspamd_min_heap *heap, unsigned int idx) return g_ptr_array_index(heap->ar, idx); } + +gsize rspamd_min_heap_size(struct rspamd_min_heap *heap) +{ + g_assert(heap != NULL); + + return heap->ar->len; +} diff --git a/src/libutil/heap.h b/src/libutil/heap.h index d56ee02b23..28cf6b9daf 100644 --- a/src/libutil/heap.h +++ b/src/libutil/heap.h @@ -90,6 +90,13 @@ void rspamd_min_heap_destroy(struct rspamd_min_heap *heap); struct rspamd_min_heap_elt *rspamd_min_heap_index(struct rspamd_min_heap *heap, unsigned int idx); +/** + * Returns the number of elements in the heap + * @param heap + * @return number of elements + */ +gsize rspamd_min_heap_size(struct rspamd_min_heap *heap); + #ifdef __cplusplus } #endif diff --git a/src/libutil/mem_pool.c b/src/libutil/mem_pool.c index 575b4e4970..ccdaf558df 100644 --- a/src/libutil/mem_pool.c +++ b/src/libutil/mem_pool.c @@ -23,6 +23,7 @@ #include "cryptobox.h" #include "contrib/uthash/utlist.h" #include "mem_pool_internal.h" +#include "heap.h" #ifdef WITH_JEMALLOC #include @@ -341,6 +342,25 @@ rspamd_mempool_new_(gsize size, const char *tag, int flags, const char *loc) size = entry->cur_suggestion; } + /* Adjust size based on pool type */ + if (flags & RSPAMD_MEMPOOL_LONG_LIVED) { + /* Long-lived pools: use larger pages to reduce fragmentation */ + if (size < 16384) { + size = 16384; /* 16KB minimum for long-lived pools */ + } + } + else if (flags & RSPAMD_MEMPOOL_SHORT_LIVED) { + /* Short-lived pools: optimize for quick allocation/deallocation */ + if (entry && entry->cur_suggestion > 0) { + /* Use entry point statistics for short-lived pools */ + size = entry->cur_suggestion; + } + else if (size == 0) { + /* Default to smaller size for short-lived */ + size = 4096; + } + } + total_size = sizeof(rspamd_mempool_t) + sizeof(struct rspamd_mempool_specific) + MIN_MEM_ALIGNMENT + @@ -644,28 +664,44 @@ void rspamd_mempool_add_destructor_full(rspamd_mempool_t *pool, rspamd_mempool_destruct_t func, void *data, const char *function, - const char *line) + const char *line, + unsigned int priority) { - struct _pool_destructors *cur; + struct _pool_destructors *dtor; + struct _pool_destructor_heap_elt *heap_wrapper; POOL_MTX_LOCK(); - cur = rspamd_mempool_alloc_(pool, sizeof(*cur), - RSPAMD_ALIGNOF(struct _pool_destructors), line); - cur->func = func; - cur->data = data; - cur->function = function; - cur->loc = line; - cur->next = NULL; - - if (pool->priv->dtors_tail) { - pool->priv->dtors_tail->next = cur; - pool->priv->dtors_tail = cur; - } - else { - pool->priv->dtors_head = cur; - pool->priv->dtors_tail = cur; + + /* Allocate destructor structure */ + dtor = rspamd_mempool_alloc_(pool, sizeof(*dtor), + RSPAMD_ALIGNOF(struct _pool_destructors), line); + dtor->func = func; + dtor->data = data; + dtor->function = function; + dtor->loc = line; + dtor->priority = priority; + + /* Initialize heap if not yet created */ + if (pool->priv->dtors_heap == NULL) { + gsize reserved_size = 8; + + /* Use entry point statistics if available */ + if (pool->priv->entry && pool->priv->entry->cur_dtors > 0) { + reserved_size = pool->priv->entry->cur_dtors; + } + + pool->priv->dtors_heap = rspamd_min_heap_create(reserved_size); } + /* Allocate heap wrapper */ + heap_wrapper = rspamd_mempool_alloc_(pool, sizeof(*heap_wrapper), + RSPAMD_ALIGNOF(struct _pool_destructor_heap_elt), line); + heap_wrapper->dtor = dtor; + heap_wrapper->heap_elt.data = heap_wrapper; + heap_wrapper->heap_elt.pri = priority; + + rspamd_min_heap_push(pool->priv->dtors_heap, &heap_wrapper->heap_elt); + POOL_MTX_UNLOCK(); } @@ -674,13 +710,24 @@ void rspamd_mempool_replace_destructor(rspamd_mempool_t *pool, void *old_data, void *new_data) { - struct _pool_destructors *tmp; + struct _pool_destructor_heap_elt *heap_wrapper; + struct _pool_destructors *dtor; + struct rspamd_min_heap_elt *elt; + gsize i, heap_size; + + if (pool->priv->dtors_heap == NULL) { + return; + } - LL_FOREACH(pool->priv->dtors_head, tmp) - { - if (tmp->func == func && tmp->data == old_data) { - tmp->func = func; - tmp->data = new_data; + /* Linear search through heap to find matching destructor */ + heap_size = rspamd_min_heap_size(pool->priv->dtors_heap); + for (i = 0; i < heap_size; i++) { + elt = rspamd_min_heap_index(pool->priv->dtors_heap, i); + heap_wrapper = (struct _pool_destructor_heap_elt *) elt->data; + dtor = heap_wrapper->dtor; + + if (dtor->func == func && dtor->data == old_data) { + dtor->data = new_data; break; } } @@ -781,19 +828,28 @@ rspamd_mempool_variables_cleanup(rspamd_mempool_t *pool) void rspamd_mempool_destructors_enforce(rspamd_mempool_t *pool) { - struct _pool_destructors *destructor; + struct _pool_destructor_heap_elt *heap_wrapper; + struct _pool_destructors *dtor; + struct rspamd_min_heap_elt *elt; POOL_MTX_LOCK(); - LL_FOREACH(pool->priv->dtors_head, destructor) - { - /* Avoid calling destructors for NULL pointers */ - if (destructor->data != NULL) { - destructor->func(destructor->data); + if (pool->priv->dtors_heap) { + /* Pop destructors in priority order (min heap = lowest priority first) */ + while ((elt = rspamd_min_heap_pop(pool->priv->dtors_heap)) != NULL) { + heap_wrapper = (struct _pool_destructor_heap_elt *) elt->data; + dtor = heap_wrapper->dtor; + + /* Avoid calling destructors for NULL pointers */ + if (dtor->data != NULL) { + dtor->func(dtor->data); + } } - } - pool->priv->dtors_head = pool->priv->dtors_tail = NULL; + /* Destroy the heap itself */ + rspamd_min_heap_destroy(pool->priv->dtors_heap); + pool->priv->dtors_heap = NULL; + } rspamd_mempool_variables_cleanup(pool); @@ -817,7 +873,8 @@ rspamd_mempool_debug_elt_cmp(const void *a, const void *b) void rspamd_mempool_delete(rspamd_mempool_t *pool) { struct _pool_chain *cur, *tmp; - struct _pool_destructors *destructor; + struct _pool_destructors *dtor; + struct rspamd_min_heap_elt *elt; gpointer ptr; unsigned int i; gsize len; @@ -830,7 +887,11 @@ void rspamd_mempool_delete(rspamd_mempool_t *pool) GHashTable *debug_tbl = *(GHashTable **) (((unsigned char *) pool) + sizeof(*pool)); /* Show debug info */ gsize ndtor = 0; - LL_COUNT(pool->priv->dtors_head, destructor, ndtor); + + if (pool->priv->dtors_heap) { + ndtor = rspamd_min_heap_size(pool->priv->dtors_heap); + } + msg_info_pool("destructing of the memory pool %p; elt size = %z; " "used memory = %Hz; wasted memory = %Hd; " "vars = %z; destructors = %z", @@ -881,13 +942,51 @@ void rspamd_mempool_delete(rspamd_mempool_t *pool) } } - /* Call all pool destructors */ - LL_FOREACH(pool->priv->dtors_head, destructor) - { - /* Avoid calling destructors for NULL pointers */ - if (destructor->data != NULL) { - destructor->func(destructor->data); + /* Call all pool destructors in priority order */ + if (pool->priv->dtors_heap) { + struct _pool_destructor_heap_elt *heap_wrapper; + gsize ndtors = 0; + + /* Update destructor statistics before destroying */ + if (pool->priv->entry && mempool_entries) { + ndtors = rspamd_min_heap_size(pool->priv->dtors_heap); + + /* Update suggestion using similar logic to variables */ + if (pool->priv->entry->cur_dtors < ndtors) { + static const unsigned int max_preallocated_dtors = 128; + + unsigned int old_guess = pool->priv->entry->cur_dtors; + unsigned int new_guess; + + if (old_guess == 0) { + new_guess = MIN(ndtors, max_preallocated_dtors); + } + else { + if (old_guess * 2 < ndtors) { + new_guess = MIN(ndtors, max_preallocated_dtors); + } + else { + /* Too large step */ + new_guess = MIN(old_guess * 2, max_preallocated_dtors); + } + } + + pool->priv->entry->cur_dtors = new_guess; + } } + + while ((elt = rspamd_min_heap_pop(pool->priv->dtors_heap)) != NULL) { + heap_wrapper = (struct _pool_destructor_heap_elt *) elt->data; + dtor = heap_wrapper->dtor; + + /* Avoid calling destructors for NULL pointers */ + if (dtor->data != NULL) { + dtor->func(dtor->data); + } + } + + rspamd_min_heap_destroy(pool->priv->dtors_heap); + pool->priv->dtors_heap = NULL; } rspamd_mempool_variables_cleanup(pool); diff --git a/src/libutil/mem_pool.h b/src/libutil/mem_pool.h index 00d1a20676..16bd6f143c 100644 --- a/src/libutil/mem_pool.h +++ b/src/libutil/mem_pool.h @@ -111,6 +111,8 @@ struct rspamd_mempool_tag { enum rspamd_mempool_flags { RSPAMD_MEMPOOL_DEBUG = (1u << 0u), + RSPAMD_MEMPOOL_LONG_LIVED = (1u << 1u), /**< Pool for long-lived data (config, etc.) */ + RSPAMD_MEMPOOL_SHORT_LIVED = (1u << 2u), /**< Pool for short-lived data (tasks, etc.) */ }; /** @@ -152,6 +154,12 @@ rspamd_mempool_t *rspamd_mempool_new_(gsize size, const char *tag, int flags, #define rspamd_mempool_new_default(tag, flags) \ rspamd_mempool_new_(rspamd_mempool_suggest_size_(G_STRLOC), (tag), (flags), G_STRLOC) +/* Convenient macros for creating typed pools */ +#define rspamd_mempool_new_long_lived(size, tag) \ + rspamd_mempool_new_((size), (tag), RSPAMD_MEMPOOL_LONG_LIVED, G_STRLOC) +#define rspamd_mempool_new_short_lived(tag) \ + rspamd_mempool_new_(rspamd_mempool_suggest_size_(G_STRLOC), (tag), RSPAMD_MEMPOOL_SHORT_LIVED, G_STRLOC) + /** * Get memory from pool * @param pool memory pool object @@ -258,20 +266,27 @@ void *rspamd_mempool_alloc0_shared_(rspamd_mempool_t *pool, gsize size, gsize al MAX(MIN_MEM_ALIGNMENT, RSPAMD_ALIGNOF(type)), (G_STRLOC))) /** - * Add destructor callback to pool + * Add destructor callback to pool with priority * @param pool memory pool object * @param func pointer to function-destructor * @param data pointer to data that would be passed to destructor + * @param function function name + * @param line line number + * @param priority destructor priority (lower values = earlier execution, 0 = default) */ void rspamd_mempool_add_destructor_full(rspamd_mempool_t *pool, rspamd_mempool_destruct_t func, void *data, const char *function, - const char *line); + const char *line, + unsigned int priority); /* Macros for common usage */ #define rspamd_mempool_add_destructor(pool, func, data) \ - rspamd_mempool_add_destructor_full(pool, func, data, (MEMPOOL_STR_FUNC), (G_STRLOC)) + rspamd_mempool_add_destructor_full(pool, func, data, (MEMPOOL_STR_FUNC), (G_STRLOC), 0) + +#define rspamd_mempool_add_destructor_priority(pool, func, data, priority) \ + rspamd_mempool_add_destructor_full(pool, func, data, (MEMPOOL_STR_FUNC), (G_STRLOC), priority) /** * Replace destructor callback to pool for specified pointer diff --git a/src/libutil/mem_pool_internal.h b/src/libutil/mem_pool_internal.h index 2f9ad15b69..26193ffbf0 100644 --- a/src/libutil/mem_pool_internal.h +++ b/src/libutil/mem_pool_internal.h @@ -17,6 +17,9 @@ #ifndef RSPAMD_MEM_POOL_INTERNAL_H #define RSPAMD_MEM_POOL_INTERNAL_H +#include "config.h" +#include "heap.h" + /* * Internal memory pool stuff */ @@ -42,20 +45,28 @@ struct rspamd_mempool_entry_point { uint32_t cur_suggestion; uint32_t cur_elts; uint32_t cur_vars; + uint32_t cur_dtors; /**< suggested number of destructors to preallocate */ struct entry_elt elts[ENTRY_NELTS]; }; /** - * Destructors list item structure + * Destructors heap item structure */ struct _pool_destructors { - rspamd_mempool_destruct_t func; /**< pointer to destructor */ - void *data; /**< data to free */ + rspamd_mempool_destruct_t func; /**< pointer to destructor */ + void *data; /**< data to free */ const char *function; /**< function from which this destructor was added */ - const char *loc; /**< line number */ - struct _pool_destructors *next; + const char *loc; /**< line number */ + unsigned int priority; /**< destructor priority (0 = default) */ }; +/** + * Wrapper for destructor with heap element + */ +struct _pool_destructor_heap_elt { + struct rspamd_min_heap_elt heap_elt; + struct _pool_destructors *dtor; +}; struct rspamd_mempool_variable { gpointer data; @@ -68,7 +79,7 @@ KHASH_INIT(rspamd_mempool_vars_hash, struct rspamd_mempool_specific { struct _pool_chain *pools[RSPAMD_MEMPOOL_MAX]; - struct _pool_destructors *dtors_head, *dtors_tail; + struct rspamd_min_heap *dtors_heap; GPtrArray *trash_stack; khash_t(rspamd_mempool_vars_hash) * variables; struct rspamd_mempool_entry_point *entry; -- 2.47.3