mirror/bitcoin
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#34384: Remove epoch logic from mempool

Browse Source 
40735450c0 Remove unused epochguard.h (Suhas Daftuar)
1a8494d16c Rework CTxMemPool::GetChildren() to not use epochs (Suhas Daftuar)

Pull request description:

  Since #33591, the epoch-based graph traversal optimization logic is only used for `CTxMempool::GetChildren()`, a function that is only used in RPC code and tests. Rewrite it without epochs, and remove `util/epochguard.h` itself, as that was its last use.

  This allows us to reduce per-transaction memory usage by 8 bytes, for no material loss. With the new TxGraph-based mempool implementation, I also don't foresee future uses for it, as TxGraph can do even better by using BitSet-based traversal tracking.

ACKs for top commit:
  ajtowns:
    ACK 40735450c0
  instagibbs:
    ACK 40735450c0
  l0rinc:
    code review ACK 40735450c0

Tree-SHA512: 7ce7c04835cd2425a71c4fd47f316b6fb7381caa27383de7ecc4aa81100fcf7bc5e062699b307c08e0b853b35f06710d9ac761d6e660af9f9331e708d36f2fe0
This commit is contained in:
merge-script committed 2026-01-23 15:10:54 +00:00
2 parents 891030ac8b 40735450c0
commit 5b8c204275
4 files changed
+9 -131
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

src/kernel/mempool_entry.h
-2
View File
@@ -12,7 +12,6 @@
#include <policy/settings.h>
#include <primitives/transaction.h>
#include <txgraph.h>
#include <util/epochguard.h>
#include <util/overflow.h>
#include <chrono>
@@ -138,7 +137,6 @@ public:
bool GetSpendsCoinbase() const { return spendsCoinbase; }
mutable size_t idx_randomized; //!< Index in mempool's txns_randomized
mutable Epoch::Marker m_epoch_marker; //!< epoch when last touched, useful for graph algorithms
};
using CTxMemPoolEntryRef = CTxMemPoolEntry::CTxMemPoolEntryRef;
src/txmempool.cpp
+8 -5
View File
@@ -56,15 +56,18 @@ bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp)
std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef> CTxMemPool::GetChildren(const CTxMemPoolEntry& entry) const
{
LOCK(cs);
std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef> ret;
WITH_FRESH_EPOCH(m_epoch);
auto iter = mapNextTx.lower_bound(COutPoint(entry.GetTx().GetHash(), 0));
for (; iter != mapNextTx.end() && iter->first->hash == entry.GetTx().GetHash(); ++iter) {
if (!visited(iter->second)) {
const auto& hash = entry.GetTx().GetHash();
{
LOCK(cs);
auto iter = mapNextTx.lower_bound(COutPoint(hash, 0));
for (; iter != mapNextTx.end() && iter->first->hash == hash; ++iter) {
ret.emplace_back(*(iter->second));
}
}
std::ranges::sort(ret, CompareIteratorByHash{});
auto removed = std::ranges::unique(ret, [](auto& a, auto& b) noexcept { return &a.get() == &b.get(); });
ret.erase(removed.begin(), removed.end());
return ret;
}
src/txmempool.h
+1 -22
View File
@@ -20,7 +20,6 @@
#include <primitives/transaction_identifier.h>
#include <sync.h>
#include <txgraph.h>
#include <util/epochguard.h>
#include <util/feefrac.h>
#include <util/hasher.h>
#include <util/result.h>
@@ -197,7 +196,6 @@ protected:
mutable int64_t lastRollingFeeUpdate GUARDED_BY(cs){GetTime()};
mutable bool blockSinceLastRollingFeeBump GUARDED_BY(cs){false};
mutable double rollingMinimumFeeRate GUARDED_BY(cs){0}; //!< minimum fee to get into the pool, decreases exponentially
mutable Epoch m_epoch GUARDED_BY(cs){};
// In-memory counter for external mempool tracking purposes.
// This number is incremented once every time a transaction
@@ -394,7 +392,7 @@ public:
* @param[in] vHashesToUpdate The set of txids from the
* disconnected block that have been accepted back into the mempool.
*/
void UpdateTransactionsFromBlock(const std::vector<Txid>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main) LOCKS_EXCLUDED(m_epoch);
void UpdateTransactionsFromBlock(const std::vector<Txid>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
std::vector<FeePerWeight> GetFeerateDiagram() const EXCLUSIVE_LOCKS_REQUIRED(cs);
FeePerWeight GetMainChunkFeerate(const CTxMemPoolEntry& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs) {
@@ -592,25 +590,6 @@ private:
/* Removal from the mempool also triggers removal of the entry's Ref from txgraph. */
void removeUnchecked(txiter entry, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
public:
/** visited marks a CTxMemPoolEntry as having been traversed
* during the lifetime of the most recently created Epoch::Guard
* and returns false if we are the first visitor, true otherwise.
*
* An Epoch::Guard must be held when visited is called or an assert will be
* triggered.
*
*/
bool visited(const txiter it) const EXCLUSIVE_LOCKS_REQUIRED(cs, m_epoch)
{
return m_epoch.visited(it->m_epoch_marker);
}
bool visited(std::optional<txiter> it) const EXCLUSIVE_LOCKS_REQUIRED(cs, m_epoch)
{
assert(m_epoch.guarded()); // verify guard even when it==nullopt
return !it || visited(*it);
}
/*
* CTxMemPool::ChangeSet:
*
src/util/epochguard.h
-102
View File
@@ -1,102 +0,0 @@
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#ifndef BITCOIN_UTIL_EPOCHGUARD_H
#define BITCOIN_UTIL_EPOCHGUARD_H
#include <threadsafety.h>
#include <util/macros.h>
#include <cassert>
/** Epoch: RAII-style guard for using epoch-based graph traversal algorithms.
* When walking ancestors or descendants, we generally want to avoid
* visiting the same transactions twice. Some traversal algorithms use
* std::set (or setEntries) to deduplicate the transaction we visit.
* However, use of std::set is algorithmically undesirable because it both
* adds an asymptotic factor of O(log n) to traversals cost and triggers O(n)
* more dynamic memory allocations.
* In many algorithms we can replace std::set with an internal mempool
* counter to track the time (or, "epoch") that we began a traversal, and
* check + update a per-transaction epoch for each transaction we look at to
* determine if that transaction has not yet been visited during the current
* traversal's epoch.
* Algorithms using std::set can be replaced on a one by one basis.
* Both techniques are not fundamentally incompatible across the codebase.
* Generally speaking, however, the remaining use of std::set for mempool
* traversal should be viewed as a TODO for replacement with an epoch based
* traversal, rather than a preference for std::set over epochs in that
* algorithm.
*/
class LOCKABLE Epoch
{
private:
uint64_t m_raw_epoch = 0;
bool m_guarded = false;
public:
Epoch() = default;
Epoch(const Epoch&) = delete;
Epoch& operator=(const Epoch&) = delete;
Epoch(Epoch&&) = delete;
Epoch& operator=(Epoch&&) = delete;
~Epoch() = default;
bool guarded() const { return m_guarded; }
class Marker
{
private:
uint64_t m_marker = 0;
// only allow modification via Epoch member functions
friend class Epoch;
Marker& operator=(const Marker&) = delete;
public:
Marker() = default;
Marker(const Marker&) = default;
Marker(Marker&&) = default;
Marker& operator=(Marker&&) = default;
~Marker() = default;
};
class SCOPED_LOCKABLE Guard
{
private:
Epoch& m_epoch;
public:
explicit Guard(Epoch& epoch) EXCLUSIVE_LOCK_FUNCTION(epoch) : m_epoch(epoch)
{
assert(!m_epoch.m_guarded);
++m_epoch.m_raw_epoch;
m_epoch.m_guarded = true;
}
~Guard() UNLOCK_FUNCTION()
{
assert(m_epoch.m_guarded);
++m_epoch.m_raw_epoch; // ensure clear separation between epochs
m_epoch.m_guarded = false;
}
};
bool visited(Marker& marker) const EXCLUSIVE_LOCKS_REQUIRED(*this)
{
assert(m_guarded);
if (marker.m_marker < m_raw_epoch) {
// marker is from a previous epoch, so this is its first visit
marker.m_marker = m_raw_epoch;
return false;
} else {
return true;
}
}
};
#define WITH_FRESH_EPOCH(epoch) const Epoch::Guard UNIQUE_NAME(epoch_guard_)(epoch)
#endif // BITCOIN_UTIL_EPOCHGUARD_H