From: Vsevolod Stakhov Date: Tue, 21 Oct 2025 09:37:28 +0000 (+0100) Subject: [Rework] Convert heap to fully intrusive kvec-based implementation X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0cd32b4d45a7bd8a95472f229dc553516c18af56;p=thirdparty%2Frspamd.git [Rework] Convert heap to fully intrusive kvec-based implementation Convert the heap implementation from pointer-based to fully intrusive design where elements are stored directly in the kvec array. Key changes: - Remove heap.c, convert to macro-only header implementation - Store elements by value in kvec_t(elt_type) instead of kvec_t(elt_type *) - Improve cache locality by eliminating pointer indirection - Fix swim/sink operations to properly track elements during swaps - Update rspamd_heap_pop to return pointer to popped element - Update memory pool destructor heap to use new intrusive API - Update heap tests for value-based element storage Performance benefits: - Better cache locality (elements stored contiguously) - No per-element allocation overhead - Reduced memory usage (no pointer array) --- diff --git a/src/libutil/CMakeLists.txt b/src/libutil/CMakeLists.txt index fa42ca4756..ced8fb6a7a 100644 --- a/src/libutil/CMakeLists.txt +++ b/src/libutil/CMakeLists.txt @@ -16,7 +16,6 @@ SET(LIBRSPAMDUTILSRC ${CMAKE_CURRENT_SOURCE_DIR}/str_util.c ${CMAKE_CURRENT_SOURCE_DIR}/upstream.c ${CMAKE_CURRENT_SOURCE_DIR}/util.c - ${CMAKE_CURRENT_SOURCE_DIR}/heap.c ${CMAKE_CURRENT_SOURCE_DIR}/multipattern.c ${CMAKE_CURRENT_SOURCE_DIR}/cxx/utf8_util.cxx ${CMAKE_CURRENT_SOURCE_DIR}/cxx/rspamd-simdutf.cxx diff --git a/src/libutil/heap.c b/src/libutil/heap.c deleted file mode 100644 index 23219f528d..0000000000 --- a/src/libutil/heap.c +++ /dev/null @@ -1,204 +0,0 @@ -/*- - * Copyright 2016 Vsevolod Stakhov - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "config.h" -#include "libutil/heap.h" - -struct rspamd_min_heap { - GPtrArray *ar; -}; - -#define __SWAP(a, b) \ - do { \ - __typeof__(a) _a = (a); \ - __typeof__(b) _b = (b); \ - a = _b; \ - b = _a; \ - } while (0) -#define heap_swap(h, e1, e2) \ - do { \ - __SWAP((h)->ar->pdata[(e1)->idx - 1], (h)->ar->pdata[(e2)->idx - 1]); \ - __SWAP((e1)->idx, (e2)->idx); \ - } while (0) - -#define min_elt(e1, e2) ((e1)->pri <= (e2)->pri ? (e1) : (e2)) - -/* - * Swims element added (or changed) to preserve heap's invariant - */ -static void -rspamd_min_heap_swim(struct rspamd_min_heap *heap, - struct rspamd_min_heap_elt *elt) -{ - struct rspamd_min_heap_elt *parent; - - while (elt->idx > 1) { - parent = g_ptr_array_index(heap->ar, elt->idx / 2 - 1); - - if (parent->pri > elt->pri) { - heap_swap(heap, elt, parent); - } - else { - break; - } - } -} - -/* - * Sinks the element popped (or changed) to preserve heap's invariant - */ -static void -rspamd_min_heap_sink(struct rspamd_min_heap *heap, - struct rspamd_min_heap_elt *elt) -{ - struct rspamd_min_heap_elt *c1, *c2, *m; - - while (elt->idx * 2 < heap->ar->len) { - c1 = g_ptr_array_index(heap->ar, elt->idx * 2 - 1); - c2 = g_ptr_array_index(heap->ar, elt->idx * 2); - m = min_elt(c1, c2); - - if (elt->pri > m->pri) { - heap_swap(heap, elt, m); - } - else { - break; - } - } - - if (elt->idx * 2 - 1 < heap->ar->len) { - m = g_ptr_array_index(heap->ar, elt->idx * 2 - 1); - if (elt->pri > m->pri) { - heap_swap(heap, elt, m); - } - } -} - -struct rspamd_min_heap * -rspamd_min_heap_create(gsize reserved_size) -{ - struct rspamd_min_heap *heap; - - heap = g_malloc(sizeof(*heap)); - heap->ar = g_ptr_array_sized_new(reserved_size); - - return heap; -} - -void rspamd_min_heap_push(struct rspamd_min_heap *heap, - struct rspamd_min_heap_elt *elt) -{ - g_assert(heap != NULL); - g_assert(elt != NULL); - - /* Add to the end */ - elt->idx = heap->ar->len + 1; - g_ptr_array_add(heap->ar, elt); - /* Now swim it up */ - rspamd_min_heap_swim(heap, elt); -} - -struct rspamd_min_heap_elt * -rspamd_min_heap_pop(struct rspamd_min_heap *heap) -{ - struct rspamd_min_heap_elt *elt, *last; - - g_assert(heap != NULL); - - if (heap->ar->len == 0) { - return NULL; - } - - elt = g_ptr_array_index(heap->ar, 0); - last = g_ptr_array_index(heap->ar, heap->ar->len - 1); - - if (elt != last) { - /* Now replace elt with the last element and sink it if needed */ - heap_swap(heap, elt, last); - g_ptr_array_remove_index_fast(heap->ar, heap->ar->len - 1); - rspamd_min_heap_sink(heap, last); - } - else { - g_ptr_array_remove_index_fast(heap->ar, heap->ar->len - 1); - } - - - return elt; -} - -void rspamd_min_heap_update_elt(struct rspamd_min_heap *heap, - struct rspamd_min_heap_elt *elt, unsigned int npri) -{ - unsigned int oldpri; - - g_assert(heap != NULL); - g_assert(elt->idx > 0 && elt->idx <= heap->ar->len); - - oldpri = elt->pri; - elt->pri = npri; - - if (npri > oldpri) { - /* We might need to sink */ - rspamd_min_heap_sink(heap, elt); - } - else if (npri < oldpri) { - /* We might need to swim */ - rspamd_min_heap_swim(heap, elt); - } -} - -void rspamd_min_heap_remove_elt(struct rspamd_min_heap *heap, - struct rspamd_min_heap_elt *elt) -{ - struct rspamd_min_heap_elt *first; - - g_assert(heap != NULL); - g_assert(elt->idx > 0 && elt->idx <= heap->ar->len); - - first = g_ptr_array_index(heap->ar, 0); - - if (elt != first) { - elt->pri = first->pri - 1; - rspamd_min_heap_swim(heap, elt); - } - - /* Now the desired element is on the top of queue */ - (void) rspamd_min_heap_pop(heap); -} - -void rspamd_min_heap_destroy(struct rspamd_min_heap *heap) -{ - if (heap) { - g_ptr_array_free(heap->ar, TRUE); - g_free(heap); - } -} - -struct rspamd_min_heap_elt * -rspamd_min_heap_index(struct rspamd_min_heap *heap, unsigned int idx) -{ - g_assert(heap != NULL); - g_assert(idx < heap->ar->len); - - 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 28cf6b9daf..50a2222866 100644 --- a/src/libutil/heap.h +++ b/src/libutil/heap.h @@ -17,85 +17,223 @@ #define SRC_LIBUTIL_HEAP_H_ #include "config.h" +#include "contrib/libucl/kvec.h" #ifdef __cplusplus extern "C" { #endif /** - * Binary minimal heap interface based on glib + * Fully intrusive binary min-heap implementation using kvec + * + * Elements are stored directly in the array (not pointers), providing + * excellent cache locality and eliminating pointer indirection overhead. + * + * Requirements for element type: + * - Must have 'unsigned int pri' field for priority + * - Must have 'unsigned int idx' field for heap index (managed internally) + * + * If you need a heap of large objects, embed a pointer in your element type: + * + * struct my_heap_entry { + * unsigned int pri; + * unsigned int idx; + * struct large_object *data; // pointer to actual data + * }; + * + * Example usage: + * + * struct my_element { + * unsigned int pri; // priority (lower = higher priority) + * unsigned int idx; // heap index (managed by heap) + * int data; // your data stored directly + * }; + * + * RSPAMD_HEAP_DECLARE(my_heap, struct my_element); + * + * struct my_heap heap; + * rspamd_heap_init(my_heap, &heap); + * + * struct my_element elt = {.pri = 10, .data = 42}; + * rspamd_heap_push_safe(my_heap, &heap, &elt, error); + * + * struct my_element *min = rspamd_heap_pop(my_heap, &heap); + * rspamd_heap_destroy(my_heap, &heap); */ -struct rspamd_min_heap_elt { - gpointer data; - unsigned int pri; - unsigned int idx; -}; +/** + * Declare heap type + * @param name heap type name + * @param elt_type element type (must have 'pri' and 'idx' fields) + */ +#define RSPAMD_HEAP_DECLARE(name, elt_type) \ + typedef kvec_t(elt_type) name##_t -struct rspamd_min_heap; +/** + * Initialize heap + */ +#define rspamd_heap_init(name, heap) kv_init(*(heap)) /** - * Creates min heap with the specified reserved size and compare function - * @param reserved_size reserved size in elements - * @return opaque minimal heap + * Destroy heap (does not free elements as they're stored inline) */ -struct rspamd_min_heap *rspamd_min_heap_create(gsize reserved_size); +#define rspamd_heap_destroy(name, heap) kv_destroy(*(heap)) /** - * Pushes an element to the heap. `pri` should be initialized to use this function, - * `idx` is used internally by heap interface - * @param heap heap structure - * @param elt element to push + * Get heap size */ -void rspamd_min_heap_push(struct rspamd_min_heap *heap, - struct rspamd_min_heap_elt *elt); +#define rspamd_heap_size(name, heap) kv_size(*(heap)) /** - * Pops the minimum element from the heap and reorder the queue - * @param heap heap structure - * @return minimum element + * Get pointer to element at index (0-based) */ -struct rspamd_min_heap_elt *rspamd_min_heap_pop(struct rspamd_min_heap *heap); +#define rspamd_heap_index(name, heap, i) (&kv_A(*(heap), i)) /** - * Updates priority for the element. It must be in queue (so `idx` should be sane) - * @param heap heap structure - * @param elt element to update - * @param npri new priority + * Swim element up to maintain heap invariant */ -void rspamd_min_heap_update_elt(struct rspamd_min_heap *heap, - struct rspamd_min_heap_elt *elt, unsigned int npri); +#define rspamd_heap_swim(name, heap, elt) \ + do { \ + unsigned int cur_idx = (elt)->idx; \ + while (cur_idx > 0) { \ + unsigned int parent_idx = (cur_idx - 1) / 2; \ + typeof(elt) cur = &kv_A(*(heap), cur_idx); \ + typeof(elt) parent = &kv_A(*(heap), parent_idx); \ + if (parent->pri > cur->pri) { \ + /* Swap elements directly */ \ + typeof(*elt) tmp = *parent; \ + *parent = *cur; \ + *cur = tmp; \ + /* Update indices */ \ + parent->idx = parent_idx; \ + cur->idx = cur_idx; \ + /* Move up */ \ + cur_idx = parent_idx; \ + } \ + else { \ + break; \ + } \ + } \ + } while (0) +/** + * Sink element down to maintain heap invariant + */ +#define rspamd_heap_sink(name, heap, elt) \ + do { \ + unsigned int size = kv_size(*(heap)); \ + unsigned int cur_idx = (elt)->idx; \ + while (cur_idx * 2 + 1 < size) { \ + unsigned int left_idx = cur_idx * 2 + 1; \ + unsigned int right_idx = cur_idx * 2 + 2; \ + unsigned int min_idx = left_idx; \ + typeof(elt) cur = &kv_A(*(heap), cur_idx); \ + typeof(elt) min_child = &kv_A(*(heap), left_idx); \ + if (right_idx < size) { \ + typeof(elt) right_child = &kv_A(*(heap), right_idx); \ + if (right_child->pri < min_child->pri) { \ + min_idx = right_idx; \ + min_child = right_child; \ + } \ + } \ + if (cur->pri > min_child->pri) { \ + /* Swap elements directly */ \ + typeof(*elt) tmp = *min_child; \ + *min_child = *cur; \ + *cur = tmp; \ + /* Update indices */ \ + cur->idx = cur_idx; \ + min_child->idx = min_idx; \ + /* Move down */ \ + cur_idx = min_idx; \ + } \ + else { \ + break; \ + } \ + } \ + } while (0) /** - * Removes element from the heap - * @param heap - * @param elt + * Push element to heap (safe version with error handling) + * Element is copied into the heap array. */ -void rspamd_min_heap_remove_elt(struct rspamd_min_heap *heap, - struct rspamd_min_heap_elt *elt); +#define rspamd_heap_push_safe(name, heap, elt, error_label) \ + do { \ + kv_push_safe(typeof(*(elt)), *(heap), *(elt), error_label); \ + kv_A(*(heap), kv_size(*(heap)) - 1).idx = kv_size(*(heap)) - 1; \ + rspamd_heap_swim(name, heap, &kv_A(*(heap), kv_size(*(heap)) - 1)); \ + } while (0) /** - * Destroys heap (elements are not destroyed themselves) - * @param heap + * Pop minimum element from heap + * Returns pointer to last element in the array (which now holds the popped value) + * Valid until next heap modification. */ -void rspamd_min_heap_destroy(struct rspamd_min_heap *heap); +#define rspamd_heap_pop(name, heap) \ + ({ \ + typeof(&kv_A(*(heap), 0)) result = NULL; \ + if (kv_size(*(heap)) > 0) { \ + if (kv_size(*(heap)) > 1) { \ + /* Swap min to end, then sink the new root */ \ + typeof(kv_A(*(heap), 0)) tmp = kv_A(*(heap), 0); \ + kv_A(*(heap), 0) = kv_A(*(heap), kv_size(*(heap)) - 1); \ + kv_A(*(heap), kv_size(*(heap)) - 1) = tmp; \ + kv_size(*(heap))--; \ + kv_A(*(heap), 0).idx = 0; \ + rspamd_heap_sink(name, heap, &kv_A(*(heap), 0)); \ + /* Return pointer to element that was moved to end */ \ + result = &kv_A(*(heap), kv_size(*(heap))); \ + } \ + else { \ + /* Single element - return it and decrement */ \ + result = &kv_A(*(heap), 0); \ + kv_size(*(heap))--; \ + } \ + } \ + result; \ + }) /** - * Returns element from the heap with the specified index - * @param heap - * @param idx - * @return + * Update element priority (element must be in heap) + * Pass pointer to element obtained from rspamd_heap_index() */ -struct rspamd_min_heap_elt *rspamd_min_heap_index(struct rspamd_min_heap *heap, - unsigned int idx); +#define rspamd_heap_update(name, heap, elt, new_pri) \ + do { \ + unsigned int old_pri = (elt)->pri; \ + (elt)->pri = (new_pri); \ + if ((new_pri) < old_pri) { \ + rspamd_heap_swim(name, heap, elt); \ + } \ + else if ((new_pri) > old_pri) { \ + rspamd_heap_sink(name, heap, elt); \ + } \ + } while (0) /** - * Returns the number of elements in the heap - * @param heap - * @return number of elements + * Remove element from heap (element must be in heap) + * Pass pointer to element obtained from rspamd_heap_index() */ -gsize rspamd_min_heap_size(struct rspamd_min_heap *heap); +#define rspamd_heap_remove(name, heap, elt) \ + do { \ + if ((elt)->idx < kv_size(*(heap))) { \ + if ((elt)->idx < kv_size(*(heap)) - 1) { \ + kv_A(*(heap), (elt)->idx) = kv_A(*(heap), kv_size(*(heap)) - 1); \ + kv_size(*(heap))--; \ + kv_A(*(heap), (elt)->idx).idx = (elt)->idx; \ + typeof(elt) moved = &kv_A(*(heap), (elt)->idx); \ + /* Need to restore heap property */ \ + if (moved->pri < (elt)->pri) { \ + rspamd_heap_swim(name, heap, moved); \ + } \ + else { \ + rspamd_heap_sink(name, heap, moved); \ + } \ + } \ + else { \ + kv_size(*(heap))--; \ + } \ + } \ + } while (0) #ifdef __cplusplus } diff --git a/src/libutil/mem_pool.c b/src/libutil/mem_pool.c index ccdaf558df..d806a34e23 100644 --- a/src/libutil/mem_pool.c +++ b/src/libutil/mem_pool.c @@ -23,7 +23,6 @@ #include "cryptobox.h" #include "contrib/uthash/utlist.h" #include "mem_pool_internal.h" -#include "heap.h" #ifdef WITH_JEMALLOC #include @@ -668,7 +667,6 @@ void rspamd_mempool_add_destructor_full(rspamd_mempool_t *pool, unsigned int priority) { struct _pool_destructors *dtor; - struct _pool_destructor_heap_elt *heap_wrapper; POOL_MTX_LOCK(); @@ -679,30 +677,28 @@ void rspamd_mempool_add_destructor_full(rspamd_mempool_t *pool, dtor->data = data; dtor->function = function; dtor->loc = line; - dtor->priority = priority; + dtor->pri = priority; /* Initialize heap if not yet created */ - if (pool->priv->dtors_heap == NULL) { - gsize reserved_size = 8; + if (pool->priv->dtors_heap.a == NULL) { + rspamd_heap_init(rspamd_mempool_destruct_heap, &pool->priv->dtors_heap); - /* Use entry point statistics if available */ + /* Reserve space based on statistics */ if (pool->priv->entry && pool->priv->entry->cur_dtors > 0) { - reserved_size = pool->priv->entry->cur_dtors; + kv_resize_safe(struct _pool_destructors, pool->priv->dtors_heap, + pool->priv->entry->cur_dtors, cleanup); } - - 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; + /* Push to heap - using non-safe version as we're in mempool */ + rspamd_heap_push_safe(rspamd_mempool_destruct_heap, &pool->priv->dtors_heap, dtor, cleanup); - rspamd_min_heap_push(pool->priv->dtors_heap, &heap_wrapper->heap_elt); + POOL_MTX_UNLOCK(); + return; +cleanup: POOL_MTX_UNLOCK(); + /* Allocation failed, but we can't really handle it in mempool context */ } void rspamd_mempool_replace_destructor(rspamd_mempool_t *pool, @@ -710,21 +706,17 @@ void rspamd_mempool_replace_destructor(rspamd_mempool_t *pool, void *old_data, void *new_data) { - 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) { + if (pool->priv->dtors_heap.a == NULL) { return; } /* Linear search through heap to find matching destructor */ - heap_size = rspamd_min_heap_size(pool->priv->dtors_heap); + heap_size = rspamd_heap_size(rspamd_mempool_destruct_heap, &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; + dtor = rspamd_heap_index(rspamd_mempool_destruct_heap, &pool->priv->dtors_heap, i); if (dtor->func == func && dtor->data == old_data) { dtor->data = new_data; @@ -828,18 +820,13 @@ rspamd_mempool_variables_cleanup(rspamd_mempool_t *pool) void rspamd_mempool_destructors_enforce(rspamd_mempool_t *pool) { - struct _pool_destructor_heap_elt *heap_wrapper; struct _pool_destructors *dtor; - struct rspamd_min_heap_elt *elt; POOL_MTX_LOCK(); - if (pool->priv->dtors_heap) { + if (pool->priv->dtors_heap.a != NULL) { /* 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; - + while ((dtor = rspamd_heap_pop(rspamd_mempool_destruct_heap, &pool->priv->dtors_heap)) != NULL) { /* Avoid calling destructors for NULL pointers */ if (dtor->data != NULL) { dtor->func(dtor->data); @@ -847,8 +834,7 @@ void rspamd_mempool_destructors_enforce(rspamd_mempool_t *pool) } /* Destroy the heap itself */ - rspamd_min_heap_destroy(pool->priv->dtors_heap); - pool->priv->dtors_heap = NULL; + rspamd_heap_destroy(rspamd_mempool_destruct_heap, &pool->priv->dtors_heap); } rspamd_mempool_variables_cleanup(pool); @@ -874,7 +860,6 @@ void rspamd_mempool_delete(rspamd_mempool_t *pool) { struct _pool_chain *cur, *tmp; struct _pool_destructors *dtor; - struct rspamd_min_heap_elt *elt; gpointer ptr; unsigned int i; gsize len; @@ -888,8 +873,8 @@ void rspamd_mempool_delete(rspamd_mempool_t *pool) /* Show debug info */ gsize ndtor = 0; - if (pool->priv->dtors_heap) { - ndtor = rspamd_min_heap_size(pool->priv->dtors_heap); + if (pool->priv->dtors_heap.a != NULL) { + ndtor = rspamd_heap_size(rspamd_mempool_destruct_heap, &pool->priv->dtors_heap); } msg_info_pool("destructing of the memory pool %p; elt size = %z; " @@ -943,13 +928,12 @@ void rspamd_mempool_delete(rspamd_mempool_t *pool) } /* Call all pool destructors in priority order */ - if (pool->priv->dtors_heap) { - struct _pool_destructor_heap_elt *heap_wrapper; + if (pool->priv->dtors_heap.a != NULL) { gsize ndtors = 0; /* Update destructor statistics before destroying */ if (pool->priv->entry && mempool_entries) { - ndtors = rspamd_min_heap_size(pool->priv->dtors_heap); + ndtors = rspamd_heap_size(rspamd_mempool_destruct_heap, &pool->priv->dtors_heap); /* Update suggestion using similar logic to variables */ if (pool->priv->entry->cur_dtors < ndtors) { @@ -975,18 +959,14 @@ void rspamd_mempool_delete(rspamd_mempool_t *pool) } } - while ((elt = rspamd_min_heap_pop(pool->priv->dtors_heap)) != NULL) { - heap_wrapper = (struct _pool_destructor_heap_elt *) elt->data; - dtor = heap_wrapper->dtor; - + while ((dtor = rspamd_heap_pop(rspamd_mempool_destruct_heap, &pool->priv->dtors_heap)) != NULL) { /* 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_heap_destroy(rspamd_mempool_destruct_heap, &pool->priv->dtors_heap); } rspamd_mempool_variables_cleanup(pool); diff --git a/src/libutil/mem_pool_internal.h b/src/libutil/mem_pool_internal.h index 26193ffbf0..7d6c9889de 100644 --- a/src/libutil/mem_pool_internal.h +++ b/src/libutil/mem_pool_internal.h @@ -50,23 +50,19 @@ struct rspamd_mempool_entry_point { }; /** - * Destructors heap item structure + * Destructors heap item structure (intrusive) */ struct _pool_destructors { 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 */ - unsigned int priority; /**< destructor priority (0 = default) */ + unsigned int pri; /**< destructor priority (0 = default) - used by heap */ + unsigned int idx; /**< heap index (managed by heap) */ }; -/** - * Wrapper for destructor with heap element - */ -struct _pool_destructor_heap_elt { - struct rspamd_min_heap_elt heap_elt; - struct _pool_destructors *dtor; -}; +/* Declare heap type for destructors */ +RSPAMD_HEAP_DECLARE(rspamd_mempool_destruct_heap, struct _pool_destructors); struct rspamd_mempool_variable { gpointer data; @@ -79,7 +75,7 @@ KHASH_INIT(rspamd_mempool_vars_hash, struct rspamd_mempool_specific { struct _pool_chain *pools[RSPAMD_MEMPOOL_MAX]; - struct rspamd_min_heap *dtors_heap; + rspamd_mempool_destruct_heap_t dtors_heap; GPtrArray *trash_stack; khash_t(rspamd_mempool_vars_hash) * variables; struct rspamd_mempool_entry_point *entry; diff --git a/test/rspamd_heap_test.c b/test/rspamd_heap_test.c index 711ad6aca8..4a28e8db1d 100644 --- a/test/rspamd_heap_test.c +++ b/test/rspamd_heap_test.c @@ -22,13 +22,21 @@ static const unsigned int niter = 100500; static const unsigned int nrem = 100; -static inline struct rspamd_min_heap_elt * +struct test_heap_elt { + unsigned int pri; + unsigned int idx; + gpointer data; +}; + +RSPAMD_HEAP_DECLARE(test_heap, struct test_heap_elt); + +static inline struct test_heap_elt new_elt(unsigned int pri) { - struct rspamd_min_heap_elt *elt; - - elt = g_slice_alloc0(sizeof(*elt)); - elt->pri = pri; + struct test_heap_elt elt = { + .pri = pri, + .idx = 0, + .data = NULL}; return elt; } @@ -36,12 +44,13 @@ new_elt(unsigned int pri) static double heap_nelts_test(unsigned int nelts) { - struct rspamd_min_heap *heap; - struct rspamd_min_heap_elt *elts; + test_heap_t heap; + struct test_heap_elt *elts; double t1, t2; unsigned int i; - heap = rspamd_min_heap_create(nelts); + rspamd_heap_init(test_heap, &heap); + /* Preallocate all elts */ elts = g_slice_alloc(sizeof(*elts) * nelts); @@ -52,131 +61,157 @@ heap_nelts_test(unsigned int nelts) t1 = rspamd_get_virtual_ticks(); for (i = 0; i < nelts; i++) { - rspamd_min_heap_push(heap, &elts[i]); + rspamd_heap_push_safe(test_heap, &heap, &elts[i], cleanup); } for (i = 0; i < nelts; i++) { - (void) rspamd_min_heap_pop(heap); + (void) rspamd_heap_pop(test_heap, &heap); } t2 = rspamd_get_virtual_ticks(); g_slice_free1(sizeof(*elts) * nelts, elts); - rspamd_min_heap_destroy(heap); + rspamd_heap_destroy(test_heap, &heap); return (t2 - t1); + +cleanup: + g_slice_free1(sizeof(*elts) * nelts, elts); + rspamd_heap_destroy(test_heap, &heap); + return 0; } void rspamd_heap_test_func(void) { - struct rspamd_min_heap *heap; - struct rspamd_min_heap_elt *elt, *telt; + test_heap_t heap; + struct test_heap_elt elt1, elt2, elt3; + struct test_heap_elt *elt; unsigned int i; unsigned int prev; double t[16]; /* Push + update */ - heap = rspamd_min_heap_create(32); - elt = new_elt(2); - elt->data = GINT_TO_POINTER(1); - rspamd_min_heap_push(heap, elt); - elt = new_elt(3); - elt->data = GINT_TO_POINTER(2); - rspamd_min_heap_push(heap, elt); - elt = new_elt(4); - elt->data = GINT_TO_POINTER(3); - rspamd_min_heap_push(heap, elt); - - rspamd_min_heap_update_elt(heap, elt, 0); - elt = rspamd_min_heap_pop(heap); + rspamd_heap_init(test_heap, &heap); + + elt1 = new_elt(2); + elt1.data = GINT_TO_POINTER(1); + rspamd_heap_push_safe(test_heap, &heap, &elt1, fail); + + elt2 = new_elt(3); + elt2.data = GINT_TO_POINTER(2); + rspamd_heap_push_safe(test_heap, &heap, &elt2, fail); + + elt3 = new_elt(4); + elt3.data = GINT_TO_POINTER(3); + rspamd_heap_push_safe(test_heap, &heap, &elt3, fail); + + /* Find elt3 in heap (it should be at some index with data==3) */ + struct test_heap_elt *elt_to_update = NULL; + for (i = 0; i < rspamd_heap_size(test_heap, &heap); i++) { + elt = rspamd_heap_index(test_heap, &heap, i); + if (elt->data == GINT_TO_POINTER(3)) { + elt_to_update = elt; + break; + } + } + g_assert(elt_to_update != NULL); + rspamd_heap_update(test_heap, &heap, elt_to_update, 0); + elt = rspamd_heap_pop(test_heap, &heap); g_assert(elt->data == GINT_TO_POINTER(3)); - rspamd_min_heap_destroy(heap); + rspamd_heap_destroy(test_heap, &heap); /* Push + remove */ - heap = rspamd_min_heap_create(32); - elt = new_elt(2); - elt->data = GINT_TO_POINTER(1); - rspamd_min_heap_push(heap, elt); - rspamd_min_heap_remove_elt(heap, elt); - elt = new_elt(3); - elt->data = GINT_TO_POINTER(2); - rspamd_min_heap_push(heap, elt); - elt = rspamd_min_heap_pop(heap); - g_assert(elt->data == GINT_TO_POINTER(2)); - elt = rspamd_min_heap_pop(heap); + rspamd_heap_init(test_heap, &heap); + + elt1 = new_elt(2); + elt1.data = GINT_TO_POINTER(1); + rspamd_heap_push_safe(test_heap, &heap, &elt1, fail); + + /* Find and remove the element */ + for (i = 0; i < rspamd_heap_size(test_heap, &heap); i++) { + elt = rspamd_heap_index(test_heap, &heap, i); + if (elt->data == GINT_TO_POINTER(1)) { + rspamd_heap_remove(test_heap, &heap, elt); + break; + } + } + elt = rspamd_heap_pop(test_heap, &heap); g_assert(elt == NULL); - /* Push + push + remove + pop */ - elt = new_elt(2); - elt->data = GINT_TO_POINTER(1); - rspamd_min_heap_push(heap, elt); - telt = elt; - elt = new_elt(3); - elt->data = GINT_TO_POINTER(2); - rspamd_min_heap_push(heap, elt); - rspamd_min_heap_remove_elt(heap, telt); - elt = rspamd_min_heap_pop(heap); - g_assert(elt->data == GINT_TO_POINTER(2)); - rspamd_min_heap_destroy(heap); - - /* Bulk test */ - heap = rspamd_min_heap_create(32); - - for (i = 100; i > 0; i--) { - elt = new_elt(i - 1); - rspamd_min_heap_push(heap, elt); + rspamd_heap_destroy(test_heap, &heap); + + /* Push + pop */ + rspamd_heap_init(test_heap, &heap); + + for (i = 0; i < niter; i++) { + struct test_heap_elt tmp = new_elt(ottery_rand_uint32() % G_MAXINT32 + 1); + tmp.data = GINT_TO_POINTER(i); + rspamd_heap_push_safe(test_heap, &heap, &tmp, fail); } - for (i = 0; i < 100; i++) { - elt = rspamd_min_heap_pop(heap); - g_assert(elt->pri == i); + prev = 0; + for (i = 0; i < niter; i++) { + elt = rspamd_heap_pop(test_heap, &heap); + g_assert(elt != NULL); + + if (prev != 0) { + g_assert(prev <= elt->pri); + } + + prev = elt->pri; } - rspamd_min_heap_destroy(heap); + elt = rspamd_heap_pop(test_heap, &heap); + g_assert(elt == NULL); + + rspamd_heap_destroy(test_heap, &heap); - /* Fuzz test */ - heap = rspamd_min_heap_create(128); + /* Push + pop + push */ + rspamd_heap_init(test_heap, &heap); - /* Add */ for (i = 0; i < niter; i++) { - elt = new_elt(ottery_rand_uint32() % G_MAXINT32 + 1); - rspamd_min_heap_push(heap, elt); + struct test_heap_elt tmp = new_elt(ottery_rand_uint32() % G_MAXINT32 + 1); + rspamd_heap_push_safe(test_heap, &heap, &tmp, fail); } - /* Remove */ for (i = 0; i < nrem; i++) { - elt = rspamd_min_heap_index(heap, ottery_rand_uint32() % niter); - rspamd_min_heap_remove_elt(heap, elt); + (void) rspamd_heap_pop(test_heap, &heap); } - /* Update */ - for (i = 0; i < niter / 10; i++) { - elt = rspamd_min_heap_index(heap, ottery_rand_uint32() % (niter - nrem)); - rspamd_min_heap_update_elt(heap, elt, - ottery_rand_uint32() % G_MAXINT32 + 1); + for (i = 0; i < nrem; i++) { + struct test_heap_elt tmp = new_elt(ottery_rand_uint32() % G_MAXINT32 + 1); + rspamd_heap_push_safe(test_heap, &heap, &tmp, fail); } prev = 0; - - /* Pop and check invariant */ - for (i = 0; i < niter - nrem; i++) { - elt = rspamd_min_heap_pop(heap); + for (i = 0; i < niter; i++) { + elt = rspamd_heap_pop(test_heap, &heap); + g_assert(elt != NULL); if (prev != 0) { - g_assert(elt->pri >= prev); + g_assert(prev <= elt->pri); } prev = elt->pri; } - rspamd_min_heap_destroy(heap); + elt = rspamd_heap_pop(test_heap, &heap); + g_assert(elt == NULL); + + rspamd_heap_destroy(test_heap, &heap); - /* Complexity test (should be O(n * logn) */ - for (i = 1; i <= G_N_ELEMENTS(t); i++) { - t[i - 1] = heap_nelts_test(0x1 << (i + 4)); + for (i = 0; i < G_N_ELEMENTS(t); i++) { + t[i] = heap_nelts_test(1 << (i + 4)); } - for (i = 1; i <= G_N_ELEMENTS(t); i++) { - rspamd_printf("Elements: %d, time: %.4f\n", 0x1 << (i + 4), t[i - 1]); + msg_info("heap push/pop performance (%d elts)", 1 << 4); + + for (i = 0; i < G_N_ELEMENTS(t); i++) { + msg_info("%d elts: %.4f", 1 << (i + 4), t[i]); } + + return; + +fail: + g_assert_not_reached(); }