From 065ffb2eb80b68c5df37eae6e1a114e41ceb586d Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 17 Jan 2025 16:54:19 -0800 Subject: [PATCH] Optimize database decref by avoiding locking with refs > 1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, this function always acquires a node write lock if it might need node cleanup in case the reference decrements to 0. In fact, the lock is unnecessary if the reference is larger than 1 and it can be optimized as an "easy" case. This optimization could even be "necessary". In some extreme cases, many worker threads could repeat acquring and releasing the reference on the same node, resulting in severe lock contention for nothing (as the ref wouldn't decrement to 0 in most cases). This change would prevent noticeable performance drop like query timeout for such cases. Co-authored-by: JINMEI Tatuya Co-authored-by: Ondřej Surý (cherry picked from commit 7f4471594db157b61218ee6377e6ba0887092a41) --- lib/dns/rbtdb.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 3e6c031733..edbf567e17 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -2046,28 +2046,35 @@ decrement_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, isc_result_t result; bool write_locked; bool locked = tlock != isc_rwlocktype_none; - rbtdb_nodelock_t *nodelock; int bucket = node->locknum; + rbtdb_nodelock_t *nodelock = &rbtdb->node_locks[bucket]; bool no_reference = true; uint_fast32_t refs; - nodelock = &rbtdb->node_locks[bucket]; - #define KEEP_NODE(n, r, l) \ ((n)->data != NULL || ((l) && (n)->down != NULL) || \ (n) == (r)->origin_node || (n) == (r)->nsec3_origin_node) + /* Handle easy and/or typical case first. */ + if (isc_refcount_decrement(&node->references) > 1) { + return false; + } + + refs = isc_refcount_decrement(&nodelock->references); + INSIST(refs > 0); + /* Handle easy and typical case first. */ if (!node->dirty && KEEP_NODE(node, rbtdb, locked)) { - if (isc_refcount_decrement(&node->references) == 1) { - refs = isc_refcount_decrement(&nodelock->references); - INSIST(refs > 0); - return true; - } else { - return false; - } + return true; } + /* + * Node lock ref has decremented to 0 and we may need to clean up the + * node. To clean it up, the node ref needs to decrement to 0 under the + * node write lock, so we regain the ref and try again. + */ + new_reference(rbtdb, node, nlock); + /* Upgrade the lock? */ if (nlock == isc_rwlocktype_read) { NODE_UNLOCK(&nodelock->lock, isc_rwlocktype_read);