From 7d8aa630269fded42be7332f4b36109b8d0d2ecf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 11:37:00 +0100 Subject: [PATCH 01/13] Make {increment,decrement}_malloced() return void The return value was only used in a single place and only for decrement_malloced() and we can easily replace that with atomic_load(). --- lib/isc/mem.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index fdb2a597d8..030a4b2444 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -211,7 +211,7 @@ static void print_active(isc_mem_t *ctx, FILE *out); #endif /* ISC_MEM_TRACKLINES */ -static size_t +static void increment_malloced(isc_mem_t *ctx, size_t size) { size_t malloced = atomic_fetch_add_relaxed(&ctx->malloced, size) + size; size_t maxmalloced = atomic_load_relaxed(&ctx->maxmalloced); @@ -220,15 +220,11 @@ increment_malloced(isc_mem_t *ctx, size_t size) { atomic_compare_exchange_strong(&ctx->maxmalloced, &maxmalloced, malloced); } - - return (malloced); } -static size_t +static void decrement_malloced(isc_mem_t *ctx, size_t size) { - size_t malloced = atomic_fetch_sub_relaxed(&ctx->malloced, size) - size; - - return (malloced); + (void)atomic_fetch_sub_relaxed(&ctx->malloced, size); } #if ISC_MEM_TRACKLINES @@ -534,7 +530,6 @@ mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags) { static void destroy(isc_mem_t *ctx) { unsigned int i; - size_t malloced; LOCK(&contextslock); ISC_LIST_UNLINK(contexts, ctx, link); @@ -590,10 +585,8 @@ destroy(isc_mem_t *ctx) { isc_mutex_destroy(&ctx->lock); - malloced = decrement_malloced(ctx, sizeof(*ctx)); - if (ctx->checkfree) { - INSIST(malloced == 0); + INSIST(atomic_load(&ctx->malloced) == 0); } sdallocx(ctx, sizeof(*ctx), ISC_MEM_ALIGN(isc_os_cacheline())); } From 971df0b4ed3956a79bb8559d0fc18f9df46ae1ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 11:41:34 +0100 Subject: [PATCH 02/13] Remove malloced and maxmalloced memory counter The malloced and maxmalloced memory counters were mostly useless since we removed the internal allocator blocks - it would only differ from inuse by the memory context size itself. --- bin/named/bind9.xsl | 8 --- bin/tests/system/statschannel/tests.sh | 10 ++-- lib/isc/include/isc/mem.h | 13 ----- lib/isc/mem.c | 77 +------------------------- 4 files changed, 5 insertions(+), 103 deletions(-) diff --git a/bin/named/bind9.xsl b/bin/named/bind9.xsl index 27161860d0..e9e4fa376c 100644 --- a/bin/named/bind9.xsl +++ b/bin/named/bind9.xsl @@ -1042,8 +1042,6 @@ TotalUse InUse MaxUse - Malloced - MaxMalloced BlockSize Pools HiWater @@ -1076,12 +1074,6 @@ - - - - - - diff --git a/bin/tests/system/statschannel/tests.sh b/bin/tests/system/statschannel/tests.sh index 7c871db27f..c7b643ad1e 100644 --- a/bin/tests/system/statschannel/tests.sh +++ b/bin/tests/system/statschannel/tests.sh @@ -119,16 +119,14 @@ if [ $PERL_XML ]; then file=`$PERL fetch.pl -p ${EXTRAPORT1} xml/v3/mem` mv $file xml.mem $PERL mem-xml.pl $file > xml.fmtmem - grep "'Malloced' => '[0-9][0-9]*'" xml.fmtmem > /dev/null || ret=1 - grep "'malloced' => '[0-9][0-9]*'" xml.fmtmem > /dev/null || ret=1 - grep "'maxmalloced' => '[0-9][0-9]*'" xml.fmtmem > /dev/null || ret=1 + grep "'InUse' => '[0-9][0-9]*'" xml.fmtmem > /dev/null || ret=1 + grep "'inuse' => '[0-9][0-9]*'" xml.fmtmem > /dev/null || ret=1 fi if [ $PERL_JSON ]; then file=`$PERL fetch.pl -p ${EXTRAPORT1} json/v1/mem` mv $file json.mem - grep '"malloced":[0-9][0-9]*,' json.mem > /dev/null || ret=1 - grep '"maxmalloced":[0-9][0-9]*,' json.mem > /dev/null || ret=1 - grep '"Malloced":[0-9][0-9]*,' json.mem > /dev/null || ret=1 + grep '"inuse":[0-9][0-9]*,' json.mem > /dev/null || ret=1 + grep '"InUse":[0-9][0-9]*,' json.mem > /dev/null || ret=1 fi if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 0249979eec..9c156a4d6d 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -285,19 +285,6 @@ isc_mem_total(isc_mem_t *mctx); * not yet used. */ -size_t -isc_mem_malloced(isc_mem_t *ctx); -/*%< - * Get an estimate of the amount of memory allocated in 'mctx', in bytes. - */ - -size_t -isc_mem_maxmalloced(isc_mem_t *ctx); -/*%< - * Get an estimate of the largest amount of memory that has been - * allocated in 'mctx' at any time. - */ - bool isc_mem_isovermem(isc_mem_t *mctx); /*%< diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 030a4b2444..dbed11eac8 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -144,8 +144,6 @@ struct isc_mem { atomic_size_t total; atomic_size_t inuse; atomic_size_t maxinuse; - atomic_size_t malloced; - atomic_size_t maxmalloced; atomic_bool hi_called; atomic_bool is_overmem; isc_mem_water_t water; @@ -211,22 +209,6 @@ static void print_active(isc_mem_t *ctx, FILE *out); #endif /* ISC_MEM_TRACKLINES */ -static void -increment_malloced(isc_mem_t *ctx, size_t size) { - size_t malloced = atomic_fetch_add_relaxed(&ctx->malloced, size) + size; - size_t maxmalloced = atomic_load_relaxed(&ctx->maxmalloced); - - if (malloced > maxmalloced) { - atomic_compare_exchange_strong(&ctx->maxmalloced, &maxmalloced, - malloced); - } -} - -static void -decrement_malloced(isc_mem_t *ctx, size_t size) { - (void)atomic_fetch_sub_relaxed(&ctx->malloced, size); -} - #if ISC_MEM_TRACKLINES /*! * mctx must not be locked. @@ -261,7 +243,6 @@ add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { dl = mallocx(sizeof(debuglink_t), 0); INSIST(dl != NULL); - increment_malloced(mctx, sizeof(debuglink_t)); ISC_LINK_INIT(dl, link); dl->ptr = ptr; @@ -308,7 +289,6 @@ delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size, while (dl != NULL) { if (dl->ptr == ptr) { ISC_LIST_UNLINK(mctx->debuglist[idx], dl, link); - decrement_malloced(mctx, sizeof(*dl)); sdallocx(dl, sizeof(*dl), 0); goto unlock; } @@ -404,8 +384,6 @@ mem_getstats(isc_mem_t *ctx, size_t size) { atomic_fetch_add_relaxed(&stats->gets, 1); atomic_fetch_add_relaxed(&stats->totalgets, 1); - - increment_malloced(ctx, size); } /*! @@ -423,8 +401,6 @@ mem_putstats(isc_mem_t *ctx, void *ptr, size_t size) { g = atomic_fetch_sub_release(&stats->gets, 1); INSIST(g >= 1); - - decrement_malloced(ctx, size); } /* @@ -487,8 +463,6 @@ mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags) { atomic_init(&ctx->total, 0); atomic_init(&ctx->inuse, 0); atomic_init(&ctx->maxinuse, 0); - atomic_init(&ctx->malloced, sizeof(*ctx)); - atomic_init(&ctx->maxmalloced, sizeof(*ctx)); atomic_init(&ctx->hi_water, 0); atomic_init(&ctx->lo_water, 0); atomic_init(&ctx->hi_called, false); @@ -511,8 +485,6 @@ mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags) { for (i = 0; i < DEBUG_TABLE_COUNT; i++) { ISC_LIST_INIT(ctx->debuglist[i]); } - increment_malloced(ctx, - DEBUG_TABLE_COUNT * sizeof(debuglist_t)); } #endif /* if ISC_MEM_TRACKLINES */ @@ -554,14 +526,11 @@ destroy(isc_mem_t *ctx) { ISC_LIST_UNLINK(ctx->debuglist[i], dl, link); sdallocx(dl, sizeof(*dl), 0); - decrement_malloced(ctx, sizeof(*dl)); } } sdallocx(ctx->debuglist, (DEBUG_TABLE_COUNT * sizeof(debuglist_t)), 0); - decrement_malloced(ctx, - DEBUG_TABLE_COUNT * sizeof(debuglist_t)); } #endif /* if ISC_MEM_TRACKLINES */ @@ -586,7 +555,7 @@ destroy(isc_mem_t *ctx) { isc_mutex_destroy(&ctx->lock); if (ctx->checkfree) { - INSIST(atomic_load(&ctx->malloced) == 0); + INSIST(atomic_load(&ctx->inuse) == 0); } sdallocx(ctx, sizeof(*ctx), ISC_MEM_ALIGN(isc_os_cacheline())); } @@ -1073,20 +1042,6 @@ isc_mem_total(isc_mem_t *ctx) { return (atomic_load_acquire(&ctx->total)); } -size_t -isc_mem_malloced(isc_mem_t *ctx) { - REQUIRE(VALID_CONTEXT(ctx)); - - return (atomic_load_acquire(&ctx->malloced)); -} - -size_t -isc_mem_maxmalloced(isc_mem_t *ctx) { - REQUIRE(VALID_CONTEXT(ctx)); - - return (atomic_load_acquire(&ctx->maxmalloced)); -} - void isc_mem_clearwater(isc_mem_t *mctx) { isc_mem_setwater(mctx, NULL, NULL, 0, 0); @@ -1459,7 +1414,6 @@ isc_mem_references(isc_mem_t *ctx) { typedef struct summarystat { uint64_t total; uint64_t inuse; - uint64_t malloced; uint64_t contextsize; } summarystat_t; @@ -1521,17 +1475,6 @@ xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { (uint64_t)isc_mem_maxinuse(ctx))); TRY0(xmlTextWriterEndElement(writer)); /* maxinuse */ - summary->malloced += isc_mem_malloced(ctx); - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "malloced")); - TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - (uint64_t)isc_mem_malloced(ctx))); - TRY0(xmlTextWriterEndElement(writer)); /* malloced */ - - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "maxmalloced")); - TRY0(xmlTextWriterWriteFormatString( - writer, "%" PRIu64 "", (uint64_t)isc_mem_maxmalloced(ctx))); - TRY0(xmlTextWriterEndElement(writer)); /* maxmalloced */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "pools")); TRY0(xmlTextWriterWriteFormatString(writer, "%u", ctx->poolcnt)); TRY0(xmlTextWriterEndElement(writer)); /* pools */ @@ -1594,11 +1537,6 @@ isc_mem_renderxml(void *writer0) { summary.inuse)); TRY0(xmlTextWriterEndElement(writer)); /* InUse */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "Malloced")); - TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - summary.malloced)); - TRY0(xmlTextWriterEndElement(writer)); /* InUse */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "ContextSize")); TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", summary.contextsize)); @@ -1632,7 +1570,6 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { summary->contextsize += sizeof(*ctx); summary->total += isc_mem_total(ctx); summary->inuse += isc_mem_inuse(ctx); - summary->malloced += isc_mem_malloced(ctx); #if ISC_MEM_TRACKLINES if (ctx->debuglist != NULL) { summary->contextsize += DEBUG_TABLE_COUNT * @@ -1671,14 +1608,6 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { CHECKMEM(obj); json_object_object_add(ctxobj, "maxinuse", obj); - obj = json_object_new_int64(isc_mem_malloced(ctx)); - CHECKMEM(obj); - json_object_object_add(ctxobj, "malloced", obj); - - obj = json_object_new_int64(isc_mem_maxmalloced(ctx)); - CHECKMEM(obj); - json_object_object_add(ctxobj, "maxmalloced", obj); - obj = json_object_new_int64(ctx->poolcnt); CHECKMEM(obj); json_object_object_add(ctxobj, "pools", obj); @@ -1731,10 +1660,6 @@ isc_mem_renderjson(void *memobj0) { CHECKMEM(obj); json_object_object_add(memobj, "InUse", obj); - obj = json_object_new_int64(summary.malloced); - CHECKMEM(obj); - json_object_object_add(memobj, "Malloced", obj); - obj = json_object_new_int64(summary.contextsize); CHECKMEM(obj); json_object_object_add(memobj, "ContextSize", obj); From 91e349433fd08e31a04c21694a9726a8b6afb785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 11:46:47 +0100 Subject: [PATCH 03/13] Remove maxinuse memory counter The maxinuse memory counter indicated the highest amount of memory allocated in the past. Checking and updating this high- water mark value every time memory was allocated had an impact on server performance, so it has been removed. Memory size can be monitored more efficiently via an external tool logging RSS. --- bin/named/bind9.xsl | 4 ---- lib/dns/cache.c | 16 ---------------- lib/isc/include/isc/mem.h | 7 ------- lib/isc/mem.c | 30 ------------------------------ 4 files changed, 57 deletions(-) diff --git a/bin/named/bind9.xsl b/bin/named/bind9.xsl index e9e4fa376c..6cbdfcc693 100644 --- a/bin/named/bind9.xsl +++ b/bin/named/bind9.xsl @@ -1041,7 +1041,6 @@ References TotalUse InUse - MaxUse BlockSize Pools HiWater @@ -1071,9 +1070,6 @@ - - - diff --git a/lib/dns/cache.c b/lib/dns/cache.c index 7aedbb6fb5..146ec8a373 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -648,17 +648,11 @@ dns_cache_dumpstats(dns_cache_t *cache, FILE *fp) { "cache tree memory total"); fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_inuse(cache->mctx), "cache tree memory in use"); - fprintf(fp, "%20" PRIu64 " %s\n", - (uint64_t)isc_mem_maxinuse(cache->mctx), - "cache tree highest memory in use"); fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_total(cache->hmctx), "cache heap memory total"); fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_inuse(cache->hmctx), "cache heap memory in use"); - fprintf(fp, "%20" PRIu64 " %s\n", - (uint64_t)isc_mem_maxinuse(cache->hmctx), - "cache heap highest memory in use"); } #ifdef HAVE_LIBXML2 @@ -716,11 +710,9 @@ dns_cache_renderxml(dns_cache_t *cache, void *writer0) { TRY0(renderstat("TreeMemTotal", isc_mem_total(cache->mctx), writer)); TRY0(renderstat("TreeMemInUse", isc_mem_inuse(cache->mctx), writer)); - TRY0(renderstat("TreeMemMax", isc_mem_maxinuse(cache->mctx), writer)); TRY0(renderstat("HeapMemTotal", isc_mem_total(cache->hmctx), writer)); TRY0(renderstat("HeapMemInUse", isc_mem_inuse(cache->hmctx), writer)); - TRY0(renderstat("HeapMemMax", isc_mem_maxinuse(cache->hmctx), writer)); error: return (xmlrc); } @@ -798,10 +790,6 @@ dns_cache_renderjson(dns_cache_t *cache, void *cstats0) { CHECKMEM(obj); json_object_object_add(cstats, "TreeMemInUse", obj); - obj = json_object_new_int64(isc_mem_maxinuse(cache->mctx)); - CHECKMEM(obj); - json_object_object_add(cstats, "TreeMemMax", obj); - obj = json_object_new_int64(isc_mem_total(cache->hmctx)); CHECKMEM(obj); json_object_object_add(cstats, "HeapMemTotal", obj); @@ -810,10 +798,6 @@ dns_cache_renderjson(dns_cache_t *cache, void *cstats0) { CHECKMEM(obj); json_object_object_add(cstats, "HeapMemInUse", obj); - obj = json_object_new_int64(isc_mem_maxinuse(cache->hmctx)); - CHECKMEM(obj); - json_object_object_add(cstats, "HeapMemMax", obj); - result = ISC_R_SUCCESS; error: return (result); diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 9c156a4d6d..f2e4b43065 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -271,13 +271,6 @@ isc_mem_inuse(isc_mem_t *mctx); * allocated from the system but not yet used. */ -size_t -isc_mem_maxinuse(isc_mem_t *mctx); -/*%< - * Get an estimate of the largest amount of memory that has been in - * use in 'mctx' at any time. - */ - size_t isc_mem_total(isc_mem_t *mctx); /*%< diff --git a/lib/isc/mem.c b/lib/isc/mem.c index dbed11eac8..f7217f3471 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -143,7 +143,6 @@ struct isc_mem { char name[16]; atomic_size_t total; atomic_size_t inuse; - atomic_size_t maxinuse; atomic_bool hi_called; atomic_bool is_overmem; isc_mem_water_t water; @@ -462,7 +461,6 @@ mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags) { atomic_init(&ctx->total, 0); atomic_init(&ctx->inuse, 0); - atomic_init(&ctx->maxinuse, 0); atomic_init(&ctx->hi_water, 0); atomic_init(&ctx->lo_water, 0); atomic_init(&ctx->hi_called, false); @@ -673,7 +671,6 @@ isc__mem_destroy(isc_mem_t **ctxp FLARG) { static bool hi_water(isc_mem_t *ctx) { size_t inuse; - size_t maxinuse; size_t hiwater = atomic_load_relaxed(&ctx->hi_water); if (hiwater == 0) { @@ -685,17 +682,6 @@ hi_water(isc_mem_t *ctx) { return (false); } - maxinuse = atomic_load_acquire(&ctx->maxinuse); - if (inuse > maxinuse) { - (void)atomic_compare_exchange_strong(&ctx->maxinuse, &maxinuse, - inuse); - - if ((ctx->debugging & ISC_MEM_DEBUGUSAGE) != 0) { - fprintf(stderr, "maxinuse = %lu\n", - (unsigned long)inuse); - } - } - if (atomic_load_acquire(&ctx->hi_called)) { return (false); } @@ -1028,13 +1014,6 @@ isc_mem_inuse(isc_mem_t *ctx) { return (atomic_load_acquire(&ctx->inuse)); } -size_t -isc_mem_maxinuse(isc_mem_t *ctx) { - REQUIRE(VALID_CONTEXT(ctx)); - - return (atomic_load_acquire(&ctx->maxinuse)); -} - size_t isc_mem_total(isc_mem_t *ctx) { REQUIRE(VALID_CONTEXT(ctx)); @@ -1470,11 +1449,6 @@ xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { (uint64_t)isc_mem_inuse(ctx))); TRY0(xmlTextWriterEndElement(writer)); /* inuse */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "maxinuse")); - TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - (uint64_t)isc_mem_maxinuse(ctx))); - TRY0(xmlTextWriterEndElement(writer)); /* maxinuse */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "pools")); TRY0(xmlTextWriterWriteFormatString(writer, "%u", ctx->poolcnt)); TRY0(xmlTextWriterEndElement(writer)); /* pools */ @@ -1604,10 +1578,6 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { CHECKMEM(obj); json_object_object_add(ctxobj, "inuse", obj); - obj = json_object_new_int64(isc_mem_maxinuse(ctx)); - CHECKMEM(obj); - json_object_object_add(ctxobj, "maxinuse", obj); - obj = json_object_new_int64(ctx->poolcnt); CHECKMEM(obj); json_object_object_add(ctxobj, "pools", obj); From 3d4e41d0767070ddfadca19540c7afcbff929257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 11:50:39 +0100 Subject: [PATCH 04/13] Remove the total memory counter The total memory counter had again little or no meaning when we removed the internal memory allocator. It was just a monotonic counter that would count add the allocation sizes but never subtracted anything, so it would be just a "big number". --- bin/named/bind9.xsl | 4 ---- lib/dns/cache.c | 14 ------------ lib/isc/include/isc/mem.h | 7 ------ lib/isc/mem.c | 31 --------------------------- tests/isc/mem_test.c | 45 --------------------------------------- 5 files changed, 101 deletions(-) diff --git a/bin/named/bind9.xsl b/bin/named/bind9.xsl index 6cbdfcc693..14901aa531 100644 --- a/bin/named/bind9.xsl +++ b/bin/named/bind9.xsl @@ -1039,7 +1039,6 @@ ID Name References - TotalUse InUse BlockSize Pools @@ -1064,9 +1063,6 @@ - - - diff --git a/lib/dns/cache.c b/lib/dns/cache.c index 146ec8a373..6f8067428c 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -644,13 +644,9 @@ dns_cache_dumpstats(dns_cache_t *cache, FILE *fp) { fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)dns_db_hashsize(cache->db), "cache database hash buckets"); - fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_total(cache->mctx), - "cache tree memory total"); fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_inuse(cache->mctx), "cache tree memory in use"); - fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_total(cache->hmctx), - "cache heap memory total"); fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_inuse(cache->hmctx), "cache heap memory in use"); } @@ -708,10 +704,8 @@ dns_cache_renderxml(dns_cache_t *cache, void *writer0) { dns_db_nodecount(cache->db, dns_dbtree_nsec), writer)); TRY0(renderstat("CacheBuckets", dns_db_hashsize(cache->db), writer)); - TRY0(renderstat("TreeMemTotal", isc_mem_total(cache->mctx), writer)); TRY0(renderstat("TreeMemInUse", isc_mem_inuse(cache->mctx), writer)); - TRY0(renderstat("HeapMemTotal", isc_mem_total(cache->hmctx), writer)); TRY0(renderstat("HeapMemInUse", isc_mem_inuse(cache->hmctx), writer)); error: return (xmlrc); @@ -782,18 +776,10 @@ dns_cache_renderjson(dns_cache_t *cache, void *cstats0) { CHECKMEM(obj); json_object_object_add(cstats, "CacheBuckets", obj); - obj = json_object_new_int64(isc_mem_total(cache->mctx)); - CHECKMEM(obj); - json_object_object_add(cstats, "TreeMemTotal", obj); - obj = json_object_new_int64(isc_mem_inuse(cache->mctx)); CHECKMEM(obj); json_object_object_add(cstats, "TreeMemInUse", obj); - obj = json_object_new_int64(isc_mem_total(cache->hmctx)); - CHECKMEM(obj); - json_object_object_add(cstats, "HeapMemTotal", obj); - obj = json_object_new_int64(isc_mem_inuse(cache->hmctx)); CHECKMEM(obj); json_object_object_add(cstats, "HeapMemInUse", obj); diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index f2e4b43065..0a0c877594 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -271,13 +271,6 @@ isc_mem_inuse(isc_mem_t *mctx); * allocated from the system but not yet used. */ -size_t -isc_mem_total(isc_mem_t *mctx); -/*%< - * Get the total amount of memory in 'mctx', in bytes, including memory - * not yet used. - */ - bool isc_mem_isovermem(isc_mem_t *mctx); /*%< diff --git a/lib/isc/mem.c b/lib/isc/mem.c index f7217f3471..63f5c4cf6e 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -141,7 +141,6 @@ struct isc_mem { struct stats stats[STATS_BUCKETS + 1]; isc_refcount_t references; char name[16]; - atomic_size_t total; atomic_size_t inuse; atomic_bool hi_called; atomic_bool is_overmem; @@ -378,7 +377,6 @@ static void mem_getstats(isc_mem_t *ctx, size_t size) { struct stats *stats = stats_bucket(ctx, size); - atomic_fetch_add_relaxed(&ctx->total, size); atomic_fetch_add_release(&ctx->inuse, size); atomic_fetch_add_relaxed(&stats->gets, 1); @@ -459,7 +457,6 @@ mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags) { isc_mutex_init(&ctx->lock); isc_refcount_init(&ctx->references, 1); - atomic_init(&ctx->total, 0); atomic_init(&ctx->inuse, 0); atomic_init(&ctx->hi_water, 0); atomic_init(&ctx->lo_water, 0); @@ -1014,13 +1011,6 @@ isc_mem_inuse(isc_mem_t *ctx) { return (atomic_load_acquire(&ctx->inuse)); } -size_t -isc_mem_total(isc_mem_t *ctx) { - REQUIRE(VALID_CONTEXT(ctx)); - - return (atomic_load_acquire(&ctx->total)); -} - void isc_mem_clearwater(isc_mem_t *mctx) { isc_mem_setwater(mctx, NULL, NULL, 0, 0); @@ -1391,7 +1381,6 @@ isc_mem_references(isc_mem_t *ctx) { } typedef struct summarystat { - uint64_t total; uint64_t inuse; uint64_t contextsize; } summarystat_t; @@ -1437,12 +1426,6 @@ xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { isc_refcount_current(&ctx->references))); TRY0(xmlTextWriterEndElement(writer)); /* references */ - summary->total += isc_mem_total(ctx); - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "total")); - TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - (uint64_t)isc_mem_total(ctx))); - TRY0(xmlTextWriterEndElement(writer)); /* total */ - summary->inuse += isc_mem_inuse(ctx); TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "inuse")); TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", @@ -1501,11 +1484,6 @@ isc_mem_renderxml(void *writer0) { TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "summary")); - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "TotalUse")); - TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - summary.total)); - TRY0(xmlTextWriterEndElement(writer)); /* TotalUse */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "InUse")); TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", summary.inuse)); @@ -1542,7 +1520,6 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { MCTXLOCK(ctx); summary->contextsize += sizeof(*ctx); - summary->total += isc_mem_total(ctx); summary->inuse += isc_mem_inuse(ctx); #if ISC_MEM_TRACKLINES if (ctx->debuglist != NULL) { @@ -1570,10 +1547,6 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { CHECKMEM(obj); json_object_object_add(ctxobj, "references", obj); - obj = json_object_new_int64(isc_mem_total(ctx)); - CHECKMEM(obj); - json_object_object_add(ctxobj, "total", obj); - obj = json_object_new_int64(isc_mem_inuse(ctx)); CHECKMEM(obj); json_object_object_add(ctxobj, "inuse", obj); @@ -1622,10 +1595,6 @@ isc_mem_renderjson(void *memobj0) { } UNLOCK(&contextslock); - obj = json_object_new_int64(summary.total); - CHECKMEM(obj); - json_object_object_add(memobj, "TotalUse", obj); - obj = json_object_new_int64(summary.inuse); CHECKMEM(obj); json_object_object_add(memobj, "InUse", obj); diff --git a/tests/isc/mem_test.c b/tests/isc/mem_test.c index 5badf1a10c..cdcbda0f28 100644 --- a/tests/isc/mem_test.c +++ b/tests/isc/mem_test.c @@ -242,50 +242,6 @@ ISC_RUN_TEST_IMPL(isc_mem_allocate_zero) { isc_mem_free(mctx, ptr); } -/* test TotalUse calculation */ -ISC_RUN_TEST_IMPL(isc_mem_total) { - isc_mem_t *mctx2 = NULL; - size_t before, after; - ssize_t diff; - int i; - - /* Local alloc, free */ - mctx2 = NULL; - isc_mem_create(&mctx2); - - before = isc_mem_total(mctx2); - - for (i = 0; i < 100000; i++) { - void *ptr; - - ptr = isc_mem_get(mctx2, 2048); - isc_mem_put(mctx2, ptr, 2048); - } - - after = isc_mem_total(mctx2); - diff = after - before; - - assert_int_equal(diff, (2048) * 100000); - - /* ISC_MEMFLAG_INTERNAL */ - - before = isc_mem_total(mctx); - - for (i = 0; i < 100000; i++) { - void *ptr; - - ptr = isc_mem_get(mctx, 2048); - isc_mem_put(mctx, ptr, 2048); - } - - after = isc_mem_total(mctx); - diff = after - before; - - assert_int_equal(diff, (2048) * 100000); - - isc_mem_destroy(&mctx2); -} - /* test InUse calculation */ ISC_RUN_TEST_IMPL(isc_mem_inuse) { isc_mem_t *mctx2 = NULL; @@ -598,7 +554,6 @@ ISC_TEST_ENTRY(isc_mem_allocate_align) #endif /* defined(HAVE_MALLOC_NP_H) || defined(HAVE_JEMALLOC) */ ISC_TEST_ENTRY(isc_mem_get_zero) ISC_TEST_ENTRY(isc_mem_allocate_zero) -ISC_TEST_ENTRY(isc_mem_total) ISC_TEST_ENTRY(isc_mem_inuse) ISC_TEST_ENTRY(isc_mem_zeroget) ISC_TEST_ENTRY(isc_mem_reget) From 1ea889462629bb5fe1e853425163799a53162b10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 11:55:00 +0100 Subject: [PATCH 05/13] Remove the 'totalgets' memory counter The totalgets falls into the same category as other "total" and "max" numbers - it's just a big number with no meaning to end user. --- lib/isc/mem.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 63f5c4cf6e..126add5c04 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -112,7 +112,6 @@ struct element { struct stats { atomic_size_t gets; - atomic_size_t totalgets; }; #define MEM_MAGIC ISC_MAGIC('M', 'e', 'm', 'C') @@ -380,7 +379,6 @@ mem_getstats(isc_mem_t *ctx, size_t size) { atomic_fetch_add_release(&ctx->inuse, size); atomic_fetch_add_relaxed(&stats->gets, 1); - atomic_fetch_add_relaxed(&stats->totalgets, 1); } /*! @@ -465,7 +463,6 @@ mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags) { for (size_t i = 0; i < STATS_BUCKETS + 1; i++) { atomic_init(&ctx->stats[i].gets, 0); - atomic_init(&ctx->stats[i].totalgets, 0); } ISC_LIST_INIT(ctx->pools); @@ -802,17 +799,14 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) { MCTXLOCK(ctx); for (size_t i = 0; i <= STATS_BUCKETS; i++) { - size_t totalgets; size_t gets; struct stats *stats = &ctx->stats[i]; - totalgets = atomic_load_acquire(&stats->totalgets); gets = atomic_load_acquire(&stats->gets); - if (totalgets != 0U && gets != 0U) { - fprintf(out, "%s%5zu: %11zu gets, %11zu rem", - (i == STATS_BUCKETS) ? ">=" : " ", i, - totalgets, gets); + if (gets != 0U) { + fprintf(out, "%s%5zu: %11zu rem", + (i == STATS_BUCKETS) ? ">=" : " ", i, gets); fputc('\n', out); } } From 7588cd5cb1606cc1bc7287f365aaab06fb10b08e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 12:00:04 +0100 Subject: [PATCH 06/13] Remove stats buckets memory counters The stats buckets were again more useful for internal allocator, because we would see the individual "block" caches where the allocations would fall into. Remove the stats buckets, and if needed, we can pull more detailed statistics out of the jemalloc. --- lib/isc/mem.c | 55 ++------------------------------------------------- 1 file changed, 2 insertions(+), 53 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 126add5c04..4fbfc743e8 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -80,8 +80,6 @@ unsigned int isc_mem_defaultflags = ISC_MEMFLAG_DEFAULT; #define ALIGNMENT 8U /*%< must be a power of 2 */ #define ALIGNMENT_SIZE sizeof(size_info) #define DEBUG_TABLE_COUNT 512U -#define STATS_BUCKETS 512U -#define STATS_BUCKET_SIZE 32U /* * Types. @@ -110,10 +108,6 @@ struct element { element *next; }; -struct stats { - atomic_size_t gets; -}; - #define MEM_MAGIC ISC_MAGIC('M', 'e', 'm', 'C') #define VALID_CONTEXT(c) ISC_MAGIC_VALID(c, MEM_MAGIC) @@ -137,7 +131,6 @@ struct isc_mem { unsigned int debugging; isc_mutex_t lock; bool checkfree; - struct stats stats[STATS_BUCKETS + 1]; isc_refcount_t references; char name[16]; atomic_size_t inuse; @@ -374,11 +367,7 @@ mem_realloc(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size, */ static void mem_getstats(isc_mem_t *ctx, size_t size) { - struct stats *stats = stats_bucket(ctx, size); - atomic_fetch_add_release(&ctx->inuse, size); - - atomic_fetch_add_relaxed(&stats->gets, 1); } /*! @@ -386,16 +375,12 @@ mem_getstats(isc_mem_t *ctx, size_t size) { */ static void mem_putstats(isc_mem_t *ctx, void *ptr, size_t size) { - struct stats *stats = stats_bucket(ctx, size); - atomic_size_t s, g; + atomic_size_t s; UNUSED(ptr); s = atomic_fetch_sub_release(&ctx->inuse, size); INSIST(s >= size); - - g = atomic_fetch_sub_release(&stats->gets, 1); - INSIST(g >= 1); } /* @@ -461,9 +446,6 @@ mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags) { atomic_init(&ctx->hi_called, false); atomic_init(&ctx->is_overmem, false); - for (size_t i = 0; i < STATS_BUCKETS + 1; i++) { - atomic_init(&ctx->stats[i].gets, 0); - } ISC_LIST_INIT(ctx->pools); #if ISC_MEM_TRACKLINES @@ -493,8 +475,6 @@ mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags) { static void destroy(isc_mem_t *ctx) { - unsigned int i; - LOCK(&contextslock); ISC_LIST_UNLINK(contexts, ctx, link); totallost += isc_mem_inuse(ctx); @@ -507,7 +487,7 @@ destroy(isc_mem_t *ctx) { #if ISC_MEM_TRACKLINES if (ctx->debuglist != NULL) { debuglink_t *dl; - for (i = 0; i < DEBUG_TABLE_COUNT; i++) { + for (size_t i = 0; i < DEBUG_TABLE_COUNT; i++) { for (dl = ISC_LIST_HEAD(ctx->debuglist[i]); dl != NULL; dl = ISC_LIST_HEAD(ctx->debuglist[i])) { @@ -526,24 +506,6 @@ destroy(isc_mem_t *ctx) { } #endif /* if ISC_MEM_TRACKLINES */ - if (ctx->checkfree) { - for (i = 0; i <= STATS_BUCKETS; i++) { - struct stats *stats = &ctx->stats[i]; - size_t gets = atomic_load_acquire(&stats->gets); - if (gets != 0U) { - fprintf(stderr, - "Failing assertion due to probable " - "leaked memory in context %p (\"%s\") " - "(stats[%u].gets == %zu).\n", - ctx, ctx->name, i, gets); -#if ISC_MEM_TRACKLINES - print_active(ctx, stderr); -#endif /* if ISC_MEM_TRACKLINES */ - INSIST(gets == 0U); - } - } - } - isc_mutex_destroy(&ctx->lock); if (ctx->checkfree) { @@ -798,19 +760,6 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) { MCTXLOCK(ctx); - for (size_t i = 0; i <= STATS_BUCKETS; i++) { - size_t gets; - struct stats *stats = &ctx->stats[i]; - - gets = atomic_load_acquire(&stats->gets); - - if (gets != 0U) { - fprintf(out, "%s%5zu: %11zu rem", - (i == STATS_BUCKETS) ? ">=" : " ", i, gets); - fputc('\n', out); - } - } - /* * Note that since a pool can be locked now, these stats might * be somewhat off if the pool is in active use at the time the From 699736b7bb7ad62089db57e40fdae1188773cd3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 12:02:17 +0100 Subject: [PATCH 07/13] Remove the Lost memory counter The Lost memory counter would count the memory "lost" by external libraries. There's really no such thing as `named` require the memory contexts to be clean on destroy. --- lib/isc/mem.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 4fbfc743e8..0c154322d6 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -119,12 +119,6 @@ static isc_once_t init_once = ISC_ONCE_INIT; static isc_once_t shut_once = ISC_ONCE_INIT; static isc_mutex_t contextslock; -/*% - * Total size of lost memory due to a bug of external library. - * Locked by the global lock. - */ -static uint64_t totallost; - struct isc_mem { unsigned int magic; unsigned int flags; @@ -401,7 +395,6 @@ mem_initialize(void) { isc_mutex_init(&contextslock); ISC_LIST_INIT(contexts); - totallost = 0; } void @@ -477,7 +470,6 @@ static void destroy(isc_mem_t *ctx) { LOCK(&contextslock); ISC_LIST_UNLINK(contexts, ctx, link); - totallost += isc_mem_inuse(ctx); UNLOCK(&contextslock); ctx->magic = 0; @@ -1404,14 +1396,12 @@ int isc_mem_renderxml(void *writer0) { isc_mem_t *ctx; summarystat_t summary = { 0 }; - uint64_t lost; int xmlrc; xmlTextWriterPtr writer = (xmlTextWriterPtr)writer0; TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "contexts")); LOCK(&contextslock); - lost = totallost; for (ctx = ISC_LIST_HEAD(contexts); ctx != NULL; ctx = ISC_LIST_NEXT(ctx, link)) { @@ -1437,10 +1427,6 @@ isc_mem_renderxml(void *writer0) { summary.contextsize)); TRY0(xmlTextWriterEndElement(writer)); /* ContextSize */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "Lost")); - TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", lost)); - TRY0(xmlTextWriterEndElement(writer)); /* Lost */ - TRY0(xmlTextWriterEndElement(writer)); /* summary */ error: return (xmlrc); @@ -1518,7 +1504,6 @@ isc_mem_renderjson(void *memobj0) { isc_result_t result = ISC_R_SUCCESS; isc_mem_t *ctx; summarystat_t summary = { 0 }; - uint64_t lost; json_object *ctxarray, *obj; json_object *memobj = (json_object *)memobj0; @@ -1526,7 +1511,6 @@ isc_mem_renderjson(void *memobj0) { CHECKMEM(ctxarray); LOCK(&contextslock); - lost = totallost; for (ctx = ISC_LIST_HEAD(contexts); ctx != NULL; ctx = ISC_LIST_NEXT(ctx, link)) { @@ -1546,10 +1530,6 @@ isc_mem_renderjson(void *memobj0) { CHECKMEM(obj); json_object_object_add(memobj, "ContextSize", obj); - obj = json_object_new_int64(lost); - CHECKMEM(obj); - json_object_object_add(memobj, "Lost", obj); - json_object_object_add(memobj, "contexts", ctxarray); return (ISC_R_SUCCESS); From a08e2d37ed2b02ae484e021f3101e1053ec550c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 12:05:44 +0100 Subject: [PATCH 08/13] Cleanup the ptr argument from mem_putstats() The ptr argument was unneeded and unused. --- lib/isc/mem.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 0c154322d6..44ad9737da 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -368,12 +368,8 @@ mem_getstats(isc_mem_t *ctx, size_t size) { * Update internal counters after a memory put. */ static void -mem_putstats(isc_mem_t *ctx, void *ptr, size_t size) { - atomic_size_t s; - - UNUSED(ptr); - - s = atomic_fetch_sub_release(&ctx->inuse, size); +mem_putstats(isc_mem_t *ctx, size_t size) { + atomic_size_t s = atomic_fetch_sub_release(&ctx->inuse, size); INSIST(s >= size); } @@ -561,7 +557,7 @@ isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size, DELETE_TRACE(ctx, ptr, size, file, line); - mem_putstats(ctx, ptr, size); + mem_putstats(ctx, size); mem_put(ctx, ptr, size, flags); if (isc_refcount_decrement(&ctx->references) == 1) { @@ -686,7 +682,7 @@ isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size, int flags FLARG) { DELETE_TRACE(ctx, ptr, size, file, line); - mem_putstats(ctx, ptr, size); + mem_putstats(ctx, size); mem_put(ctx, ptr, size, flags); CALL_LO_WATER(ctx); @@ -813,7 +809,7 @@ isc__mem_reget(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size, isc__mem_put(ctx, old_ptr, old_size, flags FLARG_PASS); } else { DELETE_TRACE(ctx, old_ptr, old_size, file, line); - mem_putstats(ctx, old_ptr, old_size); + mem_putstats(ctx, old_size); new_ptr = mem_realloc(ctx, old_ptr, old_size, new_size, flags); @@ -847,7 +843,7 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size, size_t old_size = sallocx(old_ptr, flags); DELETE_TRACE(ctx, old_ptr, old_size, file, line); - mem_putstats(ctx, old_ptr, old_size); + mem_putstats(ctx, old_size); new_ptr = mem_realloc(ctx, old_ptr, old_size, new_size, flags); @@ -880,7 +876,7 @@ isc__mem_free(isc_mem_t *ctx, void *ptr, int flags FLARG) { DELETE_TRACE(ctx, ptr, size, file, line); - mem_putstats(ctx, ptr, size); + mem_putstats(ctx, size); mem_put(ctx, ptr, size, flags); CALL_LO_WATER(ctx); @@ -1118,7 +1114,7 @@ isc__mempool_destroy(isc_mempool_t **restrict mpctxp FLARG) { item = mpctx->items; mpctx->items = item->next; - mem_putstats(mctx, item, mpctx->size); + mem_putstats(mctx, mpctx->size); mem_put(mctx, item, mpctx->size, 0); } @@ -1201,7 +1197,7 @@ isc__mempool_put(isc_mempool_t *restrict mpctx, void *mem FLARG) { * If our free list is full, return this to the mctx directly. */ if (freecount >= freemax) { - mem_putstats(mctx, mem, mpctx->size); + mem_putstats(mctx, mpctx->size); mem_put(mctx, mem, mpctx->size, 0); return; } From 863b2b8bf3feef008db61dd0f6ec43bde81e0c48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 12:10:28 +0100 Subject: [PATCH 09/13] Make the all inuse memory counter atomic operations relaxed Instead of enforcing stronger synchronization between threads, make all the atomic operations relaxed. We are not really interested in exact numbers at all times - the single place where we need the exact number is when the memory context is being destroyed. Even when there's a overmem counter, we don't care about exact ordering or exact number. --- lib/isc/mem.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 44ad9737da..07782508e3 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -361,7 +361,7 @@ mem_realloc(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size, */ static void mem_getstats(isc_mem_t *ctx, size_t size) { - atomic_fetch_add_release(&ctx->inuse, size); + atomic_fetch_add_relaxed(&ctx->inuse, size); } /*! @@ -369,7 +369,7 @@ mem_getstats(isc_mem_t *ctx, size_t size) { */ static void mem_putstats(isc_mem_t *ctx, size_t size) { - atomic_size_t s = atomic_fetch_sub_release(&ctx->inuse, size); + atomic_size_t s = atomic_fetch_sub_relaxed(&ctx->inuse, size); INSIST(s >= size); } @@ -621,7 +621,7 @@ hi_water(isc_mem_t *ctx) { return (false); } - inuse = atomic_load_acquire(&ctx->inuse); + inuse = atomic_load_relaxed(&ctx->inuse); if (inuse <= hiwater) { return (false); } @@ -645,7 +645,7 @@ lo_water(isc_mem_t *ctx) { return (false); } - inuse = atomic_load_acquire(&ctx->inuse); + inuse = atomic_load_relaxed(&ctx->inuse); if (inuse >= lowater) { return (false); } @@ -693,9 +693,9 @@ isc_mem_waterack(isc_mem_t *ctx, int flag) { REQUIRE(VALID_CONTEXT(ctx)); if (flag == ISC_MEM_LOWATER) { - atomic_store(&ctx->hi_called, false); + atomic_store_release(&ctx->hi_called, false); } else if (flag == ISC_MEM_HIWATER) { - atomic_store(&ctx->hi_called, true); + atomic_store_release(&ctx->hi_called, true); } } @@ -939,7 +939,7 @@ size_t isc_mem_inuse(isc_mem_t *ctx) { REQUIRE(VALID_CONTEXT(ctx)); - return (atomic_load_acquire(&ctx->inuse)); + return (atomic_load_relaxed(&ctx->inuse)); } void @@ -968,13 +968,13 @@ isc_mem_setwater(isc_mem_t *ctx, isc_mem_water_t water, void *water_arg, if (oldwater == NULL) { REQUIRE(water != NULL && lowater > 0); - INSIST(atomic_load(&ctx->hi_water) == 0); - INSIST(atomic_load(&ctx->lo_water) == 0); + INSIST(atomic_load_acquire(&ctx->hi_water) == 0); + INSIST(atomic_load_acquire(&ctx->lo_water) == 0); ctx->water = water; ctx->water_arg = water_arg; - atomic_store(&ctx->hi_water, hiwater); - atomic_store(&ctx->lo_water, lowater); + atomic_store_release(&ctx->hi_water, hiwater); + atomic_store_release(&ctx->lo_water, lowater); return; } @@ -982,8 +982,8 @@ isc_mem_setwater(isc_mem_t *ctx, isc_mem_water_t water, void *water_arg, REQUIRE((water == oldwater && water_arg == oldwater_arg) || (water == NULL && water_arg == NULL && hiwater == 0)); - atomic_store(&ctx->hi_water, hiwater); - atomic_store(&ctx->lo_water, lowater); + atomic_store_release(&ctx->hi_water, hiwater); + atomic_store_release(&ctx->lo_water, lowater); if (atomic_load_acquire(&ctx->hi_called) && (atomic_load_acquire(&ctx->inuse) < lowater || lowater == 0U)) From 474279e5f145f6d64842806aadadab908932e0c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 12:23:04 +0100 Subject: [PATCH 10/13] Remove ContextSize memory counter Again, this was an internal allocator counter, now it's useless. --- lib/isc/mem.c | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 07782508e3..323168690a 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -1313,7 +1313,6 @@ isc_mem_references(isc_mem_t *ctx) { typedef struct summarystat { uint64_t inuse; - uint64_t contextsize; } summarystat_t; #ifdef HAVE_LIBXML2 @@ -1343,14 +1342,6 @@ xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { TRY0(xmlTextWriterEndElement(writer)); /* name */ } - summary->contextsize += sizeof(*ctx); -#if ISC_MEM_TRACKLINES - if (ctx->debuglist != NULL) { - summary->contextsize += DEBUG_TABLE_COUNT * - sizeof(debuglist_t) + - ctx->debuglistcnt * sizeof(debuglink_t); - } -#endif /* if ISC_MEM_TRACKLINES */ TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "references")); TRY0(xmlTextWriterWriteFormatString( writer, "%" PRIuFAST32, @@ -1366,7 +1357,6 @@ xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "pools")); TRY0(xmlTextWriterWriteFormatString(writer, "%u", ctx->poolcnt)); TRY0(xmlTextWriterEndElement(writer)); /* pools */ - summary->contextsize += ctx->poolcnt * sizeof(isc_mempool_t); TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "hiwater")); TRY0(xmlTextWriterWriteFormatString( @@ -1418,11 +1408,6 @@ isc_mem_renderxml(void *writer0) { summary.inuse)); TRY0(xmlTextWriterEndElement(writer)); /* InUse */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "ContextSize")); - TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - summary.contextsize)); - TRY0(xmlTextWriterEndElement(writer)); /* ContextSize */ - TRY0(xmlTextWriterEndElement(writer)); /* summary */ error: return (xmlrc); @@ -1444,15 +1429,7 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { MCTXLOCK(ctx); - summary->contextsize += sizeof(*ctx); summary->inuse += isc_mem_inuse(ctx); -#if ISC_MEM_TRACKLINES - if (ctx->debuglist != NULL) { - summary->contextsize += DEBUG_TABLE_COUNT * - sizeof(debuglist_t) + - ctx->debuglistcnt * sizeof(debuglink_t); - } -#endif /* if ISC_MEM_TRACKLINES */ ctxobj = json_object_new_object(); CHECKMEM(ctxobj); @@ -1480,8 +1457,6 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { CHECKMEM(obj); json_object_object_add(ctxobj, "pools", obj); - summary->contextsize += ctx->poolcnt * sizeof(isc_mempool_t); - obj = json_object_new_int64(atomic_load_relaxed(&ctx->hi_water)); CHECKMEM(obj); json_object_object_add(ctxobj, "hiwater", obj); @@ -1522,10 +1497,6 @@ isc_mem_renderjson(void *memobj0) { CHECKMEM(obj); json_object_object_add(memobj, "InUse", obj); - obj = json_object_new_int64(summary.contextsize); - CHECKMEM(obj); - json_object_object_add(memobj, "ContextSize", obj); - json_object_object_add(memobj, "contexts", ctxarray); return (ISC_R_SUCCESS); From 3d674ccc1d5b61259f2bc3a7efa7500469be8b4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 16:57:18 +0100 Subject: [PATCH 11/13] Restore Malloced memory counter as InUse alias + little cleanups This restores the Malloced memory counter and it's now always equal to InUse counter. This is only for backwards compatibility reason and there is no separate counter. The commit also cleanups little things like structure with a single item (summary.inuse), and shuts up a wrong cppcheck warning (the notorious NULL check after assignment). --- lib/isc/mem.c | 45 +++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 323168690a..7b587642e2 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -1158,8 +1158,8 @@ isc__mempool_get(isc_mempool_t *restrict mpctx FLARG) { } } + INSIST(mpctx->items != NULL); item = mpctx->items; - INSIST(item != NULL); mpctx->items = item->next; @@ -1311,10 +1311,6 @@ isc_mem_references(isc_mem_t *ctx) { return (isc_refcount_current(&ctx->references)); } -typedef struct summarystat { - uint64_t inuse; -} summarystat_t; - #ifdef HAVE_LIBXML2 #define TRY0(a) \ do { \ @@ -1323,7 +1319,7 @@ typedef struct summarystat { goto error; \ } while (0) static int -xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { +xml_renderctx(isc_mem_t *ctx, size_t *inuse, xmlTextWriterPtr writer) { REQUIRE(VALID_CONTEXT(ctx)); int xmlrc; @@ -1348,12 +1344,17 @@ xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { isc_refcount_current(&ctx->references))); TRY0(xmlTextWriterEndElement(writer)); /* references */ - summary->inuse += isc_mem_inuse(ctx); + *inuse += isc_mem_inuse(ctx); TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "inuse")); TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", (uint64_t)isc_mem_inuse(ctx))); TRY0(xmlTextWriterEndElement(writer)); /* inuse */ + TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "malloced")); + TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", + (uint64_t)isc_mem_inuse(ctx))); + TRY0(xmlTextWriterEndElement(writer)); /* malloced */ + TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "pools")); TRY0(xmlTextWriterWriteFormatString(writer, "%u", ctx->poolcnt)); TRY0(xmlTextWriterEndElement(writer)); /* pools */ @@ -1381,7 +1382,7 @@ error: int isc_mem_renderxml(void *writer0) { isc_mem_t *ctx; - summarystat_t summary = { 0 }; + size_t inuse = 0; int xmlrc; xmlTextWriterPtr writer = (xmlTextWriterPtr)writer0; @@ -1391,7 +1392,7 @@ isc_mem_renderxml(void *writer0) { for (ctx = ISC_LIST_HEAD(contexts); ctx != NULL; ctx = ISC_LIST_NEXT(ctx, link)) { - xmlrc = xml_renderctx(ctx, &summary, writer); + xmlrc = xml_renderctx(ctx, &inuse, writer); if (xmlrc < 0) { UNLOCK(&contextslock); goto error; @@ -1403,9 +1404,14 @@ isc_mem_renderxml(void *writer0) { TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "summary")); + TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "Malloced")); + TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", + (uint64_t)inuse)); + TRY0(xmlTextWriterEndElement(writer)); /* malloced */ + TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "InUse")); TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - summary.inuse)); + (uint64_t)inuse)); TRY0(xmlTextWriterEndElement(writer)); /* InUse */ TRY0(xmlTextWriterEndElement(writer)); /* summary */ @@ -1419,9 +1425,8 @@ error: #define CHECKMEM(m) RUNTIME_CHECK(m != NULL) static isc_result_t -json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { +json_renderctx(isc_mem_t *ctx, size_t *inuse, json_object *array) { REQUIRE(VALID_CONTEXT(ctx)); - REQUIRE(summary != NULL); REQUIRE(array != NULL); json_object *ctxobj, *obj; @@ -1429,7 +1434,7 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { MCTXLOCK(ctx); - summary->inuse += isc_mem_inuse(ctx); + *inuse += isc_mem_inuse(ctx); ctxobj = json_object_new_object(); CHECKMEM(ctxobj); @@ -1449,6 +1454,10 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { CHECKMEM(obj); json_object_object_add(ctxobj, "references", obj); + obj = json_object_new_int64(isc_mem_inuse(ctx)); + CHECKMEM(obj); + json_object_object_add(ctxobj, "malloced", obj); + obj = json_object_new_int64(isc_mem_inuse(ctx)); CHECKMEM(obj); json_object_object_add(ctxobj, "inuse", obj); @@ -1474,7 +1483,7 @@ isc_result_t isc_mem_renderjson(void *memobj0) { isc_result_t result = ISC_R_SUCCESS; isc_mem_t *ctx; - summarystat_t summary = { 0 }; + size_t inuse = 0; json_object *ctxarray, *obj; json_object *memobj = (json_object *)memobj0; @@ -1485,7 +1494,7 @@ isc_mem_renderjson(void *memobj0) { for (ctx = ISC_LIST_HEAD(contexts); ctx != NULL; ctx = ISC_LIST_NEXT(ctx, link)) { - result = json_renderctx(ctx, &summary, ctxarray); + result = json_renderctx(ctx, &inuse, ctxarray); if (result != ISC_R_SUCCESS) { UNLOCK(&contextslock); goto error; @@ -1493,10 +1502,14 @@ isc_mem_renderjson(void *memobj0) { } UNLOCK(&contextslock); - obj = json_object_new_int64(summary.inuse); + obj = json_object_new_int64(inuse); CHECKMEM(obj); json_object_object_add(memobj, "InUse", obj); + obj = json_object_new_int64(inuse); + CHECKMEM(obj); + json_object_object_add(memobj, "Malloced", obj); + json_object_object_add(memobj, "contexts", ctxarray); return (ISC_R_SUCCESS); From a0f322a38bd5c6d70be4d4c1fc1ed2c93581fd80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 17:06:17 +0100 Subject: [PATCH 12/13] Remove BlockSize from bind9.xsl The BlockSize counter doesn't exist anymore (for some time now). --- bin/named/bind9.xsl | 4 ---- 1 file changed, 4 deletions(-) diff --git a/bin/named/bind9.xsl b/bin/named/bind9.xsl index 14901aa531..39db697ef9 100644 --- a/bin/named/bind9.xsl +++ b/bin/named/bind9.xsl @@ -1040,7 +1040,6 @@ Name References InUse - BlockSize Pools HiWater LoWater @@ -1066,9 +1065,6 @@ - - - From 122737ace63e943b157db340e810c67229f3bd02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 19 Jan 2023 17:14:31 +0100 Subject: [PATCH 13/13] Add CHANGES and release note for [GL #3718] --- CHANGES | 3 +++ doc/notes/notes-current.rst | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index c008d91d14..bee2498184 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6078. [func] Cleanup the memory statistic counters to a bare + minumum - InUse with Malloced as alias. [GL #3718] + 6077. [func] Implement query forwarding to DoT-enabled upstream servers. [GL #3726] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 8b38c42da0..600ec0745c 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -48,7 +48,10 @@ Removed Features Feature Changes ~~~~~~~~~~~~~~~ -- None. +- The memory statistics were reduced to a single counter InUse (with Malloced as + alias). Most of the counters were useful with the old BIND 9 internal memory + allocator that has been removed. The InUse/Malloced counter is the only one + that bears any real value in production. :gl:`#3718` Bug Fixes ~~~~~~~~~