From eb2e113df13c7b1ede279878f5cbad877af49f8e Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Thu, 9 Sep 2021 21:41:51 +0200 Subject: [PATCH 1/3] 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 --- src/addrman.cpp | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 772c34ae770..a1e8cb1bf16 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -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); From acf656d540a82e6fc30421590305cfe295eabbb5 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Fri, 10 Sep 2021 02:23:16 +0200 Subject: [PATCH 2/3] fuzz: Use public interface to fill addrman tried tables After the performance improvement for Good(), the direct method is only 2x faster as opposed to 60x before. --- src/test/fuzz/addrman.cpp | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index fdbfb3b93b3..1781420f4ff 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -96,31 +96,12 @@ public: for (size_t j = 0; j < num_addresses; ++j) { const auto addr = CAddress{CService{RandAddr(), 8333}, NODE_NETWORK}; const auto time_penalty = insecure_rand.randrange(100000001); -#if 1 - // 2.83 sec to fill. - if (n > 0 && mapInfo.size() % n == 0 && mapAddr.find(addr) == mapAddr.end()) { - // Add to the "tried" table (if the bucket slot is free). - const CAddrInfo dummy{addr, source}; - const int bucket = dummy.GetTriedBucket(nKey, m_asmap); - const int bucket_pos = dummy.GetBucketPosition(nKey, false, bucket); - if (vvTried[bucket][bucket_pos] == -1) { - int id; - CAddrInfo* addr_info = Create(addr, source, &id); - vvTried[bucket][bucket_pos] = id; - addr_info->fInTried = true; - ++nTried; - } - } else { - // Add to the "new" table. - Add_(addr, source, time_penalty); - } -#else - // 261.91 sec to fill. Add_(addr, source, time_penalty); + if (n > 0 && mapInfo.size() % n == 0) { Good_(addr, false, GetTime()); } -#endif + // Add 10% of the addresses from more than one source. if (insecure_rand.randrange(10) == 0 && prev_source.IsValid()) { Add_(addr, prev_source, time_penalty); From 57ce20307e604530f78ef4f0f8d9fb94f80ca81b Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Thu, 16 Sep 2021 00:16:56 +0200 Subject: [PATCH 3/3] fuzz: allow lower number of sources --- src/test/fuzz/addrman.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 1781420f4ff..95c5a99c1b0 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -85,7 +85,7 @@ public: // 0, 1, 2, 3 corresponding to 0%, 100%, 50%, 33% const size_t n = m_fuzzed_data_provider.ConsumeIntegralInRange(0, 3); - const size_t num_sources = m_fuzzed_data_provider.ConsumeIntegralInRange(10, 50); + const size_t num_sources = m_fuzzed_data_provider.ConsumeIntegralInRange(1, 50); CNetAddr prev_source; // Use insecure_rand inside the loops instead of m_fuzzed_data_provider because when // the latter is exhausted it just returns 0.