diff options
author | Wladimir J. van der Laan <laanwj@gmail.com> | 2016-10-25 14:00:27 +0200 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@gmail.com> | 2016-10-25 14:21:57 +0200 |
commit | 9bdf5269f8860b41455df1c6d82dac0d9c858e8f (patch) | |
tree | bb3a0d029a53eed9825d2851fa2403db1c690b11 | |
parent | e077e00303841ab93f4b59ecba69eea6bf707475 (diff) | |
parent | 0334430b396acd1b1c248ad98c6fdbd50c4c4309 (diff) | |
download | bitcoin-9bdf5269f8860b41455df1c6d82dac0d9c858e8f.tar.xz |
Merge #8515: A few mempool removal optimizations
0334430 Add some missing includes (Pieter Wuille)
4100499 Return shared_ptr<CTransaction> from mempool removes (Pieter Wuille)
51f2783 Make removed and conflicted arguments optional to remove (Pieter Wuille)
f48211b Bypass removeRecursive in removeForReorg (Pieter Wuille)
-rw-r--r-- | src/main.cpp | 15 | ||||
-rw-r--r-- | src/test/blockencodings_tests.cpp | 4 | ||||
-rw-r--r-- | src/test/mempool_tests.cpp | 30 | ||||
-rw-r--r-- | src/test/miner_tests.cpp | 3 | ||||
-rw-r--r-- | src/test/policyestimator_tests.cpp | 11 | ||||
-rw-r--r-- | src/txmempool.cpp | 27 | ||||
-rw-r--r-- | src/txmempool.h | 11 |
7 files changed, 51 insertions, 50 deletions
diff --git a/src/main.cpp b/src/main.cpp index 9c0f25950a..5e17ec6251 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2768,10 +2768,9 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara std::vector<uint256> vHashUpdate; BOOST_FOREACH(const CTransaction &tx, block.vtx) { // ignore validation errors in resurrected transactions - list<CTransaction> removed; CValidationState stateDummy; if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, NULL, true)) { - mempool.removeRecursive(tx, removed); + mempool.removeRecursive(tx); } else if (mempool.exists(tx.GetHash())) { vHashUpdate.push_back(tx.GetHash()); } @@ -2804,7 +2803,7 @@ static int64_t nTimePostConnect = 0; * Connect a new block to chainActive. pblock is either NULL or a pointer to a CBlock * corresponding to pindexNew, to bypass loading it again from disk. */ -bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const CBlock* pblock, std::list<CTransaction> &txConflicted, std::vector<std::tuple<CTransaction,CBlockIndex*,int>> &txChanged) +bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const CBlock* pblock, std::vector<std::shared_ptr<const CTransaction>> &txConflicted, std::vector<std::tuple<CTransaction,CBlockIndex*,int>> &txChanged) { assert(pindexNew->pprev == chainActive.Tip()); // Read block from disk. @@ -2840,7 +2839,7 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, int64_t nTime5 = GetTimeMicros(); nTimeChainState += nTime5 - nTime4; LogPrint("bench", " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001); // Remove conflicting transactions from the mempool.; - mempool.removeForBlock(pblock->vtx, pindexNew->nHeight, txConflicted, !IsInitialBlockDownload()); + mempool.removeForBlock(pblock->vtx, pindexNew->nHeight, &txConflicted, !IsInitialBlockDownload()); // Update chainActive & related variables. UpdateTip(pindexNew, chainparams); @@ -2927,7 +2926,7 @@ static void PruneBlockIndexCandidates() { * Try to make some progress towards making pindexMostWork the active block. * pblock is either NULL or a pointer to a CBlock corresponding to pindexMostWork. */ -static bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const CBlock* pblock, bool& fInvalidFound, std::list<CTransaction>& txConflicted, std::vector<std::tuple<CTransaction,CBlockIndex*,int>>& txChanged) +static bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const CBlock* pblock, bool& fInvalidFound, std::vector<std::shared_ptr<const CTransaction>>& txConflicted, std::vector<std::tuple<CTransaction,CBlockIndex*,int>>& txChanged) { AssertLockHeld(cs_main); const CBlockIndex *pindexOldTip = chainActive.Tip(); @@ -3038,7 +3037,7 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, break; const CBlockIndex *pindexFork; - std::list<CTransaction> txConflicted; + std::vector<std::shared_ptr<const CTransaction>> txConflicted; bool fInitialDownload; { LOCK(cs_main); @@ -3069,9 +3068,9 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, // throw all transactions though the signal-interface // while _not_ holding the cs_main lock - BOOST_FOREACH(const CTransaction &tx, txConflicted) + for(std::shared_ptr<const CTransaction> tx : txConflicted) { - GetMainSignals().SyncTransaction(tx, pindexNewTip, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); + GetMainSignals().SyncTransaction(*tx, pindexNewTip, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); } // ... and about transactions that got confirmed: for(unsigned int i = 0; i < txChanged.size(); i++) diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 7530b013bd..b0d9184816 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -80,8 +80,8 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); - std::list<CTransaction> removed; - pool.removeRecursive(block.vtx[2], removed); + std::vector<std::shared_ptr<const CTransaction>> removed; + pool.removeRecursive(block.vtx[2], &removed); BOOST_CHECK_EQUAL(removed.size(), 1); CBlock block2; diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 033a50f94f..555d36faac 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -55,15 +55,15 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) CTxMemPool testPool(CFeeRate(0)); - std::list<CTransaction> removed; + std::vector<std::shared_ptr<const CTransaction>> removed; // Nothing in pool, remove should do nothing: - testPool.removeRecursive(txParent, removed); + testPool.removeRecursive(txParent, &removed); BOOST_CHECK_EQUAL(removed.size(), 0); // Just the parent: testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); - testPool.removeRecursive(txParent, removed); + testPool.removeRecursive(txParent, &removed); BOOST_CHECK_EQUAL(removed.size(), 1); removed.clear(); @@ -75,16 +75,16 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i])); } // Remove Child[0], GrandChild[0] should be removed: - testPool.removeRecursive(txChild[0], removed); + testPool.removeRecursive(txChild[0], &removed); BOOST_CHECK_EQUAL(removed.size(), 2); removed.clear(); // ... make sure grandchild and child are gone: - testPool.removeRecursive(txGrandChild[0], removed); + testPool.removeRecursive(txGrandChild[0], &removed); BOOST_CHECK_EQUAL(removed.size(), 0); - testPool.removeRecursive(txChild[0], removed); + testPool.removeRecursive(txChild[0], &removed); BOOST_CHECK_EQUAL(removed.size(), 0); // Remove parent, all children/grandchildren should go: - testPool.removeRecursive(txParent, removed); + testPool.removeRecursive(txParent, &removed); BOOST_CHECK_EQUAL(removed.size(), 5); BOOST_CHECK_EQUAL(testPool.size(), 0); removed.clear(); @@ -97,7 +97,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) } // Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be // put into the mempool (maybe because it is non-standard): - testPool.removeRecursive(txParent, removed); + testPool.removeRecursive(txParent, &removed); BOOST_CHECK_EQUAL(removed.size(), 6); BOOST_CHECK_EQUAL(testPool.size(), 0); removed.clear(); @@ -281,12 +281,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) BOOST_CHECK_EQUAL(pool.size(), 10); // Now try removing tx10 and verify the sort order returns to normal - std::list<CTransaction> removed; - pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), removed); + pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx()); CheckSort<descendant_score>(pool, snapshotOrder); - pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), removed); - pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), removed); + pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx()); + pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx()); /* Now check the sort on the mining score index. * Final order should be: * @@ -413,8 +412,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) /* after tx6 is mined, tx7 should move up in the sort */ std::vector<CTransaction> vtx; vtx.push_back(tx6); - std::list<CTransaction> dummy; - pool.removeForBlock(vtx, 1, dummy, false); + pool.removeForBlock(vtx, 1, NULL, false); sortedOrder.erase(sortedOrder.begin()+1); sortedOrder.pop_back(); @@ -549,12 +547,12 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool)); std::vector<CTransaction> vtx; - std::list<CTransaction> conflicts; + std::vector<std::shared_ptr<const CTransaction>> conflicts; SetMockTime(42); SetMockTime(42 + CTxMemPool::ROLLING_FEE_HALFLIFE); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000); // ... we should keep the same min fee until we get a block - pool.removeForBlock(vtx, 1, conflicts); + pool.removeForBlock(vtx, 1); SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/2); // ... then feerate should drop 1/2 each halflife diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index d6d7b5716e..a94979fd77 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -137,8 +137,7 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, // Test that packages above the min relay fee do get included, even if one // of the transactions is below the min relay fee // Remove the low fee transaction and replace with a higher fee transaction - std::list<CTransaction> dummy; - mempool.removeRecursive(tx, dummy); + mempool.removeRecursive(tx); tx.vout[0].nValue -= 2; // Now we should be just over the min relay fee hashLowFeeTx = tx.GetHash(); mempool.addUnchecked(hashLowFeeTx, entry.Fee(feeToUse+2).FromTx(tx)); diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 5c902387f1..f57c24270c 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -46,7 +46,6 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (unsigned int i = 0; i < 128; i++) garbage.push_back('X'); CMutableTransaction tx; - std::list<CTransaction> dummyConflicted; tx.vin.resize(1); tx.vin[0].scriptSig = garbage; tx.vout.resize(1); @@ -81,7 +80,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) txHashes[9-h].pop_back(); } } - mpool.removeForBlock(block, ++blocknum, dummyConflicted); + mpool.removeForBlock(block, ++blocknum); block.clear(); if (blocknum == 30) { // At this point we should need to combine 5 buckets to get enough data points @@ -125,7 +124,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) // Mine 50 more blocks with no transactions happening, estimates shouldn't change // We haven't decayed the moving average enough so we still have enough data points in every bucket while (blocknum < 250) - mpool.removeForBlock(block, ++blocknum, dummyConflicted); + mpool.removeForBlock(block, ++blocknum); for (int i = 1; i < 10;i++) { BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() < origFeeEst[i-1] + deltaFee); @@ -146,7 +145,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) txHashes[j].push_back(hash); } } - mpool.removeForBlock(block, ++blocknum, dummyConflicted); + mpool.removeForBlock(block, ++blocknum); } int answerFound; @@ -167,7 +166,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) txHashes[j].pop_back(); } } - mpool.removeForBlock(block, 265, dummyConflicted); + mpool.removeForBlock(block, 265); block.clear(); for (int i = 1; i < 10;i++) { BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee); @@ -187,7 +186,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) block.push_back(*ptx); } } - mpool.removeForBlock(block, ++blocknum, dummyConflicted); + mpool.removeForBlock(block, ++blocknum); block.clear(); } for (int i = 1; i < 10; i++) { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 0f1c166abc..e5d28ac2ea 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -503,7 +503,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants } } -void CTxMemPool::removeRecursive(const CTransaction &origTx, std::list<CTransaction>& removed) +void CTxMemPool::removeRecursive(const CTransaction &origTx, std::vector<std::shared_ptr<const CTransaction>>* removed) { // Remove transaction from memory pool { @@ -530,8 +530,10 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, std::list<CTransact BOOST_FOREACH(txiter it, txToRemove) { CalculateDescendants(it, setAllRemoves); } - BOOST_FOREACH(txiter it, setAllRemoves) { - removed.push_back(it->GetTx()); + if (removed) { + BOOST_FOREACH(txiter it, setAllRemoves) { + removed->emplace_back(it->GetSharedTx()); + } } RemoveStaged(setAllRemoves, false); } @@ -541,7 +543,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem { // Remove transactions spending a coinbase which are now immature and no-longer-final transactions LOCK(cs); - list<CTransaction> transactionsToRemove; + setEntries txToRemove; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { const CTransaction& tx = it->GetTx(); LockPoints lp = it->GetLockPoints(); @@ -549,16 +551,16 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem if (!CheckFinalTx(tx, flags) || !CheckSequenceLocks(tx, flags, &lp, validLP)) { // Note if CheckSequenceLocks fails the LockPoints may still be invalid // So it's critical that we remove the tx and not depend on the LockPoints. - transactionsToRemove.push_back(tx); + txToRemove.insert(it); } else if (it->GetSpendsCoinbase()) { BOOST_FOREACH(const CTxIn& txin, tx.vin) { indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); if (it2 != mapTx.end()) continue; const CCoins *coins = pcoins->AccessCoins(txin.prevout.hash); - if (nCheckFrequency != 0) assert(coins); + if (nCheckFrequency != 0) assert(coins); if (!coins || (coins->IsCoinBase() && ((signed long)nMemPoolHeight) - coins->nHeight < COINBASE_MATURITY)) { - transactionsToRemove.push_back(tx); + txToRemove.insert(it); break; } } @@ -567,13 +569,14 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem mapTx.modify(it, update_lock_points(lp)); } } - BOOST_FOREACH(const CTransaction& tx, transactionsToRemove) { - list<CTransaction> removed; - removeRecursive(tx, removed); + setEntries setAllRemoves; + for (txiter it : txToRemove) { + CalculateDescendants(it, setAllRemoves); } + RemoveStaged(setAllRemoves, false); } -void CTxMemPool::removeConflicts(const CTransaction &tx, std::list<CTransaction>& removed) +void CTxMemPool::removeConflicts(const CTransaction &tx, std::vector<std::shared_ptr<const CTransaction>>* removed) { // Remove transactions which depend on inputs of tx, recursively LOCK(cs); @@ -594,7 +597,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::list<CTransaction> * Called when a block is connected. Removes from mempool and updates the miner fee estimator. */ void CTxMemPool::removeForBlock(const std::vector<CTransaction>& vtx, unsigned int nBlockHeight, - std::list<CTransaction>& conflicts, bool fCurrentEstimate) + std::vector<std::shared_ptr<const CTransaction>>* conflicts, bool fCurrentEstimate) { LOCK(cs); std::vector<CTxMemPoolEntry> entries; diff --git a/src/txmempool.h b/src/txmempool.h index 1763930ba0..bb2638c3b7 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -6,9 +6,12 @@ #ifndef BITCOIN_TXMEMPOOL_H #define BITCOIN_TXMEMPOOL_H -#include <list> #include <memory> #include <set> +#include <map> +#include <vector> +#include <utility> +#include <string> #include "amount.h" #include "coins.h" @@ -521,11 +524,11 @@ public: bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool fCurrentEstimate = true); bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true); - void removeRecursive(const CTransaction &tx, std::list<CTransaction>& removed); + void removeRecursive(const CTransaction &tx, std::vector<std::shared_ptr<const CTransaction>>* removed = NULL); void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); - void removeConflicts(const CTransaction &tx, std::list<CTransaction>& removed); + void removeConflicts(const CTransaction &tx, std::vector<std::shared_ptr<const CTransaction>>* removed = NULL); void removeForBlock(const std::vector<CTransaction>& vtx, unsigned int nBlockHeight, - std::list<CTransaction>& conflicts, bool fCurrentEstimate = true); + std::vector<std::shared_ptr<const CTransaction>>* conflicts = NULL, bool fCurrentEstimate = true); void clear(); void _clear(); //lock free bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb); |