diff --git a/include/xrpl/ledger/LedgerIndexMap.h b/include/xrpl/ledger/LedgerIndexMap.h deleted file mode 100644 index e4d3e32b51..0000000000 --- a/include/xrpl/ledger/LedgerIndexMap.h +++ /dev/null @@ -1,92 +0,0 @@ -#pragma once - -#include -#include -#include -#include -#include - -namespace xrpl { - -template -class LedgerIndexMap -{ -public: - LedgerIndexMap() = default; - explicit LedgerIndexMap(std::size_t reserve_capacity) - { - data_.reserve(reserve_capacity); - } - - LedgerIndexMap(LedgerIndexMap const&) = delete; - LedgerIndexMap& - operator=(LedgerIndexMap const&) = delete; - LedgerIndexMap(LedgerIndexMap&&) = delete; - LedgerIndexMap& - operator=(LedgerIndexMap&&) = delete; - - [[nodiscard]] std::optional - get(Key const& k) const - { - std::lock_guard lock(mutex_); - auto it = data_.find(k); - return it == data_.end() ? std::nullopt : std::optional{it->second}; - } - - bool - put(Key const& k, Mapped value) - { - std::lock_guard lock(mutex_); - return data_.insert_or_assign(k, std::move(value)).second; - } - - bool - contains(Key const& k) const - { - std::lock_guard lock(mutex_); - return data_.find(k) != data_.end(); - } - - std::size_t - size() const - { - std::lock_guard lock(mutex_); - return data_.size(); - } - - bool - empty() const - { - std::lock_guard lock(mutex_); - return data_.empty(); - } - - void - reserve(std::size_t n) - { - std::lock_guard lock(mutex_); - data_.reserve(n); - } - - void - rehash(std::size_t n) - { - std::lock_guard lock(mutex_); - data_.rehash(n); - } - - std::size_t - eraseBefore(Key const& cutoff) - { - std::lock_guard lock(mutex_); - auto const before = data_.size(); - std::erase_if(data_, [&](auto const& kv) { return kv.first < cutoff; }); - return before - data_.size(); - } - -private: - std::unordered_map data_; - mutable std::mutex mutex_; -}; - -} // namespace xrpl diff --git a/src/tests/libxrpl/ledger/LedgerIndexMap.cpp b/src/tests/libxrpl/ledger/LedgerIndexMap.cpp deleted file mode 100644 index 559de33561..0000000000 --- a/src/tests/libxrpl/ledger/LedgerIndexMap.cpp +++ /dev/null @@ -1,191 +0,0 @@ -#include - -#include - -#include -#include -#include - -using namespace xrpl; - -using TestMap = LedgerIndexMap; - -TEST(LedgerIndexMap, DefaultEmpty) -{ - TestMap m; - EXPECT_EQ(m.size(), 0); - EXPECT_TRUE(m.empty()); - EXPECT_FALSE(m.get(42).has_value()); - EXPECT_FALSE(m.contains(42)); -} - -TEST(LedgerIndexMap, PutInsertsValue) -{ - TestMap m; - bool inserted = m.put(10, ""); - EXPECT_TRUE(inserted); - EXPECT_EQ(m.size(), 1); - EXPECT_TRUE(m.contains(10)); - auto got = m.get(10); - ASSERT_TRUE(got.has_value()); - EXPECT_TRUE(got->empty()); -} - -TEST(LedgerIndexMap, PutWithValue) -{ - TestMap m; - bool inserted = m.put(7, "seven"); - EXPECT_TRUE(inserted); - EXPECT_EQ(m.size(), 1); - auto got = m.get(7); - ASSERT_TRUE(got.has_value()); - EXPECT_EQ(*got, "seven"); -} - -TEST(LedgerIndexMap, InsertThroughPut) -{ - TestMap m; - bool inserted = m.put(20, "twenty"); - EXPECT_TRUE(inserted); - auto got = m.get(20); - ASSERT_TRUE(got.has_value()); - EXPECT_EQ(*got, "twenty"); - EXPECT_EQ(m.size(), 1); -} - -TEST(LedgerIndexMap, OverwriteExistingKeyWithPut) -{ - TestMap m; - bool inserted1 = m.put(5, "five"); - EXPECT_TRUE(inserted1); - EXPECT_EQ(m.size(), 1); - bool inserted2 = m.put(5, "FIVE"); - EXPECT_FALSE(inserted2); // Not a new insertion, it's an update - EXPECT_EQ(m.size(), 1); - auto got = m.get(5); - ASSERT_TRUE(got.has_value()); - EXPECT_EQ(*got, "FIVE"); -} - -TEST(LedgerIndexMap, OnceFoundOneNotFound) -{ - TestMap m; - m.put(1, "one"); - EXPECT_TRUE(m.get(1).has_value()); - EXPECT_FALSE(m.get(2).has_value()); -} - -TEST(LedgerIndexMap, TryEraseBeforeNothingToDo) -{ - TestMap m; - m.put(10, "a"); - m.put(11, "b"); - m.put(12, "c"); - EXPECT_EQ(m.eraseBefore(10), 0); - EXPECT_EQ(m.size(), 3); - EXPECT_TRUE(m.contains(10)); - EXPECT_TRUE(m.contains(11)); - EXPECT_TRUE(m.contains(12)); -} - -TEST(LedgerIndexMap, EraseBeforeRemovesSeveralEntries) -{ - TestMap m; - m.put(10, "a"); - m.put(11, "b"); - m.put(12, "c"); - m.put(13, "d"); - EXPECT_EQ(m.eraseBefore(12), 2); - EXPECT_FALSE(m.contains(10)); - EXPECT_FALSE(m.contains(11)); - EXPECT_TRUE(m.contains(12)); - EXPECT_TRUE(m.contains(13)); - EXPECT_EQ(m.size(), 2); -} - -TEST(LedgerIndexMap, EraseBeforeRemovesAllEntries) -{ - TestMap m; - m.put(1, "x"); - m.put(2, "y"); - EXPECT_EQ(m.eraseBefore(1000), 2); - EXPECT_EQ(m.size(), 0); - EXPECT_TRUE(m.empty()); -} - -TEST(LedgerIndexMap, EraseBeforeSameCallSecondTimeNothingToDo) -{ - TestMap m; - m.put(10, "a"); - m.put(11, "b"); - m.put(12, "c"); - EXPECT_EQ(m.eraseBefore(12), 2); - EXPECT_TRUE(m.contains(12)); - EXPECT_EQ(m.eraseBefore(12), 0); - EXPECT_EQ(m.size(), 1); -} - -TEST(LedgerIndexMap, EraseBeforeSingleEntryRemoved) -{ - TestMap m; - m.put(10, "v1"); - m.put(10, "v2"); - m.put(10, "v3"); - EXPECT_EQ(m.size(), 1); - EXPECT_EQ(m.eraseBefore(11), 1); - EXPECT_EQ(m.size(), 0); -} - -TEST(LedgerIndexMap, EraseBeforeOutlierStillRemovedInOneCall) -{ - TestMap m; - m.put(10, "a"); - m.put(12, "c"); - m.put(11, "b"); // out-of-order insert - - EXPECT_EQ(m.eraseBefore(12), 2); // removes 10 and 11 - EXPECT_FALSE(m.contains(10)); - EXPECT_FALSE(m.contains(11)); - EXPECT_TRUE(m.contains(12)); - EXPECT_EQ(m.size(), 1); -} - -TEST(LedgerIndexMap, EraseBeforeEraseInTwoSteps) -{ - TestMap m; - for (int k : {10, 11, 12, 13}) - m.put(k, std::to_string(k)); - - EXPECT_EQ(m.eraseBefore(11), 1); - EXPECT_FALSE(m.contains(10)); - EXPECT_EQ(m.size(), 3); - - EXPECT_EQ(m.eraseBefore(13), 2); - EXPECT_FALSE(m.contains(11)); - EXPECT_FALSE(m.contains(12)); - EXPECT_TRUE(m.contains(13)); - EXPECT_EQ(m.size(), 1); -} - -TEST(LedgerIndexMap, RehashDoesNotLoseEntries) -{ - TestMap m; - for (int k = 0; k < 16; ++k) - m.put(k, "v" + std::to_string(k)); - - m.reserve(64); - m.rehash(32); - - for (int k = 0; k < 16; ++k) - { - auto v = m.get(k); - ASSERT_TRUE(v.has_value()); - EXPECT_EQ(*v, "v" + std::to_string(k)); - } - - EXPECT_EQ(m.eraseBefore(8), 8); - for (int k = 0; k < 8; ++k) - EXPECT_FALSE(m.contains(k)); - for (int k = 8; k < 16; ++k) - EXPECT_TRUE(m.contains(k)); -} diff --git a/src/xrpld/app/ledger/LedgerHistory.cpp b/src/xrpld/app/ledger/LedgerHistory.cpp index cbf8c7163b..c6c2c05f3a 100644 --- a/src/xrpld/app/ledger/LedgerHistory.cpp +++ b/src/xrpld/app/ledger/LedgerHistory.cpp @@ -12,22 +12,21 @@ LedgerHistory::LedgerHistory(beast::insight::Collector::ptr const& collector, Ap : app_(app) , collector_(collector) , mismatch_counter_(collector->make_counter("ledger.history", "mismatch")) - , m_ledgers_by_hash( - "LedgerCache", - app_.config().getValueFor(SizedItem::ledgerSize), - std::chrono::seconds{app_.config().getValueFor(SizedItem::ledgerAge)}, - stopwatch(), - app_.journal("TaggedCache")) , m_consensus_validated( "ConsensusValidated", 64, std::chrono::minutes{5}, stopwatch(), app_.journal("TaggedCache")) - , mLedgersByIndex( - 512) // Index map reserve capacity, aligned with typical ledger_history retention. , j_(app.journal("LedgerHistory")) { + auto lock = ledger_maps_.lock(); + lock->by_hash = std::make_unique( + "LedgerCache", + app_.config().getValueFor(SizedItem::ledgerSize), + std::chrono::seconds{app_.config().getValueFor(SizedItem::ledgerAge)}, + stopwatch(), + app_.journal("TaggedCache")); } bool @@ -39,10 +38,11 @@ LedgerHistory::insert(std::shared_ptr const& ledger, bool validate XRPL_ASSERT( ledger->stateMap().getHash().isNonZero(), "xrpl::LedgerHistory::insert : nonzero hash"); + auto lock = ledger_maps_.lock(); bool const alreadyHad = - m_ledgers_by_hash.canonicalize_replace_cache(ledger->header().hash, ledger); + lock->by_hash->canonicalize_replace_cache(ledger->header().hash, ledger); if (validated) - mLedgersByIndex.put(ledger->header().seq, ledger->header().hash); + lock->by_index[ledger->header().seq] = ledger->header().hash; return alreadyHad; } @@ -50,20 +50,31 @@ LedgerHistory::insert(std::shared_ptr const& ledger, bool validate LedgerHash LedgerHistory::getLedgerHash(LedgerIndex index) { - if (auto p = mLedgersByIndex.get(index)) - return *p; + auto lock = ledger_maps_.lock(); + if (auto it = lock->by_index.find(index); it != lock->by_index.end()) + return it->second; return {}; } std::shared_ptr LedgerHistory::getLedgerBySeq(LedgerIndex index) { - if (auto p = mLedgersByIndex.get(index)) + uint256 hash; { - uint256 const hash = *p; - return getLedgerByHash(hash); + auto lock = ledger_maps_.lock(); + if (auto it = lock->by_index.find(index); it != lock->by_index.end()) + { + hash = it->second; + } + else + { + hash = {}; + } } + if (!hash.isZero()) + return getLedgerByHash(hash); + std::shared_ptr ret = loadByIndex(index, app_); if (!ret) @@ -74,10 +85,11 @@ LedgerHistory::getLedgerBySeq(LedgerIndex index) { // Add this ledger to the local tracking by index + auto lock = ledger_maps_.lock(); XRPL_ASSERT( ret->isImmutable(), "xrpl::LedgerHistory::getLedgerBySeq : immutable result ledger"); - m_ledgers_by_hash.canonicalize_replace_client(ret->header().hash, ret); - mLedgersByIndex.put(ret->header().seq, ret->header().hash); + lock->by_hash->canonicalize_replace_client(ret->header().hash, ret); + lock->by_index[ret->header().seq] = ret->header().hash; return (ret->header().seq == index) ? ret : nullptr; } } @@ -85,7 +97,11 @@ LedgerHistory::getLedgerBySeq(LedgerIndex index) std::shared_ptr LedgerHistory::getLedgerByHash(LedgerHash const& hash) { - auto ret = m_ledgers_by_hash.fetch(hash); + std::shared_ptr ret; + { + auto lock = ledger_maps_.lock(); + ret = lock->by_hash->fetch(hash); + } if (ret) { @@ -110,7 +126,10 @@ LedgerHistory::getLedgerByHash(LedgerHash const& hash) XRPL_ASSERT( ret->header().hash == hash, "xrpl::LedgerHistory::getLedgerByHash : loaded ledger hash match"); - m_ledgers_by_hash.canonicalize_replace_client(ret->header().hash, ret); + { + auto lock = ledger_maps_.lock(); + lock->by_hash->canonicalize_replace_client(ret->header().hash, ret); + } XRPL_ASSERT( ret->header().hash == hash, "xrpl::LedgerHistory::getLedgerByHash : result hash match"); @@ -462,11 +481,12 @@ LedgerHistory::validatedLedger( bool LedgerHistory::fixIndex(LedgerIndex ledgerIndex, LedgerHash const& ledgerHash) { - if (auto cur = mLedgersByIndex.get(ledgerIndex)) + auto lock = ledger_maps_.lock(); + if (auto it = lock->by_index.find(ledgerIndex); it != lock->by_index.end()) { - if (*cur != ledgerHash) + if (it->second != ledgerHash) { - mLedgersByIndex.put(ledgerIndex, ledgerHash); + lock->by_index[ledgerIndex] = ledgerHash; return false; } } @@ -477,21 +497,44 @@ void LedgerHistory::clearLedgerCachePrior(LedgerIndex seq) { std::size_t hashesCleared = 0; - for (LedgerHash it : m_ledgers_by_hash.getKeys()) - { - auto const ledger = getLedgerByHash(it); - if (!ledger || ledger->header().seq < seq) - { - m_ledgers_by_hash.del(it, false); - ++hashesCleared; - } - } - JLOG(j_.debug()) << "LedgersByHash: cleared " << hashesCleared << " entries before seq " << seq - << " (total now " << m_ledgers_by_hash.size() << ")"; + std::size_t indexesCleared = 0; + std::size_t cacheSize = 0; + std::size_t indexSize = 0; - std::size_t const indexesCleared = mLedgersByIndex.eraseBefore(seq); - JLOG(j_.debug()) << "LedgerIndexMap: cleared " << indexesCleared << " index entries before seq " - << seq << " (total now " << mLedgersByIndex.size() << ")"; + { + auto lock = ledger_maps_.lock(); + for (LedgerHash it : lock->by_hash->getKeys()) + { + auto const ledger = getLedgerByHash(it); + if (!ledger || ledger->header().seq < seq) + { + lock->by_hash->del(it, false); + ++hashesCleared; + } + } + cacheSize = lock->by_hash->size(); + + auto it = lock->by_index.begin(); + while (it != lock->by_index.end()) + { + if (it->first < seq) + { + it = lock->by_index.erase(it); + ++indexesCleared; + } + else + { + ++it; + } + } + indexSize = lock->by_index.size(); + } + + JLOG(j_.debug()) << "LedgersByHash: cleared " << hashesCleared << " entries before seq " << seq + << " (total now " << cacheSize << ")"; + + JLOG(j_.debug()) << "LedgersByIndex: cleared " << indexesCleared << " index entries before seq " + << seq << " (total now " << indexSize << ")"; } } // namespace xrpl diff --git a/src/xrpld/app/ledger/LedgerHistory.h b/src/xrpld/app/ledger/LedgerHistory.h index c6a422a66a..f843bca5a6 100644 --- a/src/xrpld/app/ledger/LedgerHistory.h +++ b/src/xrpld/app/ledger/LedgerHistory.h @@ -3,10 +3,12 @@ #include #include +#include #include -#include #include +#include +#include #include namespace xrpl { @@ -31,7 +33,8 @@ public: float getCacheHitRate() { - return m_ledgers_by_hash.getHitRate(); + auto lock = ledger_maps_.lock(); + return lock->by_hash->getHitRate(); } /** Get a ledger given its sequence number */ @@ -54,7 +57,10 @@ public: void sweep() { - m_ledgers_by_hash.sweep(); + { + auto lock = ledger_maps_.lock(); + lock->by_hash->sweep(); + } m_consensus_validated.sweep(); } @@ -102,9 +108,15 @@ private: beast::insight::Collector::ptr collector_; beast::insight::Counter mismatch_counter_; - using LedgersByHash = TaggedCache; + struct LedgerMaps + { + using LedgersByHash = TaggedCache; - LedgersByHash m_ledgers_by_hash; + std::unique_ptr by_hash; + std::map by_index; // validated ledgers + }; + + xrpl::Mutex ledger_maps_; // Maps ledger indexes to the corresponding hashes // For debug and logging purposes @@ -124,9 +136,6 @@ private: using ConsensusValidated = TaggedCache; ConsensusValidated m_consensus_validated; - // Maps ledger indexes to the corresponding hash. - xrpl::LedgerIndexMap mLedgersByIndex; // validated ledgers - beast::Journal j_; };