Remove the fillcount from the isc_mempool API

Previously, the fillcount could be used to indicate how many elements
would be preallocated every time the memory would be empty.  This would
result in bursty allocations when the mempool would be drained.

For more smooth performance, we allocate the new pool items only as
needed.  In the future, we could consider changing the
isc_mempool_create() function to take an initial number of pre-allocated
items on the pool, so the bursty allocation happens only on the pool
creation.

diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c
index acd7225e9e9998b0e2cf1855338ec4a83093ad5d..22f5097175d0146573b195b68a287d71a633dfde 100644 (file)
--- a/bin/dig/dighost.c
+++ b/bin/dig/dighost.c
@@ -1433,7 +1433,6 @@ setup_libs(void) {
         * systems.
         */
        isc_mempool_setfreemax(commctx, 6);
-       isc_mempool_setfillcount(commctx, 2);
 
        isc_mutex_init(&lookup_lock);
 }

diff --git a/bin/plugins/filter-aaaa.c b/bin/plugins/filter-aaaa.c
index 31e6e7cfd49e24e41fe40b32e92d103d97d85e11..3b3e7f626e1d27f6ae084b310d9bab41d8d62e71 100644 (file)
--- a/bin/plugins/filter-aaaa.c
+++ b/bin/plugins/filter-aaaa.c
@@ -361,16 +361,10 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file,
        isc_mutex_init(&inst->hlock);
 
        /*
-        * Fill the mempool with 1K filter_aaaa state objects at
-        * a time; ideally after a single allocation, the mempool will
-        * have enough to handle all the simultaneous queries the system
-        * requires and it won't be necessary to allocate more.
-        *
         * We don't set any limit on the number of free state objects
         * so that they'll always be returned to the pool and not
         * freed until the pool is destroyed on shutdown.
         */
-       isc_mempool_setfillcount(inst->datapool, 1024);
        isc_mempool_setfreemax(inst->datapool, UINT_MAX);
        isc_mutex_init(&inst->plock);
        isc_mempool_associatelock(inst->datapool, &inst->plock);

diff --git a/lib/dns/adb.c b/lib/dns/adb.c
index 3665bf0ea14c49dde75a7128d34ee313f41086b5..ab8d293f70e853aebc8ebf893a04047ef16c1733 100644 (file)
--- a/lib/dns/adb.c
+++ b/lib/dns/adb.c
@@ -2716,7 +2716,6 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr,
        do {                                                  \
                isc_mempool_create(mem, sizeof(t), &(p));     \
                isc_mempool_setfreemax((p), FREE_ITEMS);      \
-               isc_mempool_setfillcount((p), FILL_COUNT);    \
                isc_mempool_setname((p), n);                  \
                isc_mempool_associatelock((p), &adb->mplock); \
        } while (0)

diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c
index ada4cce97779de592a2945af487b95b9fc04b0ab..b82580d9fa6ca3003181c5f84458a7fc1b532c7c 100644 (file)
--- a/lib/dns/dispatch.c
+++ b/lib/dns/dispatch.c
@@ -1810,17 +1810,14 @@ dns_dispatchmgr_create(isc_mem_t *mctx, dns_dispatchmgr_t **mgrp) {
        isc_mempool_setname(mgr->depool, "dispmgr_depool");
        isc_mempool_setfreemax(mgr->depool, 32768);
        isc_mempool_associatelock(mgr->depool, &mgr->depool_lock);
-       isc_mempool_setfillcount(mgr->depool, 32);
 
        isc_mempool_setname(mgr->rpool, "dispmgr_rpool");
        isc_mempool_setfreemax(mgr->rpool, 32768);
        isc_mempool_associatelock(mgr->rpool, &mgr->rpool_lock);
-       isc_mempool_setfillcount(mgr->rpool, 32);
 
        isc_mempool_setname(mgr->dpool, "dispmgr_dpool");
        isc_mempool_setfreemax(mgr->dpool, 32768);
        isc_mempool_associatelock(mgr->dpool, &mgr->dpool_lock);
-       isc_mempool_setfillcount(mgr->dpool, 32);
 
        mgr->buffers = 0;
        mgr->buffersize = 0;
@@ -1992,7 +1989,6 @@ dns_dispatchmgr_setudp(dns_dispatchmgr_t *mgr, unsigned int buffersize,
                isc_mempool_setname(mgr->bpool, "dispmgr_bpool");
                isc_mempool_setfreemax(mgr->bpool, maxbuffers);
                isc_mempool_associatelock(mgr->bpool, &mgr->bpool_lock);
-               isc_mempool_setfillcount(mgr->bpool, 32);
        }
 
        /* Create or adjust socket pool */
@@ -2009,7 +2005,6 @@ dns_dispatchmgr_setudp(dns_dispatchmgr_t *mgr, unsigned int buffersize,
        isc_mempool_setname(mgr->spool, "dispmgr_spool");
        isc_mempool_setfreemax(mgr->spool, maxrequests);
        isc_mempool_associatelock(mgr->spool, &mgr->spool_lock);
-       isc_mempool_setfillcount(mgr->spool, 32);
 
        result = qid_allocate(mgr, buckets, increment, &mgr->qid, true);
        if (result != ISC_R_SUCCESS) {
@@ -2908,7 +2903,6 @@ dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr,
        isc_mempool_setname(disp->sepool, "disp_sepool");
        isc_mempool_setfreemax(disp->sepool, 32768);
        isc_mempool_associatelock(disp->sepool, &disp->sepool_lock);
-       isc_mempool_setfillcount(disp->sepool, 16);
 
        attributes &= ~DNS_DISPATCHATTR_TCP;
        attributes |= DNS_DISPATCHATTR_UDP;

diff --git a/lib/dns/message.c b/lib/dns/message.c
index 425d8d05ef100ed27bbfd6d8bf0bacb7cf4937c4..1d3b006acd738ee2e1c8e82d9e616064fcddc4a2 100644 (file)
--- a/lib/dns/message.c
+++ b/lib/dns/message.c
@@ -750,12 +750,10 @@ dns_message_create(isc_mem_t *mctx, unsigned int intent, dns_message_t **msgp) {
        ISC_LIST_INIT(m->freerdatalist);
 
        isc_mempool_create(m->mctx, sizeof(dns_name_t), &m->namepool);
-       isc_mempool_setfillcount(m->namepool, NAME_COUNT);
        isc_mempool_setfreemax(m->namepool, NAME_COUNT);
        isc_mempool_setname(m->namepool, "msg:names");
 
        isc_mempool_create(m->mctx, sizeof(dns_rdataset_t), &m->rdspool);
-       isc_mempool_setfillcount(m->rdspool, RDATASET_COUNT);
        isc_mempool_setfreemax(m->rdspool, RDATASET_COUNT);
        isc_mempool_setname(m->rdspool, "msg:rdataset");
 

diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h
index 73c12fe60f21c9d3a4d24d712826504dd139da54..48d0dbda803b1cce89c0390e45a4f8d16b08a594 100644 (file)
--- a/lib/isc/include/isc/mem.h
+++ b/lib/isc/include/isc/mem.h
@@ -383,7 +383,6 @@ isc_mempool_create(isc_mem_t *mctx, size_t size, isc_mempool_t **mpctxp);
  *
  * Defaults:
  *\li  freemax = 1
- *\li  fillcount = 1
  *
  * Returns:
  *\li  #ISC_R_NOMEMORY         -- not enough memory to create pool
@@ -474,22 +473,6 @@ isc_mempool_getallocated(isc_mempool_t *mpctx);
  * Returns the number of items allocated from this pool.
  */
 
-unsigned int
-isc_mempool_getfillcount(isc_mempool_t *mpctx);
-/*%<
- * Returns the number of items allocated as a block from the parent
- * memory context when the free list is empty.
- */
-
-void
-isc_mempool_setfillcount(isc_mempool_t *mpctx, unsigned int limit);
-/*%<
- * Sets the fillcount.
- *
- * Additional requirements:
- *\li  limit > 0
- */
-
 /*
  * Pseudo-private functions for use via macros.  Do not call directly.
  */

diff --git a/lib/isc/mem.c b/lib/isc/mem.c
index db839740ddf9c3d9808010190590b0f906c86418..b47b5578056578006579bb4e0f151f95a858e3d8 100644 (file)
--- a/lib/isc/mem.c
+++ b/lib/isc/mem.c
@@ -173,7 +173,6 @@ struct isc_mempool {
        atomic_size_t allocated; /*%< # of items currently given out */
        atomic_size_t freecount; /*%< # of items on reserved list */
        atomic_size_t freemax;   /*%< # of items allowed on free list */
-       atomic_size_t fillcount; /*%< # of items to fetch on each fill */
        /*%< Stats only. */
        atomic_size_t gets; /*%< # of requests to this pool */
                            /*%< Debugging only. */
@@ -875,17 +874,16 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) {
        pool = ISC_LIST_HEAD(ctx->pools);
        if (pool != NULL) {
                fprintf(out, "[Pool statistics]\n");
-               fprintf(out, "%15s %10s %10s %10s %10s %10s %10s %1s\n", "name",
+               fprintf(out, "%15s %10s %10s %10s %10s %10s %1s\n", "name",
                        "size", "allocated", "freecount", "freemax",
-                       "fillcount", "gets", "L");
+                       "gets", "L");
        }
        while (pool != NULL) {
-               fprintf(out, "%15s %10zu %10zu %10zu %10zu %10zu %10zu %s\n",
+               fprintf(out, "%15s %10zu %10zu %10zu %10zu %10zu %s\n",
                        pool->name, pool->size,
                        atomic_load_relaxed(&pool->allocated),
                        atomic_load_relaxed(&pool->freecount),
                        atomic_load_relaxed(&pool->freemax),
-                       atomic_load_relaxed(&pool->fillcount),
                        atomic_load_relaxed(&pool->gets),
                        (pool->lock == NULL ? "N" : "Y"));
                pool = ISC_LIST_NEXT(pool, link);
@@ -1206,7 +1204,6 @@ isc_mempool_create(isc_mem_t *mctx, size_t size, isc_mempool_t **mpctxp) {
        atomic_init(&mpctx->allocated, 0);
        atomic_init(&mpctx->freecount, 0);
        atomic_init(&mpctx->freemax, 1);
-       atomic_init(&mpctx->fillcount, 1);
 
        *mpctxp = (isc_mempool_t *)mpctx;
 
@@ -1330,27 +1327,13 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) {
        MPCTXLOCK(mpctx);
        if (ISC_UNLIKELY(mpctx->items == NULL)) {
                isc_mem_t *mctx = mpctx->mctx;
-               size_t fillcount = atomic_load_acquire(&mpctx->fillcount);
-               /*
-                * We need to dip into the well.  Lock the memory
-                * context here and fill up our free list.
-                */
-               for (i = 0; i < fillcount; i++) {
-                       item = mem_get(mctx, mpctx->size);
-                       mem_getstats(mctx, mpctx->size);
-                       item->next = mpctx->items;
-                       mpctx->items = item;
-                       atomic_fetch_add_relaxed(&mpctx->freecount, 1);
-               }
-       }
-
-       /*
-        * If we didn't get any items, return NULL.
-        */
-       item = mpctx->items;
-       if (ISC_UNLIKELY(item == NULL)) {
-               goto out;
+               item = mem_get(mctx, mpctx->size);
+               mem_getstats(mctx, mpctx->size);
+               item->next = NULL;
+       } else {
+               item = mpctx->items;
        }
+       REQUIRE(item != NULL);
 
        mpctx->items = item->next;
 
@@ -1438,21 +1421,6 @@ isc_mempool_getallocated(isc_mempool_t *mpctx) {
        return (atomic_load_relaxed(&mpctx->allocated));
 }
 
-void
-isc_mempool_setfillcount(isc_mempool_t *mpctx, unsigned int limit) {
-       REQUIRE(VALID_MEMPOOL(mpctx));
-       REQUIRE(limit > 0);
-
-       atomic_store_release(&mpctx->fillcount, limit);
-}
-
-unsigned int
-isc_mempool_getfillcount(isc_mempool_t *mpctx) {
-       REQUIRE(VALID_MEMPOOL(mpctx));
-
-       return (atomic_load_relaxed(&mpctx->fillcount));
-}
-
 /*
  * Requires contextslock to be held by caller.
  */

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index eb7a23e3a47d4e35955953ff39b91b4e5ea61b97..d5bb613866018135d28a7626bd0af672a77d6c74 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -244,7 +244,6 @@ isc_nm_start(isc_mem_t *mctx, uint32_t workers) {
        isc_mempool_setname(mgr->reqpool, "nm_reqpool");
        isc_mempool_setfreemax(mgr->reqpool, 4096);
        isc_mempool_associatelock(mgr->reqpool, &mgr->reqlock);
-       isc_mempool_setfillcount(mgr->reqpool, 32);
 
        isc_mutex_init(&mgr->evlock);
        isc_mempool_create(mgr->mctx, sizeof(isc__netievent_storage_t),
@@ -252,7 +251,6 @@ isc_nm_start(isc_mem_t *mctx, uint32_t workers) {
        isc_mempool_setname(mgr->evpool, "nm_evpool");
        isc_mempool_setfreemax(mgr->evpool, 4096);
        isc_mempool_associatelock(mgr->evpool, &mgr->evlock);
-       isc_mempool_setfillcount(mgr->evpool, 32);
 
        mgr->workers = isc_mem_get(mctx, workers * sizeof(isc__networker_t));
        for (size_t i = 0; i < workers; i++) {

diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c
index 857951c5bc925cfd6cec0be9116b473e51001218..9e974b7db0e5385877bef6952928c4e17166cab4 100644 (file)
--- a/lib/isc/tests/mem_test.c
+++ b/lib/isc/tests/mem_test.c
@@ -59,11 +59,9 @@ _teardown(void **state) {
 }
 
 #define MP1_FREEMAX  10
-#define MP1_FILLCNT  10
 #define MP1_MAXALLOC 30
 
 #define MP2_FREEMAX 25
-#define MP2_FILLCNT 25
 
 /* general memory system tests */
 static void
@@ -81,7 +79,6 @@ isc_mem_test(void **state) {
        isc_mempool_create(test_mctx, 31, &mp2);
 
        isc_mempool_setfreemax(mp1, MP1_FREEMAX);
-       isc_mempool_setfillcount(mp1, MP1_FILLCNT);
 
        /*
         * Allocate MP1_MAXALLOC items from the pool.  This is our max.
@@ -114,7 +111,6 @@ isc_mem_test(void **state) {
         */
 
        isc_mempool_setfreemax(mp2, 25);
-       isc_mempool_setfillcount(mp2, 25);
 
        for (j = 0; j < 500000; j++) {
                for (i = 0; i < 50; i++) {
@@ -457,7 +453,6 @@ isc_mempool_benchmark(void **state) {
        isc_mempool_associatelock(mp, &mplock);
 
        isc_mempool_setfreemax(mp, 32768);
-       isc_mempool_setfillcount(mp, ISC_MAX(NUM_ITEMS / nthreads, 1));
 
        UNUSED(state);
 

diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in
index f20ac37c7b8cc9eaf6641c27cf8f629167d0fdff..92c5f1e1790e7b2a26bdb894f28e50f617d121c2 100644 (file)
--- a/lib/isc/win32/libisc.def.in
+++ b/lib/isc/win32/libisc.def.in
@@ -394,10 +394,8 @@ isc_mempool_associatelock
 isc_mempool_create
 isc_mempool_destroy
 isc_mempool_getallocated
-isc_mempool_getfillcount
 isc_mempool_getfreecount
 isc_mempool_getfreemax
-isc_mempool_setfillcount
 isc_mempool_setfreemax
 isc_mempool_setname
 isc_mutexblock_destroy