From 76248235435d37474fb9eb17d6f897912b860a50 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 25 Dec 2020 23:25:10 -0300 Subject: [PATCH] mapNextTx: use pointer as key, simplify value Saves about 10% of application memory usage once the mempool warms up. Since the mempool is DynamicUsage-regulated, this will translate to a larger mempool in the same amount of space. Map value type: eliminate the vin index; no users of the map need to know which input of the transaction is spending the prevout. Map key type: replace the COutPoint with a pointer to a COutPoint. A COutPoint is 36 bytes, but each COutPoint is accessible from the same map entry's value. A trivial DereferencingComparator functor allows indirect map keys, but the resulting syntax is misleading: `map.find(&outpoint)`. Implement an indirectmap that acts as a wrapper to a map that uses a DereferencingComparator, supporting a syntax that accurately reflect the container's semantics: inserts and iterators use pointers since they store pointers and need them to remain constant and dereferenceable, but lookup functions take const references. Coming from btc@9805f4af7ecb6becf8a146bd845fb131ffa625c9 --- CMakeLists.txt | 1 + src/Makefile.am | 1 + src/indirectmap.h | 58 +++++++++++++++++++++++++++++++++++++++++++++++ src/memusage.h | 15 ++++++++++++ src/txmempool.cpp | 38 +++++++++++++++---------------- src/txmempool.h | 25 ++------------------ 6 files changed, 95 insertions(+), 43 deletions(-) create mode 100644 src/indirectmap.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 931aebf4b2751..6a88776965aff 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -193,6 +193,7 @@ set(SERVER_SOURCES ./src/checkpoints.cpp ./src/httprpc.cpp ./src/httpserver.cpp + ./src/indirectmap.h ./src/init.cpp ./src/interfaces/handler.cpp ./src/interfaces/wallet.cpp diff --git a/src/Makefile.am b/src/Makefile.am index 986fd6cd8a684..f2c2a834e9cad 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -189,6 +189,7 @@ BITCOIN_CORE_H = \ hash.h \ httprpc.h \ httpserver.h \ + indirectmap.h \ init.h \ interfaces/handler.h \ interfaces/wallet.h \ diff --git a/src/indirectmap.h b/src/indirectmap.h new file mode 100644 index 0000000000000..af8f23b46fc4d --- /dev/null +++ b/src/indirectmap.h @@ -0,0 +1,58 @@ +// Copyright (c) 2016-2020 The Bitcoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_INDIRECTMAP_H +#define BITCOIN_INDIRECTMAP_H + +#include + +template +struct DereferencingComparator { bool operator()(const T a, const T b) const { return *a < *b; } }; + +/* Map whose keys are pointers, but are compared by their dereferenced values. + * + * Differs from a plain std::map > in + * that methods that take a key for comparison take a K rather than taking a K* + * (taking a K* would be confusing, since it's the value rather than the address + * of the object for comparison that matters due to the dereferencing comparator). + * + * Objects pointed to by keys must not be modified in any way that changes the + * result of DereferencingComparator. + */ +template +class indirectmap { +private: + typedef std::map > base; + base m; +public: + typedef typename base::iterator iterator; + typedef typename base::const_iterator const_iterator; + typedef typename base::size_type size_type; + typedef typename base::value_type value_type; + + // passthrough (pointer interface) + std::pair insert(const value_type& value) { return m.insert(value); } + + // pass address (value interface) + iterator find(const K& key) { return m.find(&key); } + const_iterator find(const K& key) const { return m.find(&key); } + iterator lower_bound(const K& key) { return m.lower_bound(&key); } + const_iterator lower_bound(const K& key) const { return m.lower_bound(&key); } + size_type erase(const K& key) { return m.erase(&key); } + size_type count(const K& key) const { return m.count(&key); } + + // passthrough + bool empty() const { return m.empty(); } + size_type size() const { return m.size(); } + size_type max_size() const { return m.max_size(); } + void clear() { m.clear(); } + iterator begin() { return m.begin(); } + iterator end() { return m.end(); } + const_iterator begin() const { return m.begin(); } + const_iterator end() const { return m.end(); } + const_iterator cbegin() const { return m.cbegin(); } + const_iterator cend() const { return m.cend(); } +}; + +#endif // BITCOIN_INDIRECTMAP_H diff --git a/src/memusage.h b/src/memusage.h index b06421a750173..f2a56b6b9b901 100644 --- a/src/memusage.h +++ b/src/memusage.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_MEMUSAGE_H #define BITCOIN_MEMUSAGE_H +#include "indirectmap.h" #include "prevector.h" #include @@ -185,6 +186,20 @@ static inline size_t DynamicUsage(const std::shared_ptr& p) return p ? MallocUsage(sizeof(X)) + MallocUsage(sizeof(stl_shared_counter)) : 0; } +// indirectmap has underlying map with pointer as key + +template +static inline size_t DynamicUsage(const indirectmap& m) +{ + return MallocUsage(sizeof(stl_tree_node >)) * m.size(); +} + +template +static inline size_t IncrementalDynamicUsage(const indirectmap& m) +{ + return MallocUsage(sizeof(stl_tree_node >)); +} + // Boost data structures template diff --git a/src/txmempool.cpp b/src/txmempool.cpp index da62d5cc738ce..cfda1f746c5ab 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -142,11 +142,11 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector &vHashes if (it == mapTx.end()) { continue; } - std::map::iterator iter = mapNextTx.lower_bound(COutPoint(hash, 0)); + auto iter = mapNextTx.lower_bound(COutPoint(hash, 0)); // First calculate the children, and update setMemPoolChildren to // include them, and update their setMemPoolParents to include this tx. - for (; iter != mapNextTx.end() && iter->first.hash == hash; ++iter) { - const uint256 &childHash = iter->second.ptx->GetHash(); + for (; iter != mapNextTx.end() && iter->first->hash == hash; ++iter) { + const uint256 &childHash = iter->second->GetHash(); txiter childIter = mapTx.find(childHash); assert(childIter != mapTx.end()); // We can skip updating entries we've encountered before or that @@ -404,7 +404,7 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, std::set setParentTransactions; if(!tx.HasZerocoinSpendInputs()) { for (unsigned int i = 0; i < tx.vin.size(); i++) { - mapNextTx[tx.vin[i].prevout] = CInPoint(&tx, i); + mapNextTx.insert(std::make_pair(&tx.vin[i].prevout, newit->GetSharedTx())); setParentTransactions.insert(tx.vin[i].prevout.hash); } } @@ -504,10 +504,10 @@ void CTxMemPool::removeRecursive(const CTransaction& origTx, std::list::iterator it = mapNextTx.find(COutPoint(origTx.GetHash(), i)); + auto it = mapNextTx.find(COutPoint(origTx.GetHash(), i)); if (it == mapNextTx.end()) continue; - txiter nextit = mapTx.find(it->second.ptx->GetHash()); + txiter nextit = mapTx.find(it->second->GetHash()); assert(nextit != mapTx.end()); txToRemove.insert(nextit); } @@ -584,9 +584,9 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::list result; LOCK(cs); for (const CTxIn& txin : tx.vin) { - std::map::iterator it = mapNextTx.find(txin.prevout); + auto it = mapNextTx.find(txin.prevout); if (it != mapNextTx.end()) { - const CTransaction& txConflict = *it->second.ptx; + const CTransaction& txConflict = *it->second; if (txConflict != tx) { removeRecursive(txConflict, removed); } @@ -704,10 +704,10 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const } // Check whether its inputs are marked in mapNextTx. if(!fHasZerocoinSpends) { - std::map::const_iterator it3 = mapNextTx.find(txin.prevout); + auto it3 = mapNextTx.find(txin.prevout); assert(it3 != mapNextTx.end()); - assert(it3->second.ptx == &tx); - assert(it3->second.n == i); + assert(it3->first == &txin.prevout); + assert(*it3->second == tx); } else { fDependsWait=false; } @@ -746,10 +746,10 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const // Check children against mapNextTx if (!fHasZerocoinSpends) { CTxMemPool::setEntries setChildrenCheck; - std::map::const_iterator iter = mapNextTx.lower_bound(COutPoint(tx.GetHash(), 0)); + auto iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0)); int64_t childSizes = 0; - for (; iter != mapNextTx.end() && iter->first.hash == tx.GetHash(); ++iter) { - txiter childit = mapTx.find(iter->second.ptx->GetHash()); + for (; iter != mapNextTx.end() && iter->first->hash == tx.GetHash(); ++iter) { + txiter childit = mapTx.find(iter->second->GetHash()); assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions if (setChildrenCheck.insert(childit).second) { childSizes += childit->GetTxSize(); @@ -787,14 +787,12 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const stepsSinceLastRemove = 0; } } - for (std::map::const_iterator it = mapNextTx.begin(); it != mapNextTx.end(); it++) { - const uint256& hash = it->second.ptx->GetHash(); + for (auto it = mapNextTx.cbegin(); it != mapNextTx.cend(); it++) { + const uint256& hash = it->second->GetHash(); indexed_transaction_set::const_iterator it2 = mapTx.find(hash); - const CTransaction& tx = it2->GetTx(); + const CTransactionRef& tx = it2->GetSharedTx(); assert(it2 != mapTx.end()); - assert(&tx == it->second.ptx); - assert(tx.vin.size() > it->second.n); - assert(it->first == it->second.ptx->vin[it->second.n].prevout); + assert(tx == it->second); } // Consistency check for sapling nullifiers diff --git a/src/txmempool.h b/src/txmempool.h index 73cf2ef5a1251..715720eac7f78 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -13,6 +13,7 @@ #include "amount.h" #include "coins.h" +#include "indirectmap.h" #include "policy/feerate.h" #include "primitives/transaction.h" #include "sync.h" @@ -285,28 +286,6 @@ struct ancestor_score {}; class CBlockPolicyEstimator; -/** An inpoint - a combination of a transaction and an index n into its vin */ -class CInPoint -{ -public: - const CTransaction* ptx; - uint32_t n; - - CInPoint() { SetNull(); } - CInPoint(const CTransaction* ptxIn, uint32_t nIn) - { - ptx = ptxIn; - n = nIn; - } - void SetNull() - { - ptx = NULL; - n = (uint32_t)-1; - } - bool IsNull() const { return (ptx == NULL && n == (uint32_t)-1); } - size_t DynamicMemoryUsage() const { return 0; } -}; - /** * CTxMemPool stores valid-according-to-the-current-best-chain * transactions that may be included in the next block. @@ -498,7 +477,7 @@ class CTxMemPool void UpdateChild(txiter entry, txiter child, bool add); public: - std::map mapNextTx; + indirectmap mapNextTx; std::map > mapDeltas; /** Create a new CTxMemPool.