diff --git a/lib/dns/include/dns/qp.h b/lib/dns/include/dns/qp.h index ef676de7d4..b089cc7db5 100644 --- a/lib/dns/include/dns/qp.h +++ b/lib/dns/include/dns/qp.h @@ -230,6 +230,15 @@ typedef struct dns_qp_memusage { bool fragmented; /*%< trie needs compaction */ } dns_qp_memusage_t; +/*% + * Choice of mode for `dns_qp_compact()` + */ +typedef enum dns_qpgc { + DNS_QPGC_MAYBE, + DNS_QPGC_NOW, + DNS_QPGC_ALL, +} dns_qpgc_t; + /*********************************************************************** * * functions - create, destory, enquire @@ -298,23 +307,28 @@ dns_qpmulti_destroy(dns_qpmulti_t **qpmp); */ void -dns_qp_compact(dns_qp_t *qp, bool all); +dns_qp_compact(dns_qp_t *qp, dns_qpgc_t mode); /*%< * Defragment the qp-trie and release unused memory. * * When modifications make a trie too fragmented, it is automatically * compacted. However, automatic compaction is limited when a * multithreaded trie has lots of immutable memory from past - * transactions, and lightweight write transactions do not do + * transactions, and lightweight write transactions do not compact on + * commit like heavyweight update transactions. * * This function can be used with a single-threaded qp-trie and during a * transaction on a multi-threaded trie. * + * \li If `mode == DNS_QPGC_MAYBE`, the trie is cleaned if it is fragmented + * + * \li If `mode == DNS_QPGC_NOW`, the trie is cleaned while avoiding + * unnecessary work + * + * \li If `mode == DNS_QPGC_ALL`, the entire trie is compacted + * * Requires: * \li `qp` is a pointer to a valid qp-trie - * \li `all` is true, to compact the whole trie - * \li `all` is false, to save time by not compacting - * chunk that are not fragmented */ void diff --git a/lib/dns/qp.c b/lib/dns/qp.c index 1cdda0f8a5..9c02c94619 100644 --- a/lib/dns/qp.c +++ b/lib/dns/qp.c @@ -877,9 +877,14 @@ compact(dns_qp_t *qp) { } void -dns_qp_compact(dns_qp_t *qp, bool all) { +dns_qp_compact(dns_qp_t *qp, dns_qpgc_t mode) { REQUIRE(QP_VALID(qp)); - qp->compact_all = all; + if (mode == DNS_QPGC_MAYBE && !QP_NEEDGC(qp)) { + return; + } + if (mode == DNS_QPGC_ALL) { + qp->compact_all = true; + } compact(qp); recycle(qp); } @@ -907,7 +912,7 @@ dns_qp_compact(dns_qp_t *qp, bool all) { static inline bool squash_twigs(dns_qp_t *qp, qp_ref_t twigs, qp_weight_t size) { bool destroyed = free_twigs(qp, twigs, size); - if (destroyed && QP_MAX_GARBAGE(qp)) { + if (destroyed && QP_AUTOGC(qp)) { compact(qp); recycle(qp); /* @@ -916,7 +921,7 @@ squash_twigs(dns_qp_t *qp, qp_ref_t twigs, qp_weight_t size) { * time and space, but recovery should be cheaper than * letting compact+recycle fail repeatedly. */ - if (QP_MAX_GARBAGE(qp)) { + if (QP_AUTOGC(qp)) { isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_QP, ISC_LOG_NOTICE, "qp %p uctx \"%s\" compact/recycle " @@ -947,16 +952,9 @@ dns_qp_memusage(dns_qp_t *qp) { .free = qp->free_count, .node_size = sizeof(qp_node_t), .chunk_size = QP_CHUNK_SIZE, + .fragmented = QP_NEEDGC(qp), }; - /* - * tell the caller about fragmentation of the whole trie - * without discounting immutable chunks - */ - qp->hold_count = 0; - memusage.fragmented = QP_MAX_GARBAGE(qp); - qp->hold_count = memusage.hold; - for (qp_chunk_t chunk = 0; chunk < qp->chunk_max; chunk++) { if (qp->base->ptr[chunk] != NULL) { memusage.chunk_count += 1; @@ -1037,7 +1035,7 @@ transaction_open(dns_qpmulti_t *multi, dns_qp_t **qptp) { } /* - * Ensure QP_MAX_GARBAGE() ignores free space in immutable chunks. + * Ensure QP_AUTOGC() ignores free space in immutable chunks. */ qp->hold_count = qp->free_count; @@ -1144,15 +1142,15 @@ dns_qpmulti_commit(dns_qpmulti_t *multi, dns_qp_t **qptp) { free_twigs(qp, multi->reader_ref, READER_SIZE); } - if (qp->transaction_mode == QP_WRITE) { - multi->reader_ref = alloc_twigs(qp, READER_SIZE); - } else { + if (qp->transaction_mode == QP_UPDATE) { /* minimize memory overhead */ compact(qp); multi->reader_ref = alloc_twigs(qp, READER_SIZE); qp->base->ptr[qp->bump] = chunk_shrink_raw( qp, qp->base->ptr[qp->bump], qp->usage[qp->bump].used * sizeof(qp_node_t)); + } else { + multi->reader_ref = alloc_twigs(qp, READER_SIZE); } /* anchor a new version of the trie */ @@ -1165,7 +1163,9 @@ dns_qpmulti_commit(dns_qpmulti_t *multi, dns_qp_t **qptp) { atomic_store_release(&multi->reader, reader); /* COMMIT */ /* clean up what we can right now */ - recycle(qp); + if (qp->transaction_mode == QP_UPDATE || QP_NEEDGC(qp)) { + recycle(qp); + } /* the reclamation phase must be sampled after the commit */ isc_qsbr_phase_t phase = isc_qsbr_phase(multi->loopmgr); diff --git a/lib/dns/qp_p.h b/lib/dns/qp_p.h index 5fc4cdf801..6f9084e831 100644 --- a/lib/dns/qp_p.h +++ b/lib/dns/qp_p.h @@ -171,9 +171,11 @@ STATIC_ASSERT(6 <= QP_CHUNK_LOG && QP_CHUNK_LOG <= 20, * commits. But they cannot be recovered immediately so they are also * counted as on hold, and discounted when we decide whether to compact. */ -#define QP_MAX_GARBAGE(qp) \ - (((qp)->free_count - (qp)->hold_count) > QP_CHUNK_SIZE * 4 && \ - ((qp)->free_count - (qp)->hold_count) > (qp)->used_count / 2) +#define QP_GC_HEURISTIC(qp, free) \ + ((free) > QP_CHUNK_SIZE * 4 && (free) > (qp)->used_count / 2) + +#define QP_NEEDGC(qp) QP_GC_HEURISTIC(qp, (qp)->free_count) +#define QP_AUTOGC(qp) QP_GC_HEURISTIC(qp, (qp)->free_count - (qp)->hold_count) /* * The chunk base and usage arrays are resized geometically and start off