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

@@ -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

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 \

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 <map>
+
+template <class T>
+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<const K*, T, DereferencingComparator<K*> > 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 K, class T>
+class indirectmap {
+private:
+    typedef std::map<const K*, T, DereferencingComparator<const K*> > 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<iterator, bool> 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

src/memusage.h

@@ -5,6 +5,7 @@
 #ifndef BITCOIN_MEMUSAGE_H
 #define BITCOIN_MEMUSAGE_H
 
+#include "indirectmap.h"
 #include "prevector.h"
 
 #include <stdlib.h>
@@ -185,6 +186,20 @@ static inline size_t DynamicUsage(const std::shared_ptr<X>& p)
     return p ? MallocUsage(sizeof(X)) + MallocUsage(sizeof(stl_shared_counter)) : 0;
 }
 
+// indirectmap has underlying map with pointer as key
+
+template<typename X, typename Y>
+static inline size_t DynamicUsage(const indirectmap<X, Y>& m)
+{
+    return MallocUsage(sizeof(stl_tree_node<std::pair<const X*, Y> >)) * m.size();
+}
+
+template<typename X, typename Y>
+static inline size_t IncrementalDynamicUsage(const indirectmap<X, Y>& m)
+{
+    return MallocUsage(sizeof(stl_tree_node<std::pair<const X*, Y> >));
+}
+
 // Boost data structures
 
 template<typename X>

src/txmempool.cpp

@@ -142,11 +142,11 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashes
         if (it == mapTx.end()) {
             continue;
         }
-        std::map<COutPoint, CInPoint>::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<uint256> 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<CTransact
             // happen during chain re-orgs if origTx isn't re-accepted into
             // the mempool for any reason.
             for (unsigned int i = 0; i < origTx.vout.size(); i++) {
-                std::map<COutPoint, CInPoint>::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<CTransactionR
     std::list<CTransaction> result;
     LOCK(cs);
     for (const CTxIn& txin : tx.vin) {
-        std::map<COutPoint, CInPoint>::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<COutPoint, CInPoint>::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<COutPoint, CInPoint>::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<COutPoint, CInPoint>::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

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<COutPoint, CInPoint> mapNextTx;
+    indirectmap<COutPoint, CTransactionRef> mapNextTx;
     std::map<uint256, std::pair<double, CAmount> > mapDeltas;
 
     /** Create a new CTxMemPool.