From 8d31b1739de55566edb8e07d1cacadb8f2df79e6 Mon Sep 17 00:00:00 2001 From: Bart Thomee <11445373+bthomee@users.noreply.github.com> Date: Fri, 28 Mar 2025 13:21:19 -0400 Subject: [PATCH 1/8] TEST: Disable tagged cache to measure performance --- include/xrpl/basics/TaggedCache.ipp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/xrpl/basics/TaggedCache.ipp b/include/xrpl/basics/TaggedCache.ipp index 0108061680..f14a4b9886 100644 --- a/include/xrpl/basics/TaggedCache.ipp +++ b/include/xrpl/basics/TaggedCache.ipp @@ -54,7 +54,7 @@ inline TaggedCache< , m_clock(clock) , m_stats(name, std::bind(&TaggedCache::collect_metrics, this), collector) , m_name(name) - , m_target_size(size) + , m_target_size(0) , m_target_age(expiration) , m_cache_count(0) , m_hits(0) From d1703842e77578424011420bf3f272485ce3e897 Mon Sep 17 00:00:00 2001 From: Bart Thomee <11445373+bthomee@users.noreply.github.com> Date: Tue, 1 Apr 2025 11:41:20 -0400 Subject: [PATCH 2/8] Fully disable cache --- include/xrpl/basics/TaggedCache.h | 8 +-- include/xrpl/basics/TaggedCache.ipp | 94 ++++++++++++++++------------- 2 files changed, 57 insertions(+), 45 deletions(-) diff --git a/include/xrpl/basics/TaggedCache.h b/include/xrpl/basics/TaggedCache.h index 64570ae061..93bf535bb2 100644 --- a/include/xrpl/basics/TaggedCache.h +++ b/include/xrpl/basics/TaggedCache.h @@ -192,8 +192,8 @@ public: // End CachedSLEs functions. private: - SharedPointerType - initialFetch(key_type const& key, std::lock_guard const& l); + // SharedPointerType + // initialFetch(key_type const& key, std::lock_guard const& l); void collect_metrics(); @@ -294,7 +294,7 @@ private: using cache_type = hardened_partitioned_hash_map; - [[nodiscard]] std::thread + /*[[nodiscard]] std::thread sweepHelper( clock_type::time_point const& when_expire, [[maybe_unused]] clock_type::time_point const& now, @@ -310,7 +310,7 @@ private: typename KeyOnlyCacheType::map_type& partition, SweptPointersVector&, std::atomic& allRemovals, - std::lock_guard const&); + std::lock_guard const&);*/ beast::Journal m_journal; clock_type& m_clock; diff --git a/include/xrpl/basics/TaggedCache.ipp b/include/xrpl/basics/TaggedCache.ipp index f14a4b9886..bc31909f95 100644 --- a/include/xrpl/basics/TaggedCache.ipp +++ b/include/xrpl/basics/TaggedCache.ipp @@ -54,7 +54,7 @@ inline TaggedCache< , m_clock(clock) , m_stats(name, std::bind(&TaggedCache::collect_metrics, this), collector) , m_name(name) - , m_target_size(0) + , m_target_size(size) , m_target_age(expiration) , m_cache_count(0) , m_hits(0) @@ -105,8 +105,9 @@ TaggedCache< KeyEqual, Mutex>::size() const { - std::lock_guard lock(m_mutex); - return m_cache.size(); + /*std::lock_guard lock(m_mutex); + return m_cache.size();*/ + return 0; } template < @@ -129,8 +130,9 @@ TaggedCache< KeyEqual, Mutex>::getCacheSize() const { - std::lock_guard lock(m_mutex); - return m_cache_count; + /*std::lock_guard lock(m_mutex); + return m_cache_count;*/ + return 0; } template < @@ -153,8 +155,9 @@ TaggedCache< KeyEqual, Mutex>::getTrackSize() const { - std::lock_guard lock(m_mutex); - return m_cache.size(); + /*std::lock_guard lock(m_mutex); + return m_cache.size();*/ + return 0; } template < @@ -177,9 +180,10 @@ TaggedCache< KeyEqual, Mutex>::getHitRate() { - std::lock_guard lock(m_mutex); + /*std::lock_guard lock(m_mutex); auto const total = static_cast(m_hits + m_misses); - return m_hits * (100.0f / std::max(1.0f, total)); + return m_hits * (100.0f / std::max(1.0f, total));*/ + return 0.0f; } template < @@ -202,9 +206,9 @@ TaggedCache< KeyEqual, Mutex>::clear() { - std::lock_guard lock(m_mutex); + /*std::lock_guard lock(m_mutex); m_cache.clear(); - m_cache_count = 0; + m_cache_count = 0;*/ } template < @@ -227,11 +231,11 @@ TaggedCache< KeyEqual, Mutex>::reset() { - std::lock_guard lock(m_mutex); + /*std::lock_guard lock(m_mutex); m_cache.clear(); m_cache_count = 0; m_hits = 0; - m_misses = 0; + m_misses = 0;*/ } template < @@ -255,7 +259,7 @@ TaggedCache< KeyEqual, Mutex>::touch_if_exists(KeyComparable const& key) { - std::lock_guard lock(m_mutex); + /*std::lock_guard lock(m_mutex); auto const iter(m_cache.find(key)); if (iter == m_cache.end()) { @@ -264,7 +268,8 @@ TaggedCache< } iter->second.touch(m_clock.now()); ++m_stats.hits; - return true; + return true;*/ + return false; } template < @@ -287,7 +292,7 @@ TaggedCache< KeyEqual, Mutex>::sweep() { - // Keep references to all the stuff we sweep + /*// Keep references to all the stuff we sweep // For performance, each worker thread should exit before the swept data // is destroyed but still within the main cache lock. std::vector allStuffToSweep(m_cache.partitions()); @@ -344,7 +349,7 @@ TaggedCache< << std::chrono::duration_cast( std::chrono::steady_clock::now() - start) .count() - << "ms"; + << "ms";*/ } template < @@ -367,7 +372,7 @@ TaggedCache< KeyEqual, Mutex>::del(const key_type& key, bool valid) { - // Remove from cache, if !valid, remove from map too. Returns true if + /*// Remove from cache, if !valid, remove from map too. Returns true if // removed from cache std::lock_guard lock(m_mutex); @@ -390,7 +395,8 @@ TaggedCache< if (!valid || entry.isExpired()) m_cache.erase(cit); - return ret; + return ret;*/ + return false; } template < @@ -418,7 +424,7 @@ TaggedCache< SharedPointerType& data, R&& replaceCallback) { - // Return canonical value, store if needed, refresh in cache + /*// Return canonical value, store if needed, refresh in cache // Return values: true=we had the data already std::lock_guard lock(m_mutex); @@ -484,7 +490,7 @@ TaggedCache< } entry.ptr = data; - ++m_cache_count; + ++m_cache_count;*/ return false; } @@ -560,11 +566,12 @@ TaggedCache< KeyEqual, Mutex>::fetch(const key_type& key) { - std::lock_guard l(m_mutex); + /*std::lock_guard l(m_mutex); auto ret = initialFetch(key, l); if (!ret) ++m_misses; - return ret; + return ret;*/ + return {}; } template < @@ -627,7 +634,7 @@ TaggedCache< Mutex>::insert(key_type const& key) -> std::enable_if_t { - std::lock_guard lock(m_mutex); + /*std::lock_guard lock(m_mutex); clock_type::time_point const now(m_clock.now()); auto [it, inserted] = m_cache.emplace( std::piecewise_construct, @@ -635,7 +642,8 @@ TaggedCache< std::forward_as_tuple(now)); if (!inserted) it->second.last_access = now; - return inserted; + return inserted;*/ + return false; } template < @@ -658,14 +666,15 @@ TaggedCache< KeyEqual, Mutex>::retrieve(const key_type& key, T& data) { - // retrieve the value of the stored data + /*// retrieve the value of the stored data auto entry = fetch(key); if (!entry) return false; data = *entry; - return true; + return true;*/ + return false; } template < @@ -713,12 +722,12 @@ TaggedCache< { std::vector v; - { + /*{ std::lock_guard lock(m_mutex); v.reserve(m_cache.size()); for (auto const& _ : m_cache) v.push_back(_.first); - } + }*/ return v; } @@ -743,11 +752,12 @@ TaggedCache< KeyEqual, Mutex>::rate() const { - std::lock_guard lock(m_mutex); + /*std::lock_guard lock(m_mutex); auto const tot = m_hits + m_misses; if (tot == 0) return 0; - return double(m_hits) / tot; + return double(m_hits) / tot;*/ + return 0.0; } template < @@ -771,27 +781,29 @@ TaggedCache< KeyEqual, Mutex>::fetch(key_type const& digest, Handler const& h) { - { + /*{ std::lock_guard l(m_mutex); if (auto ret = initialFetch(digest, l)) return ret; - } + }*/ auto sle = h(); if (!sle) return {}; - std::lock_guard l(m_mutex); + /*std::lock_guard l(m_mutex); ++m_misses; auto const [it, inserted] = m_cache.emplace(digest, Entry(m_clock.now(), std::move(sle))); if (!inserted) it->second.touch(m_clock.now()); - return it->second.ptr.getStrong(); + return it->second.ptr.getStrong();*/ + + return sle; } // End CachedSLEs functions. -template < +/*template < class Key, class T, bool IsKeyCache, @@ -834,7 +846,7 @@ TaggedCache< m_cache.erase(cit); return {}; -} +}*/ template < class Key, @@ -860,17 +872,17 @@ TaggedCache< { beast::insight::Gauge::value_type hit_rate(0); - { + /*{ std::lock_guard lock(m_mutex); auto const total(m_hits + m_misses); if (total != 0) hit_rate = (m_hits * 100) / total; - } + }*/ m_stats.hit_rate.set(hit_rate); } } -template < +/*template < class Key, class T, bool IsKeyCache, @@ -1022,7 +1034,7 @@ TaggedCache< allRemovals += cacheRemovals; }); -} +}*/ } // namespace ripple From d01851bc5afb336455551cb1863128939dcdc135 Mon Sep 17 00:00:00 2001 From: Bart Thomee <11445373+bthomee@users.noreply.github.com> Date: Tue, 1 Apr 2025 13:24:18 -0400 Subject: [PATCH 3/8] Only disable the database cache --- include/xrpl/basics/TaggedCache.h | 8 +- include/xrpl/basics/TaggedCache.ipp | 92 +++++++++----------- src/xrpld/nodestore/detail/DatabaseNodeImp.h | 4 +- 3 files changed, 46 insertions(+), 58 deletions(-) diff --git a/include/xrpl/basics/TaggedCache.h b/include/xrpl/basics/TaggedCache.h index 93bf535bb2..64570ae061 100644 --- a/include/xrpl/basics/TaggedCache.h +++ b/include/xrpl/basics/TaggedCache.h @@ -192,8 +192,8 @@ public: // End CachedSLEs functions. private: - // SharedPointerType - // initialFetch(key_type const& key, std::lock_guard const& l); + SharedPointerType + initialFetch(key_type const& key, std::lock_guard const& l); void collect_metrics(); @@ -294,7 +294,7 @@ private: using cache_type = hardened_partitioned_hash_map; - /*[[nodiscard]] std::thread + [[nodiscard]] std::thread sweepHelper( clock_type::time_point const& when_expire, [[maybe_unused]] clock_type::time_point const& now, @@ -310,7 +310,7 @@ private: typename KeyOnlyCacheType::map_type& partition, SweptPointersVector&, std::atomic& allRemovals, - std::lock_guard const&);*/ + std::lock_guard const&); beast::Journal m_journal; clock_type& m_clock; diff --git a/include/xrpl/basics/TaggedCache.ipp b/include/xrpl/basics/TaggedCache.ipp index bc31909f95..0108061680 100644 --- a/include/xrpl/basics/TaggedCache.ipp +++ b/include/xrpl/basics/TaggedCache.ipp @@ -105,9 +105,8 @@ TaggedCache< KeyEqual, Mutex>::size() const { - /*std::lock_guard lock(m_mutex); - return m_cache.size();*/ - return 0; + std::lock_guard lock(m_mutex); + return m_cache.size(); } template < @@ -130,9 +129,8 @@ TaggedCache< KeyEqual, Mutex>::getCacheSize() const { - /*std::lock_guard lock(m_mutex); - return m_cache_count;*/ - return 0; + std::lock_guard lock(m_mutex); + return m_cache_count; } template < @@ -155,9 +153,8 @@ TaggedCache< KeyEqual, Mutex>::getTrackSize() const { - /*std::lock_guard lock(m_mutex); - return m_cache.size();*/ - return 0; + std::lock_guard lock(m_mutex); + return m_cache.size(); } template < @@ -180,10 +177,9 @@ TaggedCache< KeyEqual, Mutex>::getHitRate() { - /*std::lock_guard lock(m_mutex); + std::lock_guard lock(m_mutex); auto const total = static_cast(m_hits + m_misses); - return m_hits * (100.0f / std::max(1.0f, total));*/ - return 0.0f; + return m_hits * (100.0f / std::max(1.0f, total)); } template < @@ -206,9 +202,9 @@ TaggedCache< KeyEqual, Mutex>::clear() { - /*std::lock_guard lock(m_mutex); + std::lock_guard lock(m_mutex); m_cache.clear(); - m_cache_count = 0;*/ + m_cache_count = 0; } template < @@ -231,11 +227,11 @@ TaggedCache< KeyEqual, Mutex>::reset() { - /*std::lock_guard lock(m_mutex); + std::lock_guard lock(m_mutex); m_cache.clear(); m_cache_count = 0; m_hits = 0; - m_misses = 0;*/ + m_misses = 0; } template < @@ -259,7 +255,7 @@ TaggedCache< KeyEqual, Mutex>::touch_if_exists(KeyComparable const& key) { - /*std::lock_guard lock(m_mutex); + std::lock_guard lock(m_mutex); auto const iter(m_cache.find(key)); if (iter == m_cache.end()) { @@ -268,8 +264,7 @@ TaggedCache< } iter->second.touch(m_clock.now()); ++m_stats.hits; - return true;*/ - return false; + return true; } template < @@ -292,7 +287,7 @@ TaggedCache< KeyEqual, Mutex>::sweep() { - /*// Keep references to all the stuff we sweep + // Keep references to all the stuff we sweep // For performance, each worker thread should exit before the swept data // is destroyed but still within the main cache lock. std::vector allStuffToSweep(m_cache.partitions()); @@ -349,7 +344,7 @@ TaggedCache< << std::chrono::duration_cast( std::chrono::steady_clock::now() - start) .count() - << "ms";*/ + << "ms"; } template < @@ -372,7 +367,7 @@ TaggedCache< KeyEqual, Mutex>::del(const key_type& key, bool valid) { - /*// Remove from cache, if !valid, remove from map too. Returns true if + // Remove from cache, if !valid, remove from map too. Returns true if // removed from cache std::lock_guard lock(m_mutex); @@ -395,8 +390,7 @@ TaggedCache< if (!valid || entry.isExpired()) m_cache.erase(cit); - return ret;*/ - return false; + return ret; } template < @@ -424,7 +418,7 @@ TaggedCache< SharedPointerType& data, R&& replaceCallback) { - /*// Return canonical value, store if needed, refresh in cache + // Return canonical value, store if needed, refresh in cache // Return values: true=we had the data already std::lock_guard lock(m_mutex); @@ -490,7 +484,7 @@ TaggedCache< } entry.ptr = data; - ++m_cache_count;*/ + ++m_cache_count; return false; } @@ -566,12 +560,11 @@ TaggedCache< KeyEqual, Mutex>::fetch(const key_type& key) { - /*std::lock_guard l(m_mutex); + std::lock_guard l(m_mutex); auto ret = initialFetch(key, l); if (!ret) ++m_misses; - return ret;*/ - return {}; + return ret; } template < @@ -634,7 +627,7 @@ TaggedCache< Mutex>::insert(key_type const& key) -> std::enable_if_t { - /*std::lock_guard lock(m_mutex); + std::lock_guard lock(m_mutex); clock_type::time_point const now(m_clock.now()); auto [it, inserted] = m_cache.emplace( std::piecewise_construct, @@ -642,8 +635,7 @@ TaggedCache< std::forward_as_tuple(now)); if (!inserted) it->second.last_access = now; - return inserted;*/ - return false; + return inserted; } template < @@ -666,15 +658,14 @@ TaggedCache< KeyEqual, Mutex>::retrieve(const key_type& key, T& data) { - /*// retrieve the value of the stored data + // retrieve the value of the stored data auto entry = fetch(key); if (!entry) return false; data = *entry; - return true;*/ - return false; + return true; } template < @@ -722,12 +713,12 @@ TaggedCache< { std::vector v; - /*{ + { std::lock_guard lock(m_mutex); v.reserve(m_cache.size()); for (auto const& _ : m_cache) v.push_back(_.first); - }*/ + } return v; } @@ -752,12 +743,11 @@ TaggedCache< KeyEqual, Mutex>::rate() const { - /*std::lock_guard lock(m_mutex); + std::lock_guard lock(m_mutex); auto const tot = m_hits + m_misses; if (tot == 0) return 0; - return double(m_hits) / tot;*/ - return 0.0; + return double(m_hits) / tot; } template < @@ -781,29 +771,27 @@ TaggedCache< KeyEqual, Mutex>::fetch(key_type const& digest, Handler const& h) { - /*{ + { std::lock_guard l(m_mutex); if (auto ret = initialFetch(digest, l)) return ret; - }*/ + } auto sle = h(); if (!sle) return {}; - /*std::lock_guard l(m_mutex); + std::lock_guard l(m_mutex); ++m_misses; auto const [it, inserted] = m_cache.emplace(digest, Entry(m_clock.now(), std::move(sle))); if (!inserted) it->second.touch(m_clock.now()); - return it->second.ptr.getStrong();*/ - - return sle; + return it->second.ptr.getStrong(); } // End CachedSLEs functions. -/*template < +template < class Key, class T, bool IsKeyCache, @@ -846,7 +834,7 @@ TaggedCache< m_cache.erase(cit); return {}; -}*/ +} template < class Key, @@ -872,17 +860,17 @@ TaggedCache< { beast::insight::Gauge::value_type hit_rate(0); - /*{ + { std::lock_guard lock(m_mutex); auto const total(m_hits + m_misses); if (total != 0) hit_rate = (m_hits * 100) / total; - }*/ + } m_stats.hit_rate.set(hit_rate); } } -/*template < +template < class Key, class T, bool IsKeyCache, @@ -1034,7 +1022,7 @@ TaggedCache< allRemovals += cacheRemovals; }); -}*/ +} } // namespace ripple diff --git a/src/xrpld/nodestore/detail/DatabaseNodeImp.h b/src/xrpld/nodestore/detail/DatabaseNodeImp.h index 160bf92d5e..a30c0b876c 100644 --- a/src/xrpld/nodestore/detail/DatabaseNodeImp.h +++ b/src/xrpld/nodestore/detail/DatabaseNodeImp.h @@ -67,7 +67,7 @@ public: } } - if (cacheSize != 0 || cacheAge != 0) + /*if (cacheSize != 0 || cacheAge != 0) { cache_ = std::make_shared>( "DatabaseNodeImp", @@ -75,7 +75,7 @@ public: std::chrono::minutes(cacheAge.value_or(0)), stopwatch(), j); - } + }*/ XRPL_ASSERT( backend_, From 6e7537dada286939d22adfd1baaae8b8dbfc60a3 Mon Sep 17 00:00:00 2001 From: Bart Thomee <11445373+bthomee@users.noreply.github.com> Date: Mon, 19 May 2025 16:51:32 -0400 Subject: [PATCH 4/8] Remove cache from DatabaseNodeImp --- .../nodestore/detail/DatabaseNodeImp.cpp | 152 ++++-------------- src/xrpld/nodestore/detail/DatabaseNodeImp.h | 35 ---- 2 files changed, 32 insertions(+), 155 deletions(-) diff --git a/src/xrpld/nodestore/detail/DatabaseNodeImp.cpp b/src/xrpld/nodestore/detail/DatabaseNodeImp.cpp index 2502d52815..dff61c609d 100644 --- a/src/xrpld/nodestore/detail/DatabaseNodeImp.cpp +++ b/src/xrpld/nodestore/detail/DatabaseNodeImp.cpp @@ -33,14 +33,6 @@ DatabaseNodeImp::store( auto obj = NodeObject::createObject(type, std::move(data), hash); backend_->store(obj); - if (cache_) - { - // After the store, replace a negative cache entry if there is one - cache_->canonicalize( - hash, obj, [](std::shared_ptr const& n) { - return n->getType() == hotDUMMY; - }); - } } void @@ -49,23 +41,12 @@ DatabaseNodeImp::asyncFetch( std::uint32_t ledgerSeq, std::function const&)>&& callback) { - if (cache_) - { - std::shared_ptr obj = cache_->fetch(hash); - if (obj) - { - callback(obj->getType() == hotDUMMY ? nullptr : obj); - return; - } - } Database::asyncFetch(hash, ledgerSeq, std::move(callback)); } void DatabaseNodeImp::sweep() { - if (cache_) - cache_->sweep(); } std::shared_ptr @@ -75,64 +56,33 @@ DatabaseNodeImp::fetchNodeObject( FetchReport& fetchReport, bool duplicate) { - std::shared_ptr nodeObject = - cache_ ? cache_->fetch(hash) : nullptr; + std::shared_ptr nodeObject = nullptr; + Status status; - if (!nodeObject) + try { - JLOG(j_.trace()) << "fetchNodeObject " << hash << ": record not " - << (cache_ ? "cached" : "found"); - - Status status; - - try - { - status = backend_->fetch(hash.data(), &nodeObject); - } - catch (std::exception const& e) - { - JLOG(j_.fatal()) - << "fetchNodeObject " << hash - << ": Exception fetching from backend: " << e.what(); - Rethrow(); - } - - switch (status) - { - case ok: - if (cache_) - { - if (nodeObject) - cache_->canonicalize_replace_client(hash, nodeObject); - else - { - auto notFound = - NodeObject::createObject(hotDUMMY, {}, hash); - cache_->canonicalize_replace_client(hash, notFound); - if (notFound->getType() != hotDUMMY) - nodeObject = notFound; - } - } - break; - case notFound: - break; - case dataCorrupt: - JLOG(j_.fatal()) << "fetchNodeObject " << hash - << ": nodestore data is corrupted"; - break; - default: - JLOG(j_.warn()) - << "fetchNodeObject " << hash - << ": backend returns unknown result " << status; - break; - } + status = backend_->fetch(hash.data(), &nodeObject); } - else + catch (std::exception const& e) { - JLOG(j_.trace()) << "fetchNodeObject " << hash - << ": record found in cache"; - if (nodeObject->getType() == hotDUMMY) - nodeObject.reset(); + JLOG(j_.fatal()) << "fetchNodeObject " << hash + << ": Exception fetching from backend: " << e.what(); + Rethrow(); + } + + switch (status) + { + case ok: + case notFound: + break; + case dataCorrupt: + JLOG(j_.fatal()) << "fetchNodeObject " << hash + << ": nodestore data is corrupted"; + break; + default: + JLOG(j_.warn()) << "fetchNodeObject " << hash + << ": backend returns unknown result " << status; + break; } if (nodeObject) @@ -144,71 +94,33 @@ DatabaseNodeImp::fetchNodeObject( std::vector> DatabaseNodeImp::fetchBatch(std::vector const& hashes) { - std::vector> results{hashes.size()}; using namespace std::chrono; auto const before = steady_clock::now(); - std::unordered_map indexMap; - std::vector cacheMisses; - uint64_t hits = 0; - uint64_t fetches = 0; + + std::vector batch; for (size_t i = 0; i < hashes.size(); ++i) { auto const& hash = hashes[i]; - // See if the object already exists in the cache - auto nObj = cache_ ? cache_->fetch(hash) : nullptr; - ++fetches; - if (!nObj) - { - // Try the database - indexMap[&hash] = i; - cacheMisses.push_back(&hash); - } - else - { - results[i] = nObj->getType() == hotDUMMY ? nullptr : nObj; - // It was in the cache. - ++hits; - } + batch.push_back(&hash); } - JLOG(j_.debug()) << "fetchBatch - cache hits = " - << (hashes.size() - cacheMisses.size()) - << " - cache misses = " << cacheMisses.size(); - auto dbResults = backend_->fetchBatch(cacheMisses).first; - - for (size_t i = 0; i < dbResults.size(); ++i) + std::vector> results{hashes.size()}; + results = backend_->fetchBatch(batch).first; + for (size_t i = 0; i < results.size(); ++i) { - auto nObj = std::move(dbResults[i]); - size_t index = indexMap[cacheMisses[i]]; - auto const& hash = hashes[index]; - - if (nObj) - { - // Ensure all threads get the same object - if (cache_) - cache_->canonicalize_replace_client(hash, nObj); - } - else + if (!results[i]) { JLOG(j_.error()) << "fetchBatch - " - << "record not found in db or cache. hash = " << strHex(hash); - if (cache_) - { - auto notFound = NodeObject::createObject(hotDUMMY, {}, hash); - cache_->canonicalize_replace_client(hash, notFound); - if (notFound->getType() != hotDUMMY) - nObj = std::move(notFound); - } + << "record not found in db. hash = " << strHex(hashes[i]); } - results[index] = std::move(nObj); } auto fetchDurationUs = std::chrono::duration_cast( steady_clock::now() - before) .count(); - updateFetchMetrics(fetches, hits, fetchDurationUs); + updateFetchMetrics(hashes.size(), 0, fetchDurationUs); return results; } diff --git a/src/xrpld/nodestore/detail/DatabaseNodeImp.h b/src/xrpld/nodestore/detail/DatabaseNodeImp.h index a30c0b876c..add58c9ca5 100644 --- a/src/xrpld/nodestore/detail/DatabaseNodeImp.h +++ b/src/xrpld/nodestore/detail/DatabaseNodeImp.h @@ -45,38 +45,6 @@ public: : Database(scheduler, readThreads, config, j) , backend_(std::move(backend)) { - std::optional cacheSize, cacheAge; - - if (config.exists("cache_size")) - { - cacheSize = get(config, "cache_size"); - if (cacheSize.value() < 0) - { - Throw( - "Specified negative value for cache_size"); - } - } - - if (config.exists("cache_age")) - { - cacheAge = get(config, "cache_age"); - if (cacheAge.value() < 0) - { - Throw( - "Specified negative value for cache_age"); - } - } - - /*if (cacheSize != 0 || cacheAge != 0) - { - cache_ = std::make_shared>( - "DatabaseNodeImp", - cacheSize.value_or(0), - std::chrono::minutes(cacheAge.value_or(0)), - stopwatch(), - j); - }*/ - XRPL_ASSERT( backend_, "ripple::NodeStore::DatabaseNodeImp::DatabaseNodeImp : non-null " @@ -137,9 +105,6 @@ public: sweep() override; private: - // Cache for database objects. This cache is not always initialized. Check - // for null before using. - std::shared_ptr> cache_; // Persistent key/value storage std::shared_ptr backend_; From a213127852523ec7b46520972c7fe442dca5844f Mon Sep 17 00:00:00 2001 From: Bart Thomee <11445373+bthomee@users.noreply.github.com> Date: Mon, 19 May 2025 16:59:43 -0400 Subject: [PATCH 5/8] Remove cache from SHAMapStoreImp --- src/test/app/SHAMapStore_test.cpp | 22 +++------------------- src/xrpld/app/misc/SHAMapStoreImp.cpp | 14 -------------- 2 files changed, 3 insertions(+), 33 deletions(-) diff --git a/src/test/app/SHAMapStore_test.cpp b/src/test/app/SHAMapStore_test.cpp index 1e0ec4bcf0..860e17cc12 100644 --- a/src/test/app/SHAMapStore_test.cpp +++ b/src/test/app/SHAMapStore_test.cpp @@ -558,23 +558,8 @@ public: Env env(*this, envconfig(onlineDelete)); ///////////////////////////////////////////////////////////// - // Create the backend. Normally, SHAMapStoreImp handles all these - // details - auto nscfg = env.app().config().section(ConfigSection::nodeDatabase()); - - // Provide default values: - if (!nscfg.exists("cache_size")) - nscfg.set( - "cache_size", - std::to_string(env.app().config().getValueFor( - SizedItem::treeCacheSize, std::nullopt))); - - if (!nscfg.exists("cache_age")) - nscfg.set( - "cache_age", - std::to_string(env.app().config().getValueFor( - SizedItem::treeCacheAge, std::nullopt))); - + // Create NodeStore with two backends to allow online deletion of data. + // Normally, SHAMapStoreImp handles all these details. NodeStoreScheduler scheduler(env.app().getJobQueue()); std::string const writableDb = "write"; @@ -582,9 +567,8 @@ public: auto writableBackend = makeBackendRotating(env, scheduler, writableDb); auto archiveBackend = makeBackendRotating(env, scheduler, archiveDb); - // Create NodeStore with two backends to allow online deletion of - // data constexpr int readThreads = 4; + auto nscfg = env.app().config().section(ConfigSection::nodeDatabase()); auto dbr = std::make_unique( scheduler, readThreads, diff --git a/src/xrpld/app/misc/SHAMapStoreImp.cpp b/src/xrpld/app/misc/SHAMapStoreImp.cpp index 52ea40cf94..78bc0f5cc7 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.cpp +++ b/src/xrpld/app/misc/SHAMapStoreImp.cpp @@ -162,20 +162,6 @@ std::unique_ptr SHAMapStoreImp::makeNodeStore(int readThreads) { auto nscfg = app_.config().section(ConfigSection::nodeDatabase()); - - // Provide default values: - if (!nscfg.exists("cache_size")) - nscfg.set( - "cache_size", - std::to_string(app_.config().getValueFor( - SizedItem::treeCacheSize, std::nullopt))); - - if (!nscfg.exists("cache_age")) - nscfg.set( - "cache_age", - std::to_string(app_.config().getValueFor( - SizedItem::treeCacheAge, std::nullopt))); - std::unique_ptr db; if (deleteInterval_) From 4650e7d2c63af600a2a4aadf8b5492f1cb28440c Mon Sep 17 00:00:00 2001 From: Bart Thomee <11445373+bthomee@users.noreply.github.com> Date: Tue, 20 May 2025 09:49:55 -0400 Subject: [PATCH 6/8] Removed unused caches from SHAMapStoreImp --- src/xrpld/app/misc/SHAMapStoreImp.cpp | 8 +------- src/xrpld/app/misc/SHAMapStoreImp.h | 2 -- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/xrpld/app/misc/SHAMapStoreImp.cpp b/src/xrpld/app/misc/SHAMapStoreImp.cpp index 78bc0f5cc7..b47dbe7c47 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.cpp +++ b/src/xrpld/app/misc/SHAMapStoreImp.cpp @@ -255,8 +255,6 @@ SHAMapStoreImp::run() LedgerIndex lastRotated = state_db_.getState().lastRotated; netOPs_ = &app_.getOPs(); ledgerMaster_ = &app_.getLedgerMaster(); - fullBelowCache_ = &(*app_.getNodeFamily().getFullBelowCache()); - treeNodeCache_ = &(*app_.getNodeFamily().getTreeNodeCache()); if (advisoryDelete_) canDelete_ = state_db_.getCanDelete(); @@ -549,16 +547,12 @@ void SHAMapStoreImp::clearCaches(LedgerIndex validatedSeq) { ledgerMaster_->clearLedgerCachePrior(validatedSeq); - fullBelowCache_->clear(); } void SHAMapStoreImp::freshenCaches() { - if (freshenCache(*treeNodeCache_)) - return; - if (freshenCache(app_.getMasterTransaction().getCache())) - return; + freshenCache(app_.getMasterTransaction().getCache()); } void diff --git a/src/xrpld/app/misc/SHAMapStoreImp.h b/src/xrpld/app/misc/SHAMapStoreImp.h index 2b618f6538..793dedef37 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.h +++ b/src/xrpld/app/misc/SHAMapStoreImp.h @@ -112,8 +112,6 @@ private: // as of run() or before NetworkOPs* netOPs_ = nullptr; LedgerMaster* ledgerMaster_ = nullptr; - FullBelowCache* fullBelowCache_ = nullptr; - TreeNodeCache* treeNodeCache_ = nullptr; static constexpr auto nodeStoreName_ = "NodeStore"; From 2fa1c711d3b9dbb5e07aac35fc685c6c77b62743 Mon Sep 17 00:00:00 2001 From: Bart Thomee <11445373+bthomee@users.noreply.github.com> Date: Tue, 20 May 2025 09:50:13 -0400 Subject: [PATCH 7/8] Removed unused config values --- cfg/rippled-example.cfg | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/cfg/rippled-example.cfg b/cfg/rippled-example.cfg index 8fb7d00875..aa51b3e7ad 100644 --- a/cfg/rippled-example.cfg +++ b/cfg/rippled-example.cfg @@ -940,23 +940,7 @@ # # path Location to store the database # -# Optional keys -# -# cache_size Size of cache for database records. Default is 16384. -# Setting this value to 0 will use the default value. -# -# cache_age Length of time in minutes to keep database records -# cached. Default is 5 minutes. Setting this value to -# 0 will use the default value. -# -# Note: if neither cache_size nor cache_age is -# specified, the cache for database records will not -# be created. If only one of cache_size or cache_age -# is specified, the cache will be created using the -# default value for the unspecified parameter. -# -# Note: the cache will not be created if online_delete -# is specified. +# Optional keys for NuDB and RocksDB: # # fast_load Boolean. If set, load the last persisted ledger # from disk upon process start before syncing to @@ -964,8 +948,6 @@ # if sufficient IOPS capacity is available. # Default 0. # -# Optional keys for NuDB or RocksDB: -# # earliest_seq The default is 32570 to match the XRP ledger # network's earliest allowed sequence. Alternate # networks may set this value. Minimum value of 1. From 965fc75e8a91db785e1eb22fcb72a4ea7fff52bd Mon Sep 17 00:00:00 2001 From: Bart Thomee <11445373+bthomee@users.noreply.github.com> Date: Tue, 20 May 2025 10:07:12 -0400 Subject: [PATCH 8/8] Reserve vector size --- src/xrpld/nodestore/detail/DatabaseNodeImp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xrpld/nodestore/detail/DatabaseNodeImp.cpp b/src/xrpld/nodestore/detail/DatabaseNodeImp.cpp index dff61c609d..57f4dc58f6 100644 --- a/src/xrpld/nodestore/detail/DatabaseNodeImp.cpp +++ b/src/xrpld/nodestore/detail/DatabaseNodeImp.cpp @@ -97,7 +97,7 @@ DatabaseNodeImp::fetchBatch(std::vector const& hashes) using namespace std::chrono; auto const before = steady_clock::now(); - std::vector batch; + std::vector batch{hashes.size()}; for (size_t i = 0; i < hashes.size(); ++i) { auto const& hash = hashes[i];