From 566357f8f7471f74729297868917aa32f6d3c390 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 1 Jul 2021 12:48:51 +0200 Subject: [PATCH 1/5] refactor: move GetRandomNodeEvictionCandidates() to test utilities Co-authored-by: Vasil Dimov --- src/test/net_peer_eviction_tests.cpp | 22 ---------------------- src/test/util/net.cpp | 25 +++++++++++++++++++++++++ src/test/util/net.h | 2 ++ 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 4bfd487b86a..5eb280b4989 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -17,28 +17,6 @@ BOOST_FIXTURE_TEST_SUITE(net_peer_eviction_tests, BasicTestingSetup) -std::vector GetRandomNodeEvictionCandidates(const int n_candidates, FastRandomContext& random_context) -{ - std::vector candidates; - for (int id = 0; id < n_candidates; ++id) { - candidates.push_back({ - /* id */ id, - /* nTimeConnected */ static_cast(random_context.randrange(100)), - /* m_min_ping_time */ std::chrono::microseconds{random_context.randrange(100)}, - /* nLastBlockTime */ static_cast(random_context.randrange(100)), - /* nLastTXTime */ static_cast(random_context.randrange(100)), - /* fRelevantServices */ random_context.randbool(), - /* fRelayTxes */ random_context.randbool(), - /* fBloomFilter */ random_context.randbool(), - /* nKeyedNetGroup */ random_context.randrange(100), - /* prefer_evict */ random_context.randbool(), - /* m_is_local */ random_context.randbool(), - /* m_network */ ALL_NETWORKS[random_context.randrange(ALL_NETWORKS.size())], - }); - } - return candidates; -} - // Create `num_peers` random nodes, apply setup function `candidate_setup_fn`, // call ProtectEvictionCandidatesByRatio() to apply protection logic, and then // return true if all of `protected_peer_ids` and none of `unprotected_peer_ids` diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index 847a490e033..28d79670786 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -6,6 +6,9 @@ #include #include +#include + +#include void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, Span msg_bytes, bool& complete) const { @@ -37,3 +40,25 @@ bool ConnmanTestMsg::ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) con NodeReceiveMsgBytes(node, ser_msg.data, complete); return complete; } + +std::vector GetRandomNodeEvictionCandidates(int n_candidates, FastRandomContext& random_context) +{ + std::vector candidates; + for (int id = 0; id < n_candidates; ++id) { + candidates.push_back({ + /* id */ id, + /* nTimeConnected */ static_cast(random_context.randrange(100)), + /* m_min_ping_time */ std::chrono::microseconds{random_context.randrange(100)}, + /* nLastBlockTime */ static_cast(random_context.randrange(100)), + /* nLastTXTime */ static_cast(random_context.randrange(100)), + /* fRelevantServices */ random_context.randbool(), + /* fRelayTxes */ random_context.randbool(), + /* fBloomFilter */ random_context.randbool(), + /* nKeyedNetGroup */ random_context.randrange(100), + /* prefer_evict */ random_context.randbool(), + /* m_is_local */ random_context.randbool(), + /* m_network */ ALL_NETWORKS[random_context.randrange(ALL_NETWORKS.size())], + }); + } + return candidates; +} diff --git a/src/test/util/net.h b/src/test/util/net.h index 1b49a671bdc..939ec322ede 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -141,4 +141,6 @@ private: mutable size_t m_consumed; }; +std::vector GetRandomNodeEvictionCandidates(int n_candidates, FastRandomContext& random_context); + #endif // BITCOIN_TEST_UTIL_NET_H From 5adb06457403f8c1d874e9c6748ecbb78ef8fa2b Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sun, 20 Jun 2021 13:04:45 +0200 Subject: [PATCH 2/5] bench: add peer eviction protection benchmarks Co-authored-by: Vasil Dimov --- src/Makefile.bench.include | 1 + src/bench/peer_eviction.cpp | 156 ++++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 src/bench/peer_eviction.cpp diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 56b8ca8ce68..2a8e4a0aaca 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -35,6 +35,7 @@ bench_bench_bitcoin_SOURCES = \ bench/mempool_stress.cpp \ bench/nanobench.h \ bench/nanobench.cpp \ + bench/peer_eviction.cpp \ bench/rpc_blockchain.cpp \ bench/rpc_mempool.cpp \ bench/util_time.cpp \ diff --git a/src/bench/peer_eviction.cpp b/src/bench/peer_eviction.cpp new file mode 100644 index 00000000000..0469f0cb4c0 --- /dev/null +++ b/src/bench/peer_eviction.cpp @@ -0,0 +1,156 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +static void EvictionProtectionCommon( + benchmark::Bench& bench, + int num_candidates, + std::function candidate_setup_fn) +{ + using Candidates = std::vector; + FastRandomContext random_context{true}; + bench.warmup(100).epochIterations(1100); + + Candidates candidates{GetRandomNodeEvictionCandidates(num_candidates, random_context)}; + for (auto& c : candidates) { + candidate_setup_fn(c); + } + + std::vector copies{bench.epochs() * bench.epochIterations(), candidates}; + size_t i{0}; + bench.run([&] { + ProtectEvictionCandidatesByRatio(copies.at(i)); + ++i; + }); +} + +/* Benchmarks */ + +static void EvictionProtection0Networks250Candidates(benchmark::Bench& bench) +{ + EvictionProtectionCommon( + bench, + 250 /* num_candidates */, + [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_network = NET_IPV4; + }); +} + +static void EvictionProtection1Networks250Candidates(benchmark::Bench& bench) +{ + EvictionProtectionCommon( + bench, + 250 /* num_candidates */, + [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = false; + if (c.id >= 130 && c.id < 240) { // 110 Tor + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV4; + } + }); +} + +static void EvictionProtection2Networks250Candidates(benchmark::Bench& bench) +{ + EvictionProtectionCommon( + bench, + 250 /* num_candidates */, + [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = false; + if (c.id >= 90 && c.id < 160) { // 70 Tor + c.m_network = NET_ONION; + } else if (c.id >= 170 && c.id < 250) { // 80 I2P + c.m_network = NET_I2P; + } else { + c.m_network = NET_IPV4; + } + }); +} + +static void EvictionProtection3Networks050Candidates(benchmark::Bench& bench) +{ + EvictionProtectionCommon( + bench, + 50 /* num_candidates */, + [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id == 28 || c.id == 47); // 2 localhost + if (c.id >= 30 && c.id < 47) { // 17 I2P + c.m_network = NET_I2P; + } else if (c.id >= 24 && c.id < 28) { // 4 Tor + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV4; + } + }); +} + +static void EvictionProtection3Networks100Candidates(benchmark::Bench& bench) +{ + EvictionProtectionCommon( + bench, + 100 /* num_candidates */, + [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id >= 55 && c.id < 60); // 5 localhost + if (c.id >= 70 && c.id < 80) { // 10 I2P + c.m_network = NET_I2P; + } else if (c.id >= 80 && c.id < 96) { // 16 Tor + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV4; + } + }); +} + +static void EvictionProtection3Networks250Candidates(benchmark::Bench& bench) +{ + EvictionProtectionCommon( + bench, + 250 /* num_candidates */, + [](NodeEvictionCandidate& c) { + c.nTimeConnected = c.id; + c.m_is_local = (c.id >= 140 && c.id < 160); // 20 localhost + if (c.id >= 170 && c.id < 180) { // 10 I2P + c.m_network = NET_I2P; + } else if (c.id >= 190 && c.id < 240) { // 50 Tor + c.m_network = NET_ONION; + } else { + c.m_network = NET_IPV4; + } + }); +} + +// Candidate numbers used for the benchmarks: +// - 50 candidates simulates a possible use of -maxconnections +// - 100 candidates approximates an average node with default settings +// - 250 candidates is the number of peers reported by operators of busy nodes + +// No disadvantaged networks, with 250 eviction candidates. +BENCHMARK(EvictionProtection0Networks250Candidates); + +// 1 disadvantaged network (Tor) with 250 eviction candidates. +BENCHMARK(EvictionProtection1Networks250Candidates); + +// 2 disadvantaged networks (I2P, Tor) with 250 eviction candidates. +BENCHMARK(EvictionProtection2Networks250Candidates); + +// 3 disadvantaged networks (I2P/localhost/Tor) with 50/100/250 eviction candidates. +BENCHMARK(EvictionProtection3Networks050Candidates); +BENCHMARK(EvictionProtection3Networks100Candidates); +BENCHMARK(EvictionProtection3Networks250Candidates); From 02e411ec456af80d1da76085a814c68bb3aca6de Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sat, 19 Jun 2021 17:19:38 +0200 Subject: [PATCH 3/5] p2p: iterate eviction protection only on networks having candidates in ProtectEvictionCandidatesByRatio(). Thank you to Vasil Dimov, whose suggestions during a post-merge discussion about PR 21261 reminded me that I had done this in earlier versions of the PR, e.g. commits like ef411cd2. Co-authored-by: Vasil Dimov --- src/net.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index 60059249ed0..1611b1a24b9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -935,7 +935,10 @@ void ProtectEvictionCandidatesByRatio(std::vector& evicti const size_t max_protect_by_network{total_protect_size / 2}; size_t num_protected{0}; - while (num_protected < max_protect_by_network) { + // Count the number of disadvantaged networks from which we have peers to protect. + auto num_networks = std::count_if(networks.begin(), networks.end(), [](const Net& n) { return n.count; }); + + while (num_networks != 0 && num_protected < max_protect_by_network) { const size_t disadvantaged_to_protect{max_protect_by_network - num_protected}; const size_t protect_per_network{ std::max(disadvantaged_to_protect / networks.size(), static_cast(1))}; From c9e8d8f9b168dec2bc7b845da38449e96708cf8e Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Tue, 6 Jul 2021 17:01:15 +0200 Subject: [PATCH 4/5] p2p: process more candidates per protection iteration for the usual case when some of the protected networks don't have eviction candidates, to reduce the number of iterations in ProtectEvictionCandidatesByRatio(). Picks up an idea in ef411cd2 that I had dropped. --- src/net.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 1611b1a24b9..9567e2b0cf9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -940,9 +940,7 @@ void ProtectEvictionCandidatesByRatio(std::vector& evicti while (num_networks != 0 && num_protected < max_protect_by_network) { const size_t disadvantaged_to_protect{max_protect_by_network - num_protected}; - const size_t protect_per_network{ - std::max(disadvantaged_to_protect / networks.size(), static_cast(1))}; - + const size_t protect_per_network{std::max(disadvantaged_to_protect / num_networks, static_cast(1))}; // Early exit flag if there are no remaining candidates by disadvantaged network. bool protected_at_least_one{false}; From b1d905c225e87a4a289c0cd3593c6c21cea3fba7 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Sat, 19 Jun 2021 17:07:01 +0200 Subject: [PATCH 5/5] p2p: earlier continuation when no remaining eviction candidates in ProtectEvictionCandidatesByRatio(). With this change, `if (n.count == 0) continue;` will be true if a network had candidates protected in the first iterations and has no candidates remaining to be protected in later iterations. Co-authored-by: Jon Atack --- src/net.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 9567e2b0cf9..f7d46881ee1 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -935,16 +935,18 @@ void ProtectEvictionCandidatesByRatio(std::vector& evicti const size_t max_protect_by_network{total_protect_size / 2}; size_t num_protected{0}; - // Count the number of disadvantaged networks from which we have peers to protect. - auto num_networks = std::count_if(networks.begin(), networks.end(), [](const Net& n) { return n.count; }); - - while (num_networks != 0 && num_protected < max_protect_by_network) { + while (num_protected < max_protect_by_network) { + // Count the number of disadvantaged networks from which we have peers to protect. + auto num_networks = std::count_if(networks.begin(), networks.end(), [](const Net& n) { return n.count; }); + if (num_networks == 0) { + break; + } const size_t disadvantaged_to_protect{max_protect_by_network - num_protected}; const size_t protect_per_network{std::max(disadvantaged_to_protect / num_networks, static_cast(1))}; // Early exit flag if there are no remaining candidates by disadvantaged network. bool protected_at_least_one{false}; - for (const Net& n : networks) { + for (Net& n : networks) { if (n.count == 0) continue; const size_t before = eviction_candidates.size(); EraseLastKElements(eviction_candidates, CompareNodeNetworkTime(n.is_local, n.id), @@ -954,10 +956,12 @@ void ProtectEvictionCandidatesByRatio(std::vector& evicti const size_t after = eviction_candidates.size(); if (before > after) { protected_at_least_one = true; - num_protected += before - after; + const size_t delta{before - after}; + num_protected += delta; if (num_protected >= max_protect_by_network) { break; } + n.count -= delta; } } if (!protected_at_least_one) {