daemon/worker: drop caching of kr_request mempools

This caused a huge increase in real memory usage in case of queries
arriving to kresd while being disconnected from internet.
The usage was slowly creeping up, even over 2G.

Interesting past commits: b350d38d and two preceding.

There apparently was no real memory leak.  I assume that reusal of
long-living mempools is risky in terms of memory fragmentation,
though the extent of the issue surprised me very much.
The issue seemed the same with normal glibc and jemalloc.

I generally dislike ad-hoc optimization attempts like these freelists.
Now the allocator can better decide *itself* how to reuse memory.

diff --git a/NEWS b/NEWS
index c963d074db606887750f8d755c8fd14d4ea94e98..a1813b18b11004ffb1a5d3f967de84e5ad5e6195 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,7 @@ Improvements
 ------------
 - libknot 3.2 is supported (!1309)
 - priming module: hide failures from the default log level (!1310)
+- less memory usage in some cases (!1328)
 
 Bugfixes
 --------

diff --git a/daemon/worker.c b/daemon/worker.c
index 61f2d8434cec8f8f5dc1cf52f91bae66ed988f45..a50be16fac4ac37e7ab5f16844e214d436b5c981 100644 (file)
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -37,13 +37,6 @@
 
 
 /* Magic defaults for the worker. */
-#ifndef MP_FREELIST_SIZE
-# ifdef __clang_analyzer__
-#  define MP_FREELIST_SIZE 0
-# else
-#  define MP_FREELIST_SIZE 64 /**< Maximum length of the worker mempool freelist */
-# endif
-#endif
 #ifndef MAX_PIPELINED
 #define MAX_PIPELINED 100
 #endif
@@ -197,54 +190,17 @@ static void ioreq_kill_pending(struct qr_task *task)
        task->pending_count = 0;
 }
 
-/** @cond This memory layout is internal to mempool.c, use only for debugging. */
-#if defined(__SANITIZE_ADDRESS__)
-struct mempool_chunk {
-  struct mempool_chunk *next;
-  size_t size;
-};
-static void mp_poison(struct mempool *mp, bool poison)
-{
-       if (!poison) { /* @note mempool is part of the first chunk, unpoison it first */
-               kr_asan_unpoison(mp, sizeof(*mp));
-       }
-       struct mempool_chunk *chunk = mp->state.last[0];
-       void *chunk_off = (uint8_t *)chunk - chunk->size;
-       if (poison) {
-               kr_asan_poison(chunk_off, chunk->size);
-       } else {
-               kr_asan_unpoison(chunk_off, chunk->size);
-       }
-}
-#else
-#define mp_poison(mp, enable)
-#endif
-/** @endcond */
-
-/** Get a mempool.  (Recycle if possible.)  */
+/** Get a mempool. */
 static inline struct mempool *pool_borrow(struct worker_ctx *worker)
 {
-       struct mempool *mp = NULL;
-       if (worker->pool_mp.len > 0) {
-               mp = array_tail(worker->pool_mp);
-               array_pop(worker->pool_mp);
-               mp_poison(mp, 0);
-       } else { /* No mempool on the freelist, create new one */
-               mp = mp_new (4 * CPU_PAGE_SIZE);
-       }
-       return mp;
+       /* The implementation used to have extra caching layer,
+        * but it didn't work well.  Now it's very simple. */
+       return mp_new(16 * 1024);
 }
-
-/** Return a mempool.  (Cache them up to some count.) */
+/** Return a mempool. */
 static inline void pool_release(struct worker_ctx *worker, struct mempool *mp)
 {
-       if (worker->pool_mp.len < MP_FREELIST_SIZE) {
-               mp_flush(mp);
-               array_push(worker->pool_mp, mp);
-               mp_poison(mp, 1);
-       } else {
-               mp_delete(mp);
-       }
+       mp_delete(mp);
 }
 
 /** Create a key for an outgoing subrequest: qname, qclass, qtype.
@@ -2177,32 +2133,17 @@ bool worker_task_finished(struct qr_task *task)
 }
 
 /** Reserve worker buffers.  We assume worker's been zeroed. */
-static int worker_reserve(struct worker_ctx *worker, size_t ring_maxlen)
+static int worker_reserve(struct worker_ctx *worker)
 {
        worker->tcp_connected = trie_create(NULL);
        worker->tcp_waiting = trie_create(NULL);
        worker->subreq_out = trie_create(NULL);
 
-       array_init(worker->pool_mp);
-       if (array_reserve(worker->pool_mp, ring_maxlen)) {
-               return kr_error(ENOMEM);
-       }
-
        mm_ctx_mempool(&worker->pkt_pool, 4 * sizeof(knot_pkt_t));
 
        return kr_ok();
 }
 
-static inline void reclaim_mp_freelist(mp_freelist_t *list)
-{
-       for (unsigned i = 0; i < list->len; ++i) {
-               struct mempool *e = list->at[i];
-               kr_asan_unpoison(e, sizeof(*e));
-               mp_delete(e);
-       }
-       array_clear(*list);
-}
-
 void worker_deinit(void)
 {
        struct worker_ctx *worker = the_worker;
@@ -2217,7 +2158,6 @@ void worker_deinit(void)
                free((void *)worker->doh_qry_headers.at[i]);
        array_clear(worker->doh_qry_headers);
 
-       reclaim_mp_freelist(&worker->pool_mp);
        mp_delete(worker->pkt_pool.ctx);
        worker->pkt_pool.ctx = NULL;
 
@@ -2253,7 +2193,7 @@ int worker_init(struct engine *engine, int worker_count)
 
        array_init(worker->doh_qry_headers);
 
-       int ret = worker_reserve(worker, MP_FREELIST_SIZE);
+       int ret = worker_reserve(worker);
        if (ret) return ret;
        worker->next_request_uid = UINT16_MAX + 1;
 

diff --git a/daemon/worker.h b/daemon/worker.h
index 3314cebcd66973125bda5766a579f9eb5ae8d65d..2eaf0907c122d4b031feea5bc31d0659ef58bee0 100644 (file)
--- a/daemon/worker.h
+++ b/daemon/worker.h
@@ -152,9 +152,6 @@ struct worker_stats {
 #define RECVMMSG_BATCH 1
 #endif
 
-/** Freelist of available mempools. */
-typedef array_t(struct mempool *) mp_freelist_t;
-
 /** List of query resolution tasks. */
 typedef array_t(struct qr_task *) qr_tasklist_t;
 
@@ -185,7 +182,6 @@ struct worker_ctx {
        trie_t *tcp_waiting;
        /** Subrequest leaders (struct qr_task*), indexed by qname+qtype+qclass. */
        trie_t *subreq_out;
-       mp_freelist_t pool_mp;
        knot_mm_t pkt_pool;
        unsigned int next_request_uid;