diff --git a/include/xrpl/basics/SHAMapHash.h b/include/xrpl/basics/SHAMapHash.h index 12aef37dd0..df8fb8d7e5 100644 --- a/include/xrpl/basics/SHAMapHash.h +++ b/include/xrpl/basics/SHAMapHash.h @@ -2,7 +2,6 @@ #define XRPL_BASICS_SHAMAP_HASH_H_INCLUDED #include -#include #include diff --git a/include/xrpl/basics/TaggedCache.h b/include/xrpl/basics/TaggedCache.h index 77b8b3c6d4..276eaabebd 100644 --- a/include/xrpl/basics/TaggedCache.h +++ b/include/xrpl/basics/TaggedCache.h @@ -71,9 +71,6 @@ public: int getCacheSize() const; - int - getTrackSize() const; - float getHitRate(); @@ -151,9 +148,6 @@ public: bool retrieve(key_type const& key, T& data); - mutex_type& - peekMutex(); - std::vector getKeys() const; @@ -174,11 +168,14 @@ public: private: SharedPointerType - initialFetch(key_type const& key, std::lock_guard const& l); + initialFetch(key_type const& key); void collect_metrics(); + Mutex& + lockPartition(key_type const& key) const; + private: struct Stats { @@ -281,8 +278,8 @@ private: [[maybe_unused]] clock_type::time_point const& now, typename KeyValueCacheType::map_type& partition, SweptPointersVector& stuffToSweep, - std::atomic& allRemovals, - std::lock_guard const&); + std::atomic& allRemoval, + Mutex& partitionLock); [[nodiscard]] std::thread sweepHelper( @@ -291,14 +288,12 @@ private: typename KeyOnlyCacheType::map_type& partition, SweptPointersVector&, std::atomic& allRemovals, - std::lock_guard const&); + Mutex& partitionLock); beast::Journal m_journal; clock_type& m_clock; Stats m_stats; - mutex_type mutable m_mutex; - // Used for logging std::string m_name; @@ -309,10 +304,11 @@ private: clock_type::duration const m_target_age; // Number of items cached - int m_cache_count; + std::atomic m_cache_count; cache_type m_cache; // Hold strong reference to recent objects - std::uint64_t m_hits; - std::uint64_t m_misses; + std::atomic m_hits; + std::atomic m_misses; + mutable std::vector partitionLocks_; }; } // namespace ripple diff --git a/include/xrpl/basics/TaggedCache.ipp b/include/xrpl/basics/TaggedCache.ipp index 558198883c..454a1c807b 100644 --- a/include/xrpl/basics/TaggedCache.ipp +++ b/include/xrpl/basics/TaggedCache.ipp @@ -3,6 +3,7 @@ #include #include +#include namespace ripple { @@ -41,6 +42,7 @@ inline TaggedCache< , m_hits(0) , m_misses(0) { + partitionLocks_ = std::vector(m_cache.partitions()); } template < @@ -86,8 +88,13 @@ TaggedCache< KeyEqual, Mutex>::size() const { - std::lock_guard lock(m_mutex); - return m_cache.size(); + std::size_t totalSize = 0; + for (size_t i = 0; i < partitionLocks_.size(); ++i) + { + std::lock_guard lock(partitionLocks_[i]); + totalSize += m_cache.map()[i].size(); + } + return totalSize; } template < @@ -110,32 +117,7 @@ TaggedCache< KeyEqual, Mutex>::getCacheSize() const { - std::lock_guard lock(m_mutex); - return m_cache_count; -} - -template < - class Key, - class T, - bool IsKeyCache, - class SharedWeakUnionPointer, - class SharedPointerType, - class Hash, - class KeyEqual, - class Mutex> -inline int -TaggedCache< - Key, - T, - IsKeyCache, - SharedWeakUnionPointer, - SharedPointerType, - Hash, - KeyEqual, - Mutex>::getTrackSize() const -{ - std::lock_guard lock(m_mutex); - return m_cache.size(); + return m_cache_count.load(std::memory_order_relaxed); } template < @@ -158,9 +140,10 @@ TaggedCache< KeyEqual, Mutex>::getHitRate() { - 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)); + auto const hits = m_hits.load(std::memory_order_relaxed); + auto const misses = m_misses.load(std::memory_order_relaxed); + float const total = float(hits + misses); + return hits * (100.0f / std::max(1.0f, total)); } template < @@ -183,9 +166,12 @@ TaggedCache< KeyEqual, Mutex>::clear() { - std::lock_guard lock(m_mutex); + for (auto& mutex : partitionLocks_) + mutex.lock(); m_cache.clear(); - m_cache_count = 0; + for (auto& mutex : partitionLocks_) + mutex.unlock(); + m_cache_count.store(0, std::memory_order_relaxed); } template < @@ -208,11 +194,9 @@ TaggedCache< KeyEqual, Mutex>::reset() { - std::lock_guard lock(m_mutex); - m_cache.clear(); - m_cache_count = 0; - m_hits = 0; - m_misses = 0; + clear(); + m_hits.store(0, std::memory_order_relaxed); + m_misses.store(0, std::memory_order_relaxed); } template < @@ -236,7 +220,7 @@ TaggedCache< KeyEqual, Mutex>::touch_if_exists(KeyComparable const& key) { - std::lock_guard lock(m_mutex); + std::lock_guard lock(lockPartition(key)); auto const iter(m_cache.find(key)); if (iter == m_cache.end()) { @@ -278,8 +262,6 @@ TaggedCache< auto const start = std::chrono::steady_clock::now(); { - std::lock_guard lock(m_mutex); - if (m_target_size == 0 || (static_cast(m_cache.size()) <= m_target_size)) { @@ -311,12 +293,13 @@ TaggedCache< m_cache.map()[p], allStuffToSweep[p], allRemovals, - lock)); + partitionLocks_[p])); } for (std::thread& worker : workers) worker.join(); - m_cache_count -= allRemovals; + int removals = allRemovals.load(std::memory_order_relaxed); + m_cache_count.fetch_sub(removals, std::memory_order_relaxed); } // At this point allStuffToSweep will go out of scope outside the lock // and decrement the reference count on each strong pointer. @@ -350,7 +333,8 @@ TaggedCache< { // Remove from cache, if !valid, remove from map too. Returns true if // removed from cache - std::lock_guard lock(m_mutex); + + std::lock_guard lock(lockPartition(key)); auto cit = m_cache.find(key); @@ -363,7 +347,7 @@ TaggedCache< if (entry.isCached()) { - --m_cache_count; + m_cache_count.fetch_sub(1, std::memory_order_relaxed); entry.ptr.convertToWeak(); ret = true; } @@ -401,17 +385,16 @@ TaggedCache< { // Return canonical value, store if needed, refresh in cache // Return values: true=we had the data already - std::lock_guard lock(m_mutex); + std::lock_guard lock(lockPartition(key)); auto cit = m_cache.find(key); - if (cit == m_cache.end()) { m_cache.emplace( std::piecewise_construct, std::forward_as_tuple(key), std::forward_as_tuple(m_clock.now(), data)); - ++m_cache_count; + m_cache_count.fetch_add(1, std::memory_order_relaxed); return false; } @@ -460,12 +443,12 @@ TaggedCache< data = cachedData; } - ++m_cache_count; + m_cache_count.fetch_add(1, std::memory_order_relaxed); return true; } entry.ptr = data; - ++m_cache_count; + m_cache_count.fetch_add(1, std::memory_order_relaxed); return false; } @@ -541,10 +524,11 @@ TaggedCache< KeyEqual, Mutex>::fetch(key_type const& key) { - std::lock_guard l(m_mutex); - auto ret = initialFetch(key, l); + std::lock_guard lock(lockPartition(key)); + + auto ret = initialFetch(key); if (!ret) - ++m_misses; + m_misses.fetch_add(1, std::memory_order_relaxed); return ret; } @@ -608,8 +592,8 @@ TaggedCache< Mutex>::insert(key_type const& key) -> std::enable_if_t { - std::lock_guard lock(m_mutex); clock_type::time_point const now(m_clock.now()); + std::lock_guard lock(lockPartition(key)); auto [it, inserted] = m_cache.emplace( std::piecewise_construct, std::forward_as_tuple(key), @@ -649,29 +633,6 @@ TaggedCache< return true; } -template < - class Key, - class T, - bool IsKeyCache, - class SharedWeakUnionPointer, - class SharedPointerType, - class Hash, - class KeyEqual, - class Mutex> -inline auto -TaggedCache< - Key, - T, - IsKeyCache, - SharedWeakUnionPointer, - SharedPointerType, - Hash, - KeyEqual, - Mutex>::peekMutex() -> mutex_type& -{ - return m_mutex; -} - template < class Key, class T, @@ -695,10 +656,13 @@ TaggedCache< std::vector v; { - std::lock_guard lock(m_mutex); v.reserve(m_cache.size()); - for (auto const& _ : m_cache) - v.push_back(_.first); + for (std::size_t i = 0; i < partitionLocks_.size(); ++i) + { + std::lock_guard lock(partitionLocks_[i]); + for (auto const& entry : m_cache.map()[i]) + v.push_back(entry.first); + } } return v; @@ -724,11 +688,12 @@ TaggedCache< KeyEqual, Mutex>::rate() const { - std::lock_guard lock(m_mutex); - auto const tot = m_hits + m_misses; + auto const hits = m_hits.load(std::memory_order_relaxed); + auto const misses = m_misses.load(std::memory_order_relaxed); + auto const tot = hits + misses; if (tot == 0) - return 0; - return double(m_hits) / tot; + return 0.0; + return double(hits) / tot; } template < @@ -752,18 +717,16 @@ 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; - } + std::lock_guard lock(lockPartition(digest)); + + if (auto ret = initialFetch(digest)) + return ret; auto sle = h(); if (!sle) return {}; - std::lock_guard l(m_mutex); - ++m_misses; + m_misses.fetch_add(1, std::memory_order_relaxed); auto const [it, inserted] = m_cache.emplace(digest, Entry(m_clock.now(), std::move(sle))); if (!inserted) @@ -790,9 +753,10 @@ TaggedCache< SharedPointerType, Hash, KeyEqual, - Mutex>:: - initialFetch(key_type const& key, std::lock_guard const& l) + Mutex>::initialFetch(key_type const& key) { + std::lock_guard lock(lockPartition(key)); + auto cit = m_cache.find(key); if (cit == m_cache.end()) return {}; @@ -800,7 +764,7 @@ TaggedCache< Entry& entry = cit->second; if (entry.isCached()) { - ++m_hits; + m_hits.fetch_add(1, std::memory_order_relaxed); entry.touch(m_clock.now()); return entry.ptr.getStrong(); } @@ -808,12 +772,13 @@ TaggedCache< if (entry.isCached()) { // independent of cache size, so not counted as a hit - ++m_cache_count; + m_cache_count.fetch_add(1, std::memory_order_relaxed); entry.touch(m_clock.now()); return entry.ptr.getStrong(); } m_cache.erase(cit); + return {}; } @@ -842,10 +807,11 @@ TaggedCache< { beast::insight::Gauge::value_type hit_rate(0); { - std::lock_guard lock(m_mutex); - auto const total(m_hits + m_misses); + auto const hits = m_hits.load(std::memory_order_relaxed); + auto const misses = m_misses.load(std::memory_order_relaxed); + auto const total = hits + misses; if (total != 0) - hit_rate = (m_hits * 100) / total; + hit_rate = (hits * 100) / total; } m_stats.hit_rate.set(hit_rate); } @@ -876,12 +842,16 @@ TaggedCache< typename KeyValueCacheType::map_type& partition, SweptPointersVector& stuffToSweep, std::atomic& allRemovals, - std::lock_guard const&) + Mutex& partitionLock) { return std::thread([&, this]() { + beast::setCurrentThreadName("sweep-KVCache"); + int cacheRemovals = 0; int mapRemovals = 0; + std::lock_guard lock(partitionLock); + // Keep references to all the stuff we sweep // so that we can destroy them outside the lock. stuffToSweep.reserve(partition.size()); @@ -965,12 +935,16 @@ TaggedCache< typename KeyOnlyCacheType::map_type& partition, SweptPointersVector&, std::atomic& allRemovals, - std::lock_guard const&) + Mutex& partitionLock) { return std::thread([&, this]() { + beast::setCurrentThreadName("sweep-KCache"); + int cacheRemovals = 0; int mapRemovals = 0; + std::lock_guard lock(partitionLock); + // Keep references to all the stuff we sweep // so that we can destroy them outside the lock. { @@ -1005,6 +979,29 @@ TaggedCache< }); } +template < + class Key, + class T, + bool IsKeyCache, + class SharedWeakUnionPointer, + class SharedPointerType, + class Hash, + class KeyEqual, + class Mutex> +inline Mutex& +TaggedCache< + Key, + T, + IsKeyCache, + SharedWeakUnionPointer, + SharedPointerType, + Hash, + KeyEqual, + Mutex>::lockPartition(key_type const& key) const +{ + return partitionLocks_[m_cache.partition_index(key)]; +} + } // namespace ripple #endif diff --git a/include/xrpl/basics/partitioned_unordered_map.h b/include/xrpl/basics/partitioned_unordered_map.h index 01872963fd..d3afcd348b 100644 --- a/include/xrpl/basics/partitioned_unordered_map.h +++ b/include/xrpl/basics/partitioned_unordered_map.h @@ -258,6 +258,12 @@ public: return map_; } + partition_map_type const& + map() const + { + return map_; + } + iterator begin() { @@ -302,6 +308,12 @@ public: return cend(); } + std::size_t + partition_index(key_type const& key) const + { + return partitioner(key); + } + private: template void diff --git a/src/test/basics/TaggedCache_test.cpp b/src/test/basics/TaggedCache_test.cpp index be0eb5d77b..9cf0c5086c 100644 --- a/src/test/basics/TaggedCache_test.cpp +++ b/src/test/basics/TaggedCache_test.cpp @@ -39,10 +39,10 @@ public: // Insert an item, retrieve it, and age it so it gets purged. { BEAST_EXPECT(c.getCacheSize() == 0); - BEAST_EXPECT(c.getTrackSize() == 0); + BEAST_EXPECT(c.size() == 0); BEAST_EXPECT(!c.insert(1, "one")); BEAST_EXPECT(c.getCacheSize() == 1); - BEAST_EXPECT(c.getTrackSize() == 1); + BEAST_EXPECT(c.size() == 1); { std::string s; @@ -53,7 +53,7 @@ public: ++clock; c.sweep(); BEAST_EXPECT(c.getCacheSize() == 0); - BEAST_EXPECT(c.getTrackSize() == 0); + BEAST_EXPECT(c.size() == 0); } // Insert an item, maintain a strong pointer, age it, and @@ -61,7 +61,7 @@ public: { BEAST_EXPECT(!c.insert(2, "two")); BEAST_EXPECT(c.getCacheSize() == 1); - BEAST_EXPECT(c.getTrackSize() == 1); + BEAST_EXPECT(c.size() == 1); { auto p = c.fetch(2); @@ -69,14 +69,14 @@ public: ++clock; c.sweep(); BEAST_EXPECT(c.getCacheSize() == 0); - BEAST_EXPECT(c.getTrackSize() == 1); + BEAST_EXPECT(c.size() == 1); } // Make sure its gone now that our reference is gone ++clock; c.sweep(); BEAST_EXPECT(c.getCacheSize() == 0); - BEAST_EXPECT(c.getTrackSize() == 0); + BEAST_EXPECT(c.size() == 0); } // Insert the same key/value pair and make sure we get the same result @@ -92,7 +92,7 @@ public: ++clock; c.sweep(); BEAST_EXPECT(c.getCacheSize() == 0); - BEAST_EXPECT(c.getTrackSize() == 0); + BEAST_EXPECT(c.size() == 0); } // Put an object in but keep a strong pointer to it, advance the clock a @@ -102,24 +102,24 @@ public: // Put an object in BEAST_EXPECT(!c.insert(4, "four")); BEAST_EXPECT(c.getCacheSize() == 1); - BEAST_EXPECT(c.getTrackSize() == 1); + BEAST_EXPECT(c.size() == 1); { // Keep a strong pointer to it auto const p1 = c.fetch(4); BEAST_EXPECT(p1 != nullptr); BEAST_EXPECT(c.getCacheSize() == 1); - BEAST_EXPECT(c.getTrackSize() == 1); + BEAST_EXPECT(c.size() == 1); // Advance the clock a lot ++clock; c.sweep(); BEAST_EXPECT(c.getCacheSize() == 0); - BEAST_EXPECT(c.getTrackSize() == 1); + BEAST_EXPECT(c.size() == 1); // Canonicalize a new object with the same key auto p2 = std::make_shared("four"); BEAST_EXPECT(c.canonicalize_replace_client(4, p2)); BEAST_EXPECT(c.getCacheSize() == 1); - BEAST_EXPECT(c.getTrackSize() == 1); + BEAST_EXPECT(c.size() == 1); // Make sure we get the original object BEAST_EXPECT(p1.get() == p2.get()); } @@ -127,7 +127,7 @@ public: ++clock; c.sweep(); BEAST_EXPECT(c.getCacheSize() == 0); - BEAST_EXPECT(c.getTrackSize() == 0); + BEAST_EXPECT(c.size() == 0); } } }; diff --git a/src/xrpld/app/ledger/LedgerHistory.cpp b/src/xrpld/app/ledger/LedgerHistory.cpp index 4f2660c70a..389aa5c064 100644 --- a/src/xrpld/app/ledger/LedgerHistory.cpp +++ b/src/xrpld/app/ledger/LedgerHistory.cpp @@ -44,8 +44,6 @@ LedgerHistory::insert( ledger->stateMap().getHash().isNonZero(), "ripple::LedgerHistory::insert : nonzero hash"); - std::unique_lock sl(m_ledgers_by_hash.peekMutex()); - bool const alreadyHad = m_ledgers_by_hash.canonicalize_replace_cache( ledger->info().hash, ledger); if (validated) @@ -57,7 +55,6 @@ LedgerHistory::insert( LedgerHash LedgerHistory::getLedgerHash(LedgerIndex index) { - std::unique_lock sl(m_ledgers_by_hash.peekMutex()); if (auto it = mLedgersByIndex.find(index); it != mLedgersByIndex.end()) return it->second; return {}; @@ -67,13 +64,11 @@ std::shared_ptr LedgerHistory::getLedgerBySeq(LedgerIndex index) { { - std::unique_lock sl(m_ledgers_by_hash.peekMutex()); auto it = mLedgersByIndex.find(index); if (it != mLedgersByIndex.end()) { uint256 hash = it->second; - sl.unlock(); return getLedgerByHash(hash); } } @@ -89,7 +84,6 @@ LedgerHistory::getLedgerBySeq(LedgerIndex index) { // Add this ledger to the local tracking by index - std::unique_lock sl(m_ledgers_by_hash.peekMutex()); XRPL_ASSERT( ret->isImmutable(), @@ -439,8 +433,6 @@ LedgerHistory::builtLedger( XRPL_ASSERT( !hash.isZero(), "ripple::LedgerHistory::builtLedger : nonzero hash"); - std::unique_lock sl(m_consensus_validated.peekMutex()); - auto entry = std::make_shared(); m_consensus_validated.canonicalize_replace_client(index, entry); @@ -481,8 +473,6 @@ LedgerHistory::validatedLedger( !hash.isZero(), "ripple::LedgerHistory::validatedLedger : nonzero hash"); - std::unique_lock sl(m_consensus_validated.peekMutex()); - auto entry = std::make_shared(); m_consensus_validated.canonicalize_replace_client(index, entry); @@ -516,10 +506,9 @@ LedgerHistory::validatedLedger( bool LedgerHistory::fixIndex(LedgerIndex ledgerIndex, LedgerHash const& ledgerHash) { - std::unique_lock sl(m_ledgers_by_hash.peekMutex()); + auto ledger = m_ledgers_by_hash.fetch(ledgerHash); auto it = mLedgersByIndex.find(ledgerIndex); - - if ((it != mLedgersByIndex.end()) && (it->second != ledgerHash)) + if (ledger && (it != mLedgersByIndex.end()) && (it->second != ledgerHash)) { it->second = ledgerHash; return false; diff --git a/src/xrpld/rpc/handlers/GetCounts.cpp b/src/xrpld/rpc/handlers/GetCounts.cpp index 17b2c8565b..c90cd69421 100644 --- a/src/xrpld/rpc/handlers/GetCounts.cpp +++ b/src/xrpld/rpc/handlers/GetCounts.cpp @@ -95,7 +95,7 @@ getCountsJson(Application& app, int minObjectCount) ret[jss::treenode_cache_size] = app.getNodeFamily().getTreeNodeCache()->getCacheSize(); ret[jss::treenode_track_size] = - app.getNodeFamily().getTreeNodeCache()->getTrackSize(); + static_cast(app.getNodeFamily().getTreeNodeCache()->size()); std::string uptime; auto s = UptimeClock::now();