Skip to content

Commit a7c41f2

Browse files
laanwjlaanwj
committed
Merge #8126: std::shared_ptr based CTransaction storage in mempool
288d85d Get rid of CTxMempool::lookup() entirely (Pieter Wuille) c2a4724 Optimization: use usec in expiration and reuse nNow (Pieter Wuille) e9b4780 Optimization: don't check the mempool at all if no mempool req ever (Pieter Wuille) dbfb426 Optimize the relay map to use shared_ptr's (Pieter Wuille) 8d39d7a Switch CTransaction storage in mempool to std::shared_ptr (Pieter Wuille) 1b9e6d3 Add support for unique_ptr and shared_ptr to memusage (Pieter Wuille)
2 parents 761cddb + 288d85d commit a7c41f2

File tree

5 files changed

+144
-84
lines changed

5 files changed

+144
-84
lines changed

‎src/main.cpp

Lines changed: 31 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,6 @@ uint64_t nPruneTarget = 0;
8181
int64_t nMaxTipAge = DEFAULT_MAX_TIP_AGE;
8282
bool fEnableReplacement = DEFAULT_ENABLE_REPLACEMENT;
8383

84-
std::map<uint256, CTransaction> mapRelay;
85-
std::deque<std::pair<int64_t, uint256> > vRelayExpiration;
86-
CCriticalSection cs_mapRelay;
8784

8885
CFeeRate minRelayTxFee = CFeeRate(DEFAULT_MIN_RELAY_TX_FEE);
8986
CAmount maxTxFee = DEFAULT_TRANSACTION_MAXFEE;
@@ -216,6 +213,12 @@ namespace {
216213

217214
/** Number of peers from which we're downloading blocks. */
218215
int nPeersWithValidatedDownloads = 0;
216+
217+
/** Relay map, protected by cs_main. */
218+
typedef std::map<uint256, std::shared_ptr<const CTransaction>> MapRelay;
219+
MapRelay mapRelay;
220+
/** Expiration-time ordered list of (expire time, relay map entry) pairs, protected by cs_main). */
221+
std::deque<std::pair<int64_t, MapRelay::iterator>> vRelayExpiration;
219222
} // anon namespace
220223

221224
//////////////////////////////////////////////////////////////////////////////
@@ -1443,8 +1446,10 @@ bool GetTransaction(const uint256 &hash, CTransaction &txOut, const Consensus::P
14431446

14441447
LOCK(cs_main);
14451448

1446-
if (mempool.lookup(hash, txOut))
1449+
std::shared_ptr<const CTransaction> ptx = mempool.get(hash);
1450+
if (ptx)
14471451
{
1452+
txOut = *ptx;
14481453
return true;
14491454
}
14501455

@@ -4521,30 +4526,24 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
45214526
}
45224527
}
45234528
}
4524-
else if (inv.IsKnownType())
4529+
else if (inv.type == MSG_TX)
45254530
{
4526-
CTransaction tx;
45274531
// Send stream from relay memory
45284532
bool push = false;
4529-
{
4530-
LOCK(cs_mapRelay);
4531-
map<uint256, CTransaction>::iterator mi = mapRelay.find(inv.hash);
4532-
if (mi != mapRelay.end()) {
4533-
tx = (*mi).second;
4534-
push = true;
4535-
}
4536-
}
4537-
if (!push && inv.type == MSG_TX) {
4538-
int64_t txtime;
4533+
auto mi = mapRelay.find(inv.hash);
4534+
if (mi != mapRelay.end()) {
4535+
pfrom->PushMessage(NetMsgType::TX, *mi->second);
4536+
push = true;
4537+
} else if (pfrom->timeLastMempoolReq) {
4538+
auto txinfo = mempool.info(inv.hash);
45394539
// To protect privacy, do not answer getdata using the mempool when
45404540
// that TX couldn't have been INVed in reply to a MEMPOOL request.
4541-
if (mempool.lookup(inv.hash, tx, txtime) && txtime <= pfrom->timeLastMempoolReq) {
4541+
if (txinfo.tx && txinfo.nTime <= pfrom->timeLastMempoolReq) {
4542+
pfrom->PushMessage(NetMsgType::TX, *txinfo.tx);
45424543
push = true;
45434544
}
45444545
}
4545-
if (push) {
4546-
pfrom->PushMessage(inv.GetCommand(), tx);
4547-
} else {
4546+
if (!push) {
45484547
vNotFound.push_back(inv);
45494548
}
45504549
}
@@ -5923,8 +5922,7 @@ bool SendMessages(CNode* pto)
59235922

59245923
// Respond to BIP35 mempool requests
59255924
if (fSendTrickle && pto->fSendMempool) {
5926-
std::vector<uint256> vtxid;
5927-
mempool.queryHashes(vtxid);
5925+
auto vtxinfo = mempool.infoAll();
59285926
pto->fSendMempool = false;
59295927
CAmount filterrate = 0;
59305928
{
@@ -5934,20 +5932,16 @@ bool SendMessages(CNode* pto)
59345932

59355933
LOCK(pto->cs_filter);
59365934

5937-
BOOST_FOREACH(const uint256& hash, vtxid) {
5935+
for (const auto& txinfo : vtxinfo) {
5936+
const uint256& hash = txinfo.tx->GetHash();
59385937
CInv inv(MSG_TX, hash);
59395938
pto->setInventoryTxToSend.erase(hash);
59405939
if (filterrate) {
5941-
CFeeRate feeRate;
5942-
mempool.lookupFeeRate(hash, feeRate);
5943-
if (feeRate.GetFeePerK() < filterrate)
5940+
if (txinfo.feeRate.GetFeePerK() < filterrate)
59445941
continue;
59455942
}
59465943
if (pto->pfilter) {
5947-
CTransaction tx;
5948-
bool fInMemPool = mempool.lookup(hash, tx);
5949-
if (!fInMemPool) continue; // another thread removed since queryHashes, maybe...
5950-
if (!pto->pfilter->IsRelevantAndUpdate(tx)) continue;
5944+
if (!pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue;
59515945
}
59525946
pto->filterInventoryKnown.insert(hash);
59535947
vInv.push_back(inv);
@@ -5993,31 +5987,28 @@ bool SendMessages(CNode* pto)
59935987
continue;
59945988
}
59955989
// Not in the mempool anymore? don't bother sending it.
5996-
CFeeRate feeRate;
5997-
if (!mempool.lookupFeeRate(hash, feeRate)) {
5990+
auto txinfo = mempool.info(hash);
5991+
if (!txinfo.tx) {
59985992
continue;
59995993
}
6000-
if (filterrate && feeRate.GetFeePerK() < filterrate) {
5994+
if (filterrate && txinfo.feeRate.GetFeePerK() < filterrate) {
60015995
continue;
60025996
}
6003-
CTransaction tx;
6004-
if (!mempool.lookup(hash, tx)) continue;
6005-
if (pto->pfilter && !pto->pfilter->IsRelevantAndUpdate(tx)) continue;
5997+
if (pto->pfilter && !pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue;
60065998
// Send
60075999
vInv.push_back(CInv(MSG_TX, hash));
60086000
nRelayedTransactions++;
60096001
{
6010-
LOCK(cs_mapRelay);
60116002
// Expire old relay messages
6012-
while (!vRelayExpiration.empty() && vRelayExpiration.front().first < GetTime())
6003+
while (!vRelayExpiration.empty() && vRelayExpiration.front().first < nNow)
60136004
{
60146005
mapRelay.erase(vRelayExpiration.front().second);
60156006
vRelayExpiration.pop_front();
60166007
}
60176008

6018-
auto ret = mapRelay.insert(std::make_pair(hash, tx));
6009+
auto ret = mapRelay.insert(std::make_pair(hash, std::move(txinfo.tx)));
60196010
if (ret.second) {
6020-
vRelayExpiration.push_back(std::make_pair(GetTime() + 15 * 60, hash));
6011+
vRelayExpiration.push_back(std::make_pair(nNow + 15 * 60 * 1000000, ret.first));
60216012
}
60226013
}
60236014
if (vInv.size() == MAX_INV_SZ) {

‎src/memusage.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,15 @@ struct stl_tree_node
7272
X x;
7373
};
7474

75+
struct stl_shared_counter
76+
{
77+
/* Various platforms use different sized counters here.
78+
* Conservatively assume that they won't be larger than size_t. */
79+
void* class_type;
80+
size_t use_count;
81+
size_t weak_count;
82+
};
83+
7584
template<typename X>
7685
static inline size_t DynamicUsage(const std::vector<X>& v)
7786
{
@@ -122,6 +131,21 @@ static inline size_t IncrementalDynamicUsage(const indirectmap<X, Y>& m)
122131
return MallocUsage(sizeof(stl_tree_node<std::pair<const X*, Y> >));
123132
}
124133

134+
template<typename X>
135+
static inline size_t DynamicUsage(const std::unique_ptr<X>& p)
136+
{
137+
return p ? MallocUsage(sizeof(X)) : 0;
138+
}
139+
140+
template<typename X>
141+
static inline size_t DynamicUsage(const std::shared_ptr<X>& p)
142+
{
143+
// A shared_ptr can either use a single continuous memory block for both
144+
// the counter and the storage (when using std::make_shared), or separate.
145+
// We can't observe the difference, however, so assume the worst.
146+
return p ? MallocUsage(sizeof(X)) + MallocUsage(sizeof(stl_shared_counter)) : 0;
147+
}
148+
125149
// Boost data structures
126150

127151
template<typename X>

‎src/test/policyestimator_tests.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
7474
// 9/10 blocks add 2nd highest and so on until ...
7575
// 1/10 blocks add lowest fee/pri transactions
7676
while (txHashes[9-h].size()) {
77-
CTransaction btx;
78-
if (mpool.lookup(txHashes[9-h].back(), btx))
79-
block.push_back(btx);
77+
std::shared_ptr<const CTransaction> ptx = mpool.get(txHashes[9-h].back());
78+
if (ptx)
79+
block.push_back(*ptx);
8080
txHashes[9-h].pop_back();
8181
}
8282
}
@@ -160,9 +160,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
160160
// Estimates should still not be below original
161161
for (int j = 0; j < 10; j++) {
162162
while(txHashes[j].size()) {
163-
CTransaction btx;
164-
if (mpool.lookup(txHashes[j].back(), btx))
165-
block.push_back(btx);
163+
std::shared_ptr<const CTransaction> ptx = mpool.get(txHashes[j].back());
164+
if (ptx)
165+
block.push_back(*ptx);
166166
txHashes[j].pop_back();
167167
}
168168
}
@@ -181,9 +181,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
181181
tx.vin[0].prevout.n = 10000*blocknum+100*j+k;
182182
uint256 hash = tx.GetHash();
183183
mpool.addUnchecked(hash, entry.Fee(feeV[k/4][j]).Time(GetTime()).Priority(priV[k/4][j]).Height(blocknum).FromTx(tx, &mpool));
184-
CTransaction btx;
185-
if (mpool.lookup(hash, btx))
186-
block.push_back(btx);
184+
std::shared_ptr<const CTransaction> ptx = mpool.get(hash);
185+
if (ptx)
186+
block.push_back(*ptx);
187187
}
188188
}
189189
mpool.removeForBlock(block, ++blocknum, dummyConflicted);

‎src/txmempool.cpp

Lines changed: 56 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,18 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee,
2323
int64_t _nTime, double _entryPriority, unsigned int _entryHeight,
2424
bool poolHasNoInputsOf, CAmount _inChainInputValue,
2525
bool _spendsCoinbase, unsigned int _sigOps, LockPoints lp):
26-
tx(_tx), nFee(_nFee), nTime(_nTime), entryPriority(_entryPriority), entryHeight(_entryHeight),
26+
tx(std::make_shared<CTransaction>(_tx)), nFee(_nFee), nTime(_nTime), entryPriority(_entryPriority), entryHeight(_entryHeight),
2727
hadNoDependencies(poolHasNoInputsOf), inChainInputValue(_inChainInputValue),
2828
spendsCoinbase(_spendsCoinbase), sigOpCount(_sigOps), lockPoints(lp)
2929
{
30-
nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION);
31-
nModSize = tx.CalculateModifiedSize(nTxSize);
32-
nUsageSize = RecursiveDynamicUsage(tx);
30+
nTxSize = ::GetSerializeSize(_tx, SER_NETWORK, PROTOCOL_VERSION);
31+
nModSize = _tx.CalculateModifiedSize(nTxSize);
32+
nUsageSize = RecursiveDynamicUsage(*tx) + memusage::DynamicUsage(tx);
3333

3434
nCountWithDescendants = 1;
3535
nSizeWithDescendants = nTxSize;
3636
nModFeesWithDescendants = nFee;
37-
CAmount nValueIn = tx.GetValueOut()+nFee;
37+
CAmount nValueIn = _tx.GetValueOut()+nFee;
3838
assert(inChainInputValue <= nValueIn);
3939

4040
feeDelta = 0;
@@ -768,50 +768,76 @@ bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb
768768
namespace {
769769
class DepthAndScoreComparator
770770
{
771-
CTxMemPool *mp;
772771
public:
773-
DepthAndScoreComparator(CTxMemPool *mempool) : mp(mempool) {}
774-
bool operator()(const uint256& a, const uint256& b) { return mp->CompareDepthAndScore(a, b); }
772+
bool operator()(const CTxMemPool::indexed_transaction_set::const_iterator& a, const CTxMemPool::indexed_transaction_set::const_iterator& b)
773+
{
774+
uint64_t counta = a->GetCountWithAncestors();
775+
uint64_t countb = b->GetCountWithAncestors();
776+
if (counta == countb) {
777+
return CompareTxMemPoolEntryByScore()(*a, *b);
778+
}
779+
return counta < countb;
780+
}
775781
};
776782
}
777783

778-
void CTxMemPool::queryHashes(vector<uint256>& vtxid)
784+
std::vector<CTxMemPool::indexed_transaction_set::const_iterator> CTxMemPool::GetSortedDepthAndScore() const
779785
{
780-
vtxid.clear();
786+
std::vector<indexed_transaction_set::const_iterator> iters;
787+
AssertLockHeld(cs);
788+
789+
iters.reserve(mapTx.size());
781790

791+
for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) {
792+
iters.push_back(mi);
793+
}
794+
std::sort(iters.begin(), iters.end(), DepthAndScoreComparator());
795+
return iters;
796+
}
797+
798+
void CTxMemPool::queryHashes(vector<uint256>& vtxid)
799+
{
782800
LOCK(cs);
801+
auto iters = GetSortedDepthAndScore();
802+
803+
vtxid.clear();
783804
vtxid.reserve(mapTx.size());
784-
for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi)
785-
vtxid.push_back(mi->GetTx().GetHash());
786805

787-
std::sort(vtxid.begin(), vtxid.end(), DepthAndScoreComparator(this));
806+
for (auto it : iters) {
807+
vtxid.push_back(it->GetTx().GetHash());
808+
}
788809
}
789810

790-
791-
bool CTxMemPool::lookup(uint256 hash, CTransaction& result, int64_t& time) const
811+
std::vector<TxMempoolInfo> CTxMemPool::infoAll() const
792812
{
793813
LOCK(cs);
794-
indexed_transaction_set::const_iterator i = mapTx.find(hash);
795-
if (i == mapTx.end()) return false;
796-
result = i->GetTx();
797-
time = i->GetTime();
798-
return true;
814+
auto iters = GetSortedDepthAndScore();
815+
816+
std::vector<TxMempoolInfo> ret;
817+
ret.reserve(mapTx.size());
818+
for (auto it : iters) {
819+
ret.push_back(TxMempoolInfo{it->GetSharedTx(), it->GetTime(), CFeeRate(it->GetFee(), it->GetTxSize())});
820+
}
821+
822+
return ret;
799823
}
800824

801-
bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const
825+
std::shared_ptr<const CTransaction> CTxMemPool::get(const uint256& hash) const
802826
{
803-
int64_t time;
804-
return CTxMemPool::lookup(hash, result, time);
827+
LOCK(cs);
828+
indexed_transaction_set::const_iterator i = mapTx.find(hash);
829+
if (i == mapTx.end())
830+
return nullptr;
831+
return i->GetSharedTx();
805832
}
806833

807-
bool CTxMemPool::lookupFeeRate(const uint256& hash, CFeeRate& feeRate) const
834+
TxMempoolInfo CTxMemPool::info(const uint256& hash) const
808835
{
809836
LOCK(cs);
810837
indexed_transaction_set::const_iterator i = mapTx.find(hash);
811838
if (i == mapTx.end())
812-
return false;
813-
feeRate = CFeeRate(i->GetFee(), i->GetTxSize());
814-
return true;
839+
return TxMempoolInfo();
840+
return TxMempoolInfo{i->GetSharedTx(), i->GetTime(), CFeeRate(i->GetFee(), i->GetTxSize())};
815841
}
816842

817843
CFeeRate CTxMemPool::estimateFee(int nBlocks) const
@@ -924,9 +950,9 @@ bool CCoinsViewMemPool::GetCoins(const uint256 &txid, CCoins &coins) const {
924950
// If an entry in the mempool exists, always return that one, as it's guaranteed to never
925951
// conflict with the underlying cache, and it cannot have pruned entries (as it contains full)
926952
// transactions. First checking the underlying cache risks returning a pruned entry instead.
927-
CTransaction tx;
928-
if (mempool.lookup(txid, tx)) {
929-
coins = CCoins(tx, MEMPOOL_HEIGHT);
953+
shared_ptr<const CTransaction> ptx = mempool.get(txid);
954+
if (ptx) {
955+
coins = CCoins(*ptx, MEMPOOL_HEIGHT);
930956
return true;
931957
}
932958
return (base->GetCoins(txid, coins) && !coins.IsPruned());

0 commit comments

Comments
 (0)