It's probably not too important, but this is a bit of a performance and memory regression to construct the vector of TxMempoolInfo when really we only need the underlying txref.
I think it could make sense to add std::vector<const CTxMemPoolEntry*> entryAll() const EXCLUSIVE_LOCKS_REQUIRED(cs); to reduce memory and computational overhead and as a bonus make the code a bit easier to read when we just want to iterate over the mempool entries?
<details>
<summary>git diff on 2be5e799ba</summary>
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index a73530c142..155546c3a1 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -807,8 +807,8 @@ public:
{
if (!m_node.mempool) return;
LOCK2(::cs_main, m_node.mempool->cs);
- for (const auto& txinfo : m_node.mempool->infoAll()) {
- notifications.transactionAddedToMempool(txinfo.tx);
+ for (const auto& entry : m_node.mempool->entryAll()) {
+ notifications.transactionAddedToMempool(Assert(entry)->GetSharedTx());
}
}
bool hasAssumedValidChain() override
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 4093eb99af..82fe394b45 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -343,16 +343,13 @@ UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose, bool include_mempoo
}
LOCK(pool.cs);
UniValue o(UniValue::VOBJ);
- for (const auto& txinfo : pool.infoAll()) {
- const Txid& hash = txinfo.tx->GetHash();
- const auto entry{pool.GetEntry(hash)};
- CHECK_NONFATAL(entry != nullptr);
+ for (const auto& entry : pool.entryAll()) {
UniValue info(UniValue::VOBJ);
- entryToJSON(pool, info, *entry);
+ entryToJSON(pool, info, *CHECK_NONFATAL(entry));
// Mempool has unique entries so there is no advantage in using
// UniValue::pushKV, which checks if the key already exists in O(N).
// UniValue::pushKVEnd is used instead which currently is O(1).
- o.pushKVEnd(hash.ToString(), info);
+ o.pushKVEnd(entry->GetTx().ToString(), info);
}
return o;
} else {
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 684116c011..542c904a1a 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -838,6 +838,20 @@ static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator
return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), it->GetFee(), it->GetTxSize(), it->GetModifiedFee() - it->GetFee()};
}
+std::vector<const CTxMemPoolEntry*> CTxMemPool::entryAll() const
+{
+ AssertLockHeld(cs);
+ auto iters{GetSortedDepthAndScore()};
+
+ std::vector<const CTxMemPoolEntry*> ret;
+ ret.reserve(mapTx.size());
+ for (const auto& it : iters) {
+ ret.push_back(&(*it));
+ }
+
+ return ret;
+}
+
std::vector<TxMempoolInfo> CTxMemPool::infoAll() const
{
LOCK(cs);
diff --git a/src/txmempool.h b/src/txmempool.h
index e15b2972a4..7eb205b2df 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -697,6 +697,7 @@ public:
/** Returns info for a transaction if its entry_sequence < last_sequence */
TxMempoolInfo info_for_relay(const GenTxid& gtxid, uint64_t last_sequence) const;
+ std::vector<const CTxMemPoolEntry*> entryAll() const EXCLUSIVE_LOCKS_REQUIRED(cs);
std::vector<TxMempoolInfo> infoAll() const;
size_t DynamicMemoryUsage() const;
</details>
