From 34628a18070064e75b35f28fd6a43d5c23832eb8 Mon Sep 17 00:00:00 2001 From: Ashley Holman Date: Wed, 24 Jun 2015 03:32:20 -0500 Subject: [PATCH] TxMemPool: Change mapTx to a boost::multi_index_container Indexes on: - Tx Hash - Fee Rate (fee-per-kb) --- src/miner.cpp | 8 +++--- src/rpcblockchain.cpp | 5 ++-- src/test/mempool_tests.cpp | 52 ++++++++++++++++++++++++++++++++++++++ src/txmempool.cpp | 52 ++++++++++++++++++++------------------ src/txmempool.h | 42 +++++++++++++++++++++++++++++- 5 files changed, 127 insertions(+), 32 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index 9dd1d459b..b2a356e52 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -158,10 +158,10 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) // This vector will be sorted into a priority queue: vector vecPriority; vecPriority.reserve(mempool.mapTx.size()); - for (map::iterator mi = mempool.mapTx.begin(); + for (CTxMemPool::indexed_transaction_set::iterator mi = mempool.mapTx.begin(); mi != mempool.mapTx.end(); ++mi) { - const CTransaction& tx = mi->second.GetTx(); + const CTransaction& tx = mi->GetTx(); if (tx.IsCoinBase() || !IsFinalTx(tx, nHeight, pblock->nTime)) continue; @@ -196,7 +196,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) } mapDependers[txin.prevout.hash].push_back(porphan); porphan->setDependsOn.insert(txin.prevout.hash); - nTotalIn += mempool.mapTx[txin.prevout.hash].GetTx().vout[txin.prevout.n].nValue; + nTotalIn += mempool.mapTx.find(txin.prevout.hash)->GetTx().vout[txin.prevout.n].nValue; continue; } const CCoins* coins = view.AccessCoins(txin.prevout.hash); @@ -226,7 +226,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) porphan->feeRate = feeRate; } else - vecPriority.push_back(TxPriority(dPriority, feeRate, &mi->second.GetTx())); + vecPriority.push_back(TxPriority(dPriority, feeRate, &(mi->GetTx()))); } // Collect transactions into block diff --git a/src/rpcblockchain.cpp b/src/rpcblockchain.cpp index e6751de96..a1da31b61 100644 --- a/src/rpcblockchain.cpp +++ b/src/rpcblockchain.cpp @@ -181,10 +181,9 @@ UniValue mempoolToJSON(bool fVerbose = false) { LOCK(mempool.cs); UniValue o(UniValue::VOBJ); - BOOST_FOREACH(const PAIRTYPE(uint256, CTxMemPoolEntry)& entry, mempool.mapTx) + BOOST_FOREACH(const CTxMemPoolEntry& e, mempool.mapTx) { - const uint256& hash = entry.first; - const CTxMemPoolEntry& e = entry.second; + const uint256& hash = e.GetTx().GetHash(); UniValue info(UniValue::VOBJ); info.push_back(Pair("size", (int)e.GetTxSize())); info.push_back(Pair("fee", ValueFromAmount(e.GetFee()))); diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 2439689d7..7f82a61bf 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -100,4 +100,56 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) removed.clear(); } +BOOST_AUTO_TEST_CASE(MempoolIndexingTest) +{ + CTxMemPool pool(CFeeRate(0)); + + /* 3rd highest fee */ + CMutableTransaction tx1 = CMutableTransaction(); + tx1.vout.resize(1); + tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx1.vout[0].nValue = 10 * COIN; + pool.addUnchecked(tx1.GetHash(), CTxMemPoolEntry(tx1, 10000LL, 0, 10.0, 1, true)); + + /* highest fee */ + CMutableTransaction tx2 = CMutableTransaction(); + tx2.vout.resize(1); + tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx2.vout[0].nValue = 2 * COIN; + pool.addUnchecked(tx2.GetHash(), CTxMemPoolEntry(tx2, 20000LL, 0, 9.0, 1, true)); + + /* lowest fee */ + CMutableTransaction tx3 = CMutableTransaction(); + tx3.vout.resize(1); + tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx3.vout[0].nValue = 5 * COIN; + pool.addUnchecked(tx3.GetHash(), CTxMemPoolEntry(tx3, 0LL, 0, 100.0, 1, true)); + + /* 2nd highest fee */ + CMutableTransaction tx4 = CMutableTransaction(); + tx4.vout.resize(1); + tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx4.vout[0].nValue = 6 * COIN; + pool.addUnchecked(tx4.GetHash(), CTxMemPoolEntry(tx4, 15000LL, 0, 1.0, 1, true)); + + /* equal fee rate to tx1, but newer */ + CMutableTransaction tx5 = CMutableTransaction(); + tx5.vout.resize(1); + tx5.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx5.vout[0].nValue = 11 * COIN; + pool.addUnchecked(tx5.GetHash(), CTxMemPoolEntry(tx5, 10000LL, 1, 10.0, 1, true)); + + // there should be 4 transactions in the mempool + BOOST_CHECK_EQUAL(pool.size(), 5); + + // Check the fee-rate index is in order, should be tx2, tx4, tx1, tx5, tx3 + CTxMemPool::indexed_transaction_set::nth_index<1>::type::iterator it = pool.mapTx.get<1>().begin(); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx2.GetHash().ToString()); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx4.GetHash().ToString()); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx1.GetHash().ToString()); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx5.GetHash().ToString()); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx3.GetHash().ToString()); + BOOST_CHECK(it == pool.mapTx.get<1>().end()); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txmempool.cpp b/src/txmempool.cpp index c921dae45..c410cd083 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -32,6 +32,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); nModSize = tx.CalculateModifiedSize(nTxSize); nUsageSize = RecursiveDynamicUsage(tx); + feeRate = CFeeRate(nFee, nTxSize); } CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other) @@ -96,8 +97,8 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, // Used by main.cpp AcceptToMemoryPool(), which DOES do // all the appropriate checks. LOCK(cs); - mapTx[hash] = entry; - const CTransaction& tx = mapTx[hash].GetTx(); + mapTx.insert(entry); + const CTransaction& tx = mapTx.find(hash)->GetTx(); for (unsigned int i = 0; i < tx.vin.size(); i++) mapNextTx[tx.vin[i].prevout] = CInPoint(&tx, i); nTransactionsUpdated++; @@ -134,7 +135,7 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem txToRemove.pop_front(); if (!mapTx.count(hash)) continue; - const CTransaction& tx = mapTx[hash].GetTx(); + const CTransaction& tx = mapTx.find(hash)->GetTx(); if (fRecursive) { for (unsigned int i = 0; i < tx.vout.size(); i++) { std::map::iterator it = mapNextTx.find(COutPoint(hash, i)); @@ -147,8 +148,8 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem mapNextTx.erase(txin.prevout); removed.push_back(tx); - totalTxSize -= mapTx[hash].GetTxSize(); - cachedInnerUsage -= mapTx[hash].DynamicMemoryUsage(); + totalTxSize -= mapTx.find(hash)->GetTxSize(); + cachedInnerUsage -= mapTx.find(hash)->DynamicMemoryUsage(); mapTx.erase(hash); nTransactionsUpdated++; minerPolicyEstimator->removeTx(hash); @@ -161,10 +162,10 @@ void CTxMemPool::removeCoinbaseSpends(const CCoinsViewCache *pcoins, unsigned in // Remove transactions spending a coinbase which are now immature LOCK(cs); list transactionsToRemove; - for (std::map::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { - const CTransaction& tx = it->second.GetTx(); + for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { + const CTransaction& tx = it->GetTx(); BOOST_FOREACH(const CTxIn& txin, tx.vin) { - std::map::const_iterator it2 = mapTx.find(txin.prevout.hash); + indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); if (it2 != mapTx.end()) continue; const CCoins *coins = pcoins->AccessCoins(txin.prevout.hash); @@ -209,8 +210,10 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigned i BOOST_FOREACH(const CTransaction& tx, vtx) { uint256 hash = tx.GetHash(); - if (mapTx.count(hash)) - entries.push_back(mapTx[hash]); + + indexed_transaction_set::iterator i = mapTx.find(hash); + if (i != mapTx.end()) + entries.push_back(*i); } BOOST_FOREACH(const CTransaction& tx, vtx) { @@ -247,17 +250,17 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const LOCK(cs); list waitingOnDependants; - for (std::map::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { + for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { unsigned int i = 0; - checkTotal += it->second.GetTxSize(); - innerUsage += it->second.DynamicMemoryUsage(); - const CTransaction& tx = it->second.GetTx(); + checkTotal += it->GetTxSize(); + innerUsage += it->DynamicMemoryUsage(); + const CTransaction& tx = it->GetTx(); bool fDependsWait = false; BOOST_FOREACH(const CTxIn &txin, tx.vin) { // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's. - std::map::const_iterator it2 = mapTx.find(txin.prevout.hash); + indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); if (it2 != mapTx.end()) { - const CTransaction& tx2 = it2->second.GetTx(); + const CTransaction& tx2 = it2->GetTx(); assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull()); fDependsWait = true; } else { @@ -272,7 +275,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const i++; } if (fDependsWait) - waitingOnDependants.push_back(&it->second); + waitingOnDependants.push_back(&(*it)); else { CValidationState state; assert(CheckInputs(tx, state, mempoolDuplicate, false, 0, false, NULL)); @@ -296,8 +299,8 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const } for (std::map::const_iterator it = mapNextTx.begin(); it != mapNextTx.end(); it++) { uint256 hash = it->second.ptx->GetHash(); - map::const_iterator it2 = mapTx.find(hash); - const CTransaction& tx = it2->second.GetTx(); + indexed_transaction_set::const_iterator it2 = mapTx.find(hash); + const CTransaction& tx = it2->GetTx(); assert(it2 != mapTx.end()); assert(&tx == it->second.ptx); assert(tx.vin.size() > it->second.n); @@ -314,16 +317,16 @@ void CTxMemPool::queryHashes(vector& vtxid) LOCK(cs); vtxid.reserve(mapTx.size()); - for (map::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) - vtxid.push_back((*mi).first); + for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) + vtxid.push_back(mi->GetTx().GetHash()); } bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const { LOCK(cs); - map::const_iterator i = mapTx.find(hash); + indexed_transaction_set::const_iterator i = mapTx.find(hash); if (i == mapTx.end()) return false; - result = i->second.GetTx(); + result = i->GetTx(); return true; } @@ -429,5 +432,6 @@ bool CCoinsViewMemPool::HaveCoins(const uint256 &txid) const { size_t CTxMemPool::DynamicMemoryUsage() const { LOCK(cs); - return memusage::DynamicUsage(mapTx) + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + cachedInnerUsage; + // Estimate the overhead of mapTx to be 6 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 6 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + cachedInnerUsage; } diff --git a/src/txmempool.h b/src/txmempool.h index ea36ce1ad..6b6b05454 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -13,6 +13,10 @@ #include "primitives/transaction.h" #include "sync.h" +#undef foreach +#include "boost/multi_index_container.hpp" +#include "boost/multi_index/ordered_index.hpp" + class CAutoFile; inline double AllowFreeThreshold() @@ -41,6 +45,7 @@ private: size_t nTxSize; //! ... and avoid recomputing tx size size_t nModSize; //! ... and modified size for priority size_t nUsageSize; //! ... and total memory usage + CFeeRate feeRate; //! ... and fee per kB int64_t nTime; //! Local time when entering the mempool double dPriority; //! Priority when entering the mempool unsigned int nHeight; //! Chain height when entering the mempool @@ -55,6 +60,7 @@ public: const CTransaction& GetTx() const { return this->tx; } double GetPriority(unsigned int currentHeight) const; CAmount GetFee() const { return nFee; } + CFeeRate GetFeeRate() const { return feeRate; } size_t GetTxSize() const { return nTxSize; } int64_t GetTime() const { return nTime; } unsigned int GetHeight() const { return nHeight; } @@ -62,6 +68,27 @@ public: size_t DynamicMemoryUsage() const { return nUsageSize; } }; +// extracts a TxMemPoolEntry's transaction hash +struct mempoolentry_txid +{ + typedef uint256 result_type; + result_type operator() (const CTxMemPoolEntry &entry) const + { + return entry.GetTx().GetHash(); + } +}; + +class CompareTxMemPoolEntryByFee +{ +public: + bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) + { + if (a.GetFeeRate() == b.GetFeeRate()) + return a.GetTime() < b.GetTime(); + return a.GetFeeRate() > b.GetFeeRate(); + } +}; + class CBlockPolicyEstimator; /** An inpoint - a combination of a transaction and an index n into its vin */ @@ -99,8 +126,21 @@ private: uint64_t cachedInnerUsage; //! sum of dynamic memory usage of all the map elements (NOT the maps themselves) public: + typedef boost::multi_index_container< + CTxMemPoolEntry, + boost::multi_index::indexed_by< + // sorted by txid + boost::multi_index::ordered_unique, + // sorted by fee rate + boost::multi_index::ordered_non_unique< + boost::multi_index::identity, + CompareTxMemPoolEntryByFee + > + > + > indexed_transaction_set; + mutable CCriticalSection cs; - std::map mapTx; + indexed_transaction_set mapTx; std::map mapNextTx; std::map > mapDeltas;