Skip to content

Commit 21998bc

Browse files
committed
Merge bitcoin/bitcoin#22284: p2p, refactor: performance improvements to ProtectEvictionCandidatesByRatio()
b1d905c p2p: earlier continuation when no remaining eviction candidates (Vasil Dimov) c9e8d8f p2p: process more candidates per protection iteration (Jon Atack) 02e411e p2p: iterate eviction protection only on networks having candidates (Jon Atack) 5adb064 bench: add peer eviction protection benchmarks (Jon Atack) 566357f refactor: move GetRandomNodeEvictionCandidates() to test utilities (Jon Atack) Pull request description: This follow-up to #21261 improves `ProtectEvictionCandidatesByRatio()` for better performance. Benchmarks are added; the performance improvement is between 2x and 5x for the benchmarked cases (CPU 2.50GHz, Turbo off, performance mode, Debian Clang 11 non-debug build). ``` $ ./src/bench/bench_bitcoin -filter="EvictionProtection*.*" ``` The refactored code is well-covered by existing unit tests and also a fuzzer. - `$ ./src/test/test_bitcoin -t net_peer_eviction_tests` - `$ FUZZ=node_eviction ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/node_eviction` ACKs for top commit: klementtan: Tested and code review ACK b1d905c. vasild: ACK b1d905c jarolrod: ACK b1d905c Tree-SHA512: a3a6607b9ea2fec138da9780c03f63e177b6712091c5a3ddc3804b896a7585216446310280791f5e20cc023d02d2f03a4139237e12b5c1d7f2a1fa1011610e96
2 parents c0224bc + b1d905c commit 21998bc

File tree

6 files changed

+194
-27
lines changed

6 files changed

+194
-27
lines changed

src/Makefile.bench.include

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ bench_bench_bitcoin_SOURCES = \
3535
bench/mempool_stress.cpp \
3636
bench/nanobench.h \
3737
bench/nanobench.cpp \
38+
bench/peer_eviction.cpp \
3839
bench/rpc_blockchain.cpp \
3940
bench/rpc_mempool.cpp \
4041
bench/util_time.cpp \

src/bench/peer_eviction.cpp

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
// Copyright (c) 2021 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <bench/bench.h>
6+
#include <net.h>
7+
#include <netaddress.h>
8+
#include <random.h>
9+
#include <test/util/net.h>
10+
#include <test/util/setup_common.h>
11+
12+
#include <algorithm>
13+
#include <functional>
14+
#include <vector>
15+
16+
static void EvictionProtectionCommon(
17+
benchmark::Bench& bench,
18+
int num_candidates,
19+
std::function<void(NodeEvictionCandidate&)> candidate_setup_fn)
20+
{
21+
using Candidates = std::vector<NodeEvictionCandidate>;
22+
FastRandomContext random_context{true};
23+
bench.warmup(100).epochIterations(1100);
24+
25+
Candidates candidates{GetRandomNodeEvictionCandidates(num_candidates, random_context)};
26+
for (auto& c : candidates) {
27+
candidate_setup_fn(c);
28+
}
29+
30+
std::vector<Candidates> copies{bench.epochs() * bench.epochIterations(), candidates};
31+
size_t i{0};
32+
bench.run([&] {
33+
ProtectEvictionCandidatesByRatio(copies.at(i));
34+
++i;
35+
});
36+
}
37+
38+
/* Benchmarks */
39+
40+
static void EvictionProtection0Networks250Candidates(benchmark::Bench& bench)
41+
{
42+
EvictionProtectionCommon(
43+
bench,
44+
250 /* num_candidates */,
45+
[](NodeEvictionCandidate& c) {
46+
c.nTimeConnected = c.id;
47+
c.m_network = NET_IPV4;
48+
});
49+
}
50+
51+
static void EvictionProtection1Networks250Candidates(benchmark::Bench& bench)
52+
{
53+
EvictionProtectionCommon(
54+
bench,
55+
250 /* num_candidates */,
56+
[](NodeEvictionCandidate& c) {
57+
c.nTimeConnected = c.id;
58+
c.m_is_local = false;
59+
if (c.id >= 130 && c.id < 240) { // 110 Tor
60+
c.m_network = NET_ONION;
61+
} else {
62+
c.m_network = NET_IPV4;
63+
}
64+
});
65+
}
66+
67+
static void EvictionProtection2Networks250Candidates(benchmark::Bench& bench)
68+
{
69+
EvictionProtectionCommon(
70+
bench,
71+
250 /* num_candidates */,
72+
[](NodeEvictionCandidate& c) {
73+
c.nTimeConnected = c.id;
74+
c.m_is_local = false;
75+
if (c.id >= 90 && c.id < 160) { // 70 Tor
76+
c.m_network = NET_ONION;
77+
} else if (c.id >= 170 && c.id < 250) { // 80 I2P
78+
c.m_network = NET_I2P;
79+
} else {
80+
c.m_network = NET_IPV4;
81+
}
82+
});
83+
}
84+
85+
static void EvictionProtection3Networks050Candidates(benchmark::Bench& bench)
86+
{
87+
EvictionProtectionCommon(
88+
bench,
89+
50 /* num_candidates */,
90+
[](NodeEvictionCandidate& c) {
91+
c.nTimeConnected = c.id;
92+
c.m_is_local = (c.id == 28 || c.id == 47); // 2 localhost
93+
if (c.id >= 30 && c.id < 47) { // 17 I2P
94+
c.m_network = NET_I2P;
95+
} else if (c.id >= 24 && c.id < 28) { // 4 Tor
96+
c.m_network = NET_ONION;
97+
} else {
98+
c.m_network = NET_IPV4;
99+
}
100+
});
101+
}
102+
103+
static void EvictionProtection3Networks100Candidates(benchmark::Bench& bench)
104+
{
105+
EvictionProtectionCommon(
106+
bench,
107+
100 /* num_candidates */,
108+
[](NodeEvictionCandidate& c) {
109+
c.nTimeConnected = c.id;
110+
c.m_is_local = (c.id >= 55 && c.id < 60); // 5 localhost
111+
if (c.id >= 70 && c.id < 80) { // 10 I2P
112+
c.m_network = NET_I2P;
113+
} else if (c.id >= 80 && c.id < 96) { // 16 Tor
114+
c.m_network = NET_ONION;
115+
} else {
116+
c.m_network = NET_IPV4;
117+
}
118+
});
119+
}
120+
121+
static void EvictionProtection3Networks250Candidates(benchmark::Bench& bench)
122+
{
123+
EvictionProtectionCommon(
124+
bench,
125+
250 /* num_candidates */,
126+
[](NodeEvictionCandidate& c) {
127+
c.nTimeConnected = c.id;
128+
c.m_is_local = (c.id >= 140 && c.id < 160); // 20 localhost
129+
if (c.id >= 170 && c.id < 180) { // 10 I2P
130+
c.m_network = NET_I2P;
131+
} else if (c.id >= 190 && c.id < 240) { // 50 Tor
132+
c.m_network = NET_ONION;
133+
} else {
134+
c.m_network = NET_IPV4;
135+
}
136+
});
137+
}
138+
139+
// Candidate numbers used for the benchmarks:
140+
// - 50 candidates simulates a possible use of -maxconnections
141+
// - 100 candidates approximates an average node with default settings
142+
// - 250 candidates is the number of peers reported by operators of busy nodes
143+
144+
// No disadvantaged networks, with 250 eviction candidates.
145+
BENCHMARK(EvictionProtection0Networks250Candidates);
146+
147+
// 1 disadvantaged network (Tor) with 250 eviction candidates.
148+
BENCHMARK(EvictionProtection1Networks250Candidates);
149+
150+
// 2 disadvantaged networks (I2P, Tor) with 250 eviction candidates.
151+
BENCHMARK(EvictionProtection2Networks250Candidates);
152+
153+
// 3 disadvantaged networks (I2P/localhost/Tor) with 50/100/250 eviction candidates.
154+
BENCHMARK(EvictionProtection3Networks050Candidates);
155+
BENCHMARK(EvictionProtection3Networks100Candidates);
156+
BENCHMARK(EvictionProtection3Networks250Candidates);

src/net.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -937,14 +937,17 @@ void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& evicti
937937
size_t num_protected{0};
938938

939939
while (num_protected < max_protect_by_network) {
940+
// Count the number of disadvantaged networks from which we have peers to protect.
941+
auto num_networks = std::count_if(networks.begin(), networks.end(), [](const Net& n) { return n.count; });
942+
if (num_networks == 0) {
943+
break;
944+
}
940945
const size_t disadvantaged_to_protect{max_protect_by_network - num_protected};
941-
const size_t protect_per_network{
942-
std::max(disadvantaged_to_protect / networks.size(), static_cast<size_t>(1))};
943-
946+
const size_t protect_per_network{std::max(disadvantaged_to_protect / num_networks, static_cast<size_t>(1))};
944947
// Early exit flag if there are no remaining candidates by disadvantaged network.
945948
bool protected_at_least_one{false};
946949

947-
for (const Net& n : networks) {
950+
for (Net& n : networks) {
948951
if (n.count == 0) continue;
949952
const size_t before = eviction_candidates.size();
950953
EraseLastKElements(eviction_candidates, CompareNodeNetworkTime(n.is_local, n.id),
@@ -954,10 +957,12 @@ void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& evicti
954957
const size_t after = eviction_candidates.size();
955958
if (before > after) {
956959
protected_at_least_one = true;
957-
num_protected += before - after;
960+
const size_t delta{before - after};
961+
num_protected += delta;
958962
if (num_protected >= max_protect_by_network) {
959963
break;
960964
}
965+
n.count -= delta;
961966
}
962967
}
963968
if (!protected_at_least_one) {

src/test/net_peer_eviction_tests.cpp

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,6 @@
1717

1818
BOOST_FIXTURE_TEST_SUITE(net_peer_eviction_tests, BasicTestingSetup)
1919

20-
std::vector<NodeEvictionCandidate> GetRandomNodeEvictionCandidates(const int n_candidates, FastRandomContext& random_context)
21-
{
22-
std::vector<NodeEvictionCandidate> candidates;
23-
for (int id = 0; id < n_candidates; ++id) {
24-
candidates.push_back({
25-
/* id */ id,
26-
/* nTimeConnected */ static_cast<int64_t>(random_context.randrange(100)),
27-
/* m_min_ping_time */ std::chrono::microseconds{random_context.randrange(100)},
28-
/* nLastBlockTime */ static_cast<int64_t>(random_context.randrange(100)),
29-
/* nLastTXTime */ static_cast<int64_t>(random_context.randrange(100)),
30-
/* fRelevantServices */ random_context.randbool(),
31-
/* fRelayTxes */ random_context.randbool(),
32-
/* fBloomFilter */ random_context.randbool(),
33-
/* nKeyedNetGroup */ random_context.randrange(100),
34-
/* prefer_evict */ random_context.randbool(),
35-
/* m_is_local */ random_context.randbool(),
36-
/* m_network */ ALL_NETWORKS[random_context.randrange(ALL_NETWORKS.size())],
37-
});
38-
}
39-
return candidates;
40-
}
41-
4220
// Create `num_peers` random nodes, apply setup function `candidate_setup_fn`,
4321
// call ProtectEvictionCandidatesByRatio() to apply protection logic, and then
4422
// return true if all of `protected_peer_ids` and none of `unprotected_peer_ids`

src/test/util/net.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66

77
#include <chainparams.h>
88
#include <net.h>
9+
#include <span.h>
10+
11+
#include <vector>
912

1013
void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_bytes, bool& complete) const
1114
{
@@ -37,3 +40,25 @@ bool ConnmanTestMsg::ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) con
3740
NodeReceiveMsgBytes(node, ser_msg.data, complete);
3841
return complete;
3942
}
43+
44+
std::vector<NodeEvictionCandidate> GetRandomNodeEvictionCandidates(int n_candidates, FastRandomContext& random_context)
45+
{
46+
std::vector<NodeEvictionCandidate> candidates;
47+
for (int id = 0; id < n_candidates; ++id) {
48+
candidates.push_back({
49+
/* id */ id,
50+
/* nTimeConnected */ static_cast<int64_t>(random_context.randrange(100)),
51+
/* m_min_ping_time */ std::chrono::microseconds{random_context.randrange(100)},
52+
/* nLastBlockTime */ static_cast<int64_t>(random_context.randrange(100)),
53+
/* nLastTXTime */ static_cast<int64_t>(random_context.randrange(100)),
54+
/* fRelevantServices */ random_context.randbool(),
55+
/* fRelayTxes */ random_context.randbool(),
56+
/* fBloomFilter */ random_context.randbool(),
57+
/* nKeyedNetGroup */ random_context.randrange(100),
58+
/* prefer_evict */ random_context.randbool(),
59+
/* m_is_local */ random_context.randbool(),
60+
/* m_network */ ALL_NETWORKS[random_context.randrange(ALL_NETWORKS.size())],
61+
});
62+
}
63+
return candidates;
64+
}

src/test/util/net.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,4 +141,6 @@ class StaticContentsSock : public Sock
141141
mutable size_t m_consumed;
142142
};
143143

144+
std::vector<NodeEvictionCandidate> GetRandomNodeEvictionCandidates(int n_candidates, FastRandomContext& random_context);
145+
144146
#endif // BITCOIN_TEST_UTIL_NET_H

0 commit comments

Comments
 (0)