addrman: Improve performance of Good

This is done by removing an unnecessary loop in Good_() and looping
through the new tables in MakeTried() more efficiently, choosing a
starting value that allow us to stop early in typical cases.

Co-authored-by: John Newbery <john@johnnewbery.com>

1 files changed, 9 insertions, 17 deletions

diff --git a/src/addrman.cpp b/src/addrman.cpp
index 772c34ae77..a1e8cb1bf1 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -11,6 +11,7 @@
 #include <netaddress.h>
 #include <serialize.h>
 #include <streams.h>
+#include <util/check.h>
 
 #include <cmath>
 #include <optional>
@@ -488,11 +489,14 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId)
     AssertLockHeld(cs);
 
     // remove the entry from all new buckets
-    for (int bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) {
-        int pos = info.GetBucketPosition(nKey, true, bucket);
+    const int start_bucket{info.GetNewBucket(nKey, m_asmap)};
+    for (int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; ++n) {
+        const int bucket{(start_bucket + n) % ADDRMAN_NEW_BUCKET_COUNT};
+        const int pos{info.GetBucketPosition(nKey, true, bucket)};
         if (vvNew[bucket][pos] == nId) {
             vvNew[bucket][pos] = -1;
             info.nRefCount--;
+            if (info.nRefCount == 0) break;
         }
     }
     nNew--;
@@ -564,22 +568,10 @@ void CAddrMan::Good_(const CService& addr, bool test_before_evict, int64_t nTime
     if (info.fInTried)
         return;
 
-    // find a bucket it is in now
-    int nRnd = insecure_rand.randrange(ADDRMAN_NEW_BUCKET_COUNT);
-    int nUBucket = -1;
-    for (unsigned int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) {
-        int nB = (n + nRnd) % ADDRMAN_NEW_BUCKET_COUNT;
-        int nBpos = info.GetBucketPosition(nKey, true, nB);
-        if (vvNew[nB][nBpos] == nId) {
-            nUBucket = nB;
-            break;
-        }
-    }
-
-    // if no bucket is found, something bad happened;
-    // TODO: maybe re-add the node, but for now, just bail out
-    if (nUBucket == -1)
+    // if it is not in new, something bad happened
+    if (!Assume(info.nRefCount > 0)) {
         return;
+    }
 
     // which tried bucket to move the entry to
     int tried_bucket = info.GetTriedBucket(nKey, m_asmap);