Optimize database decref by avoiding locking with refs > 1

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 <jtatuya@infoblox.com>
Co-authored-by: Ondřej Surý <ondrej@isc.org>
(cherry picked from commit 7f4471594db157b61218ee6377e6ba0887092a41)

diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index 3e6c0317332aa822425216458fc3fe009f10e510..edbf567e17822702ea3db78ef1437657f64d22f3 100644 (file)
--- 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);