Skip to content

Commit 1a369f0

Browse files
committed
Merge bitcoin/bitcoin#18722: addrman: improve performance by using more suitable containers
a92485b addrman: use unordered_map instead of map (Vasil Dimov) Pull request description: `CAddrMan` uses `std::map` internally even though it does not require that the map's elements are sorted. `std::map`'s access time is `O(log(map size))`. `std::unordered_map` is more suitable as it has a `O(1)` access time. This patch lowers the execution times of `CAddrMan`'s methods as follows (as per `src/bench/addrman.cpp`): ``` AddrMan::Add(): -3.5% AddrMan::GetAddr(): -76% AddrMan::Good(): -0.38% AddrMan::Select(): -45% ``` ACKs for top commit: jonatack: ACK a92485b achow101: ACK a92485b hebasto: re-ACK a92485b, only suggested changes and rebased since my [previous](bitcoin/bitcoin#18722 (review)) review. Tree-SHA512: d82959a00e6bd68a6c4c5a265dd08849e6602ac3231293b7a3a3b7bf82ab1d3ba77f8ca682919c15c5d601b13e468b8836fcf19595248116635f7a50d02ed603
2 parents 9795e8e + a92485b commit 1a369f0

File tree

3 files changed

+36
-15
lines changed

3 files changed

+36
-15
lines changed

src/addrman.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
#include <cmath>
1414
#include <optional>
15+
#include <unordered_map>
16+
#include <unordered_set>
1517

1618
int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool> &asmap) const
1719
{
@@ -77,12 +79,12 @@ double CAddrInfo::GetChance(int64_t nNow) const
7779

7880
CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId)
7981
{
80-
std::map<CNetAddr, int>::iterator it = mapAddr.find(addr);
82+
const auto it = mapAddr.find(addr);
8183
if (it == mapAddr.end())
8284
return nullptr;
8385
if (pnId)
8486
*pnId = (*it).second;
85-
std::map<int, CAddrInfo>::iterator it2 = mapInfo.find((*it).second);
87+
const auto it2 = mapInfo.find((*it).second);
8688
if (it2 != mapInfo.end())
8789
return &(*it2).second;
8890
return nullptr;
@@ -408,8 +410,8 @@ CAddrInfo CAddrMan::Select_(bool newOnly)
408410
#ifdef DEBUG_ADDRMAN
409411
int CAddrMan::Check_()
410412
{
411-
std::set<int> setTried;
412-
std::map<int, int> mapNew;
413+
std::unordered_set<int> setTried;
414+
std::unordered_map<int, int> mapNew;
413415

414416
if (vRandom.size() != (size_t)(nTried + nNew))
415417
return -7;

src/addrman.h

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,22 @@
88

99
#include <clientversion.h>
1010
#include <config/bitcoin-config.h>
11+
#include <fs.h>
12+
#include <hash.h>
1113
#include <netaddress.h>
1214
#include <protocol.h>
1315
#include <random.h>
16+
#include <streams.h>
1417
#include <sync.h>
1518
#include <timedata.h>
1619
#include <tinyformat.h>
1720
#include <util/system.h>
1821

19-
#include <fs.h>
20-
#include <hash.h>
2122
#include <iostream>
22-
#include <map>
2323
#include <optional>
2424
#include <set>
2525
#include <stdint.h>
26-
#include <streams.h>
26+
#include <unordered_map>
2727
#include <vector>
2828

2929
/**
@@ -251,7 +251,7 @@ class CAddrMan
251251

252252
int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30);
253253
s << nUBuckets;
254-
std::map<int, int> mapUnkIds;
254+
std::unordered_map<int, int> mapUnkIds;
255255
int nIds = 0;
256256
for (const auto& entry : mapInfo) {
257257
mapUnkIds[entry.first] = nIds;
@@ -435,13 +435,13 @@ class CAddrMan
435435

436436
// Prune new entries with refcount 0 (as a result of collisions).
437437
int nLostUnk = 0;
438-
for (std::map<int, CAddrInfo>::const_iterator it = mapInfo.begin(); it != mapInfo.end(); ) {
438+
for (auto it = mapInfo.cbegin(); it != mapInfo.cend(); ) {
439439
if (it->second.fInTried == false && it->second.nRefCount == 0) {
440-
std::map<int, CAddrInfo>::const_iterator itCopy = it++;
440+
const auto itCopy = it++;
441441
Delete(itCopy->first);
442-
nLostUnk++;
442+
++nLostUnk;
443443
} else {
444-
it++;
444+
++it;
445445
}
446446
}
447447
if (nLost + nLostUnk > 0) {
@@ -662,10 +662,10 @@ class CAddrMan
662662
int nIdCount GUARDED_BY(cs);
663663

664664
//! table with information about all nIds
665-
std::map<int, CAddrInfo> mapInfo GUARDED_BY(cs);
665+
std::unordered_map<int, CAddrInfo> mapInfo GUARDED_BY(cs);
666666

667667
//! find an nId based on its network address
668-
std::map<CNetAddr, int> mapAddr GUARDED_BY(cs);
668+
std::unordered_map<CNetAddr, int, CNetAddrHash> mapAddr GUARDED_BY(cs);
669669

670670
//! randomly-ordered vector of all nIds
671671
std::vector<int> vRandom GUARDED_BY(cs);

src/netaddress.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111

1212
#include <attributes.h>
1313
#include <compat.h>
14+
#include <crypto/siphash.h>
1415
#include <prevector.h>
16+
#include <random.h>
1517
#include <serialize.h>
1618
#include <tinyformat.h>
1719
#include <util/strencodings.h>
@@ -251,6 +253,7 @@ class CNetAddr
251253
}
252254
}
253255

256+
friend class CNetAddrHash;
254257
friend class CSubNet;
255258

256259
private:
@@ -464,6 +467,22 @@ class CNetAddr
464467
}
465468
};
466469

470+
class CNetAddrHash
471+
{
472+
public:
473+
size_t operator()(const CNetAddr& a) const noexcept
474+
{
475+
CSipHasher hasher(m_salt_k0, m_salt_k1);
476+
hasher.Write(a.m_net);
477+
hasher.Write(a.m_addr.data(), a.m_addr.size());
478+
return static_cast<size_t>(hasher.Finalize());
479+
}
480+
481+
private:
482+
const uint64_t m_salt_k0 = GetRand(std::numeric_limits<uint64_t>::max());
483+
const uint64_t m_salt_k1 = GetRand(std::numeric_limits<uint64_t>::max());
484+
};
485+
467486
class CSubNet
468487
{
469488
protected:

0 commit comments

Comments
 (0)