encasulate lock inside container

include/xrpl/basics/TaggedCache.h

@@ -170,9 +170,6 @@ public:
     bool
     retrieve(key_type const& key, T& data);
 
-    mutex_type&
-    peekMutex();
-
     std::vector<key_type>
     getKeys() const;
 

include/xrpl/basics/TaggedCache.ipp

@@ -668,29 +668,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,

src/xrpld/app/ledger/LedgerHistory.cpp

@@ -27,8 +27,6 @@
 
 namespace ripple {
 
-// FIXME: Need to clean up ledgers by index at some point
-
 LedgerHistory::LedgerHistory(
     beast::insight::Collector::ptr const& collector,
     Application& app)
@@ -47,6 +45,7 @@ LedgerHistory::LedgerHistory(
           std::chrono::minutes{5},
           stopwatch(),
           app_.journal("TaggedCache"))
+    , mLedgersByIndex(512)
     , j_(app.journal("LedgerHistory"))
 {
 }
@@ -63,8 +62,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)
@@ -76,25 +73,18 @@ 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;
+    if (auto p = mLedgersByIndex.get(index))
+        return *p;
     return {};
 }
 
 std::shared_ptr<Ledger const>
 LedgerHistory::getLedgerBySeq(LedgerIndex index)
 {
+    if (auto p = mLedgersByIndex.get(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);
-        }
+        uint256 const hash = *p;
+        return getLedgerByHash(hash);
     }
 
     std::shared_ptr<Ledger const> ret = loadByIndex(index, app_);
@@ -108,8 +98,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(),
             "ripple::LedgerHistory::getLedgerBySeq : immutable result ledger");
@@ -458,8 +446,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<cv_entry>();
     m_consensus_validated.canonicalize_replace_client(index, entry);
 
@@ -500,8 +486,6 @@ LedgerHistory::validatedLedger(
         !hash.isZero(),
         "ripple::LedgerHistory::validatedLedger : nonzero hash");
 
-    std::unique_lock sl(m_consensus_validated.peekMutex());
-
     auto entry = std::make_shared<cv_entry>();
     m_consensus_validated.canonicalize_replace_client(index, entry);
 
@@ -535,13 +519,13 @@ LedgerHistory::validatedLedger(
 bool
 LedgerHistory::fixIndex(LedgerIndex ledgerIndex, LedgerHash const& ledgerHash)
 {
-    std::unique_lock sl(m_ledgers_by_hash.peekMutex());
-    auto it = mLedgersByIndex.find(ledgerIndex);
-
-    if ((it != mLedgersByIndex.end()) && (it->second != ledgerHash))
+    if (auto cur = mLedgersByIndex.get(ledgerIndex))
     {
-        it->second = ledgerHash;
-        return false;
+        if (*cur != ledgerHash)
+        {
+            mLedgersByIndex.put(ledgerIndex, ledgerHash);
+            return false;
+        }
     }
     return true;
 }
@@ -556,11 +540,9 @@ LedgerHistory::clearLedgerCachePrior(LedgerIndex seq)
             m_ledgers_by_hash.del(it, false);
     }
 
-    std::unique_lock sl(m_ledgers_by_hash.peekMutex());
     JLOG(j_.debug()) << "mLedgersByIndex size before clear: "
                      << mLedgersByIndex.size();
-    auto first_keep = mLedgersByIndex.lower_bound(seq);
-    mLedgersByIndex.erase(mLedgersByIndex.begin(), first_keep);
+    mLedgersByIndex.eraseBefore(seq);
     JLOG(j_.debug()) << "mLedgersByIndex size after clear: "
                      << mLedgersByIndex.size();
 }

src/xrpld/app/ledger/LedgerHistory.h

@@ -22,6 +22,7 @@
 
 #include <xrpld/app/ledger/Ledger.h>
 #include <xrpld/app/main/Application.h>
+#include <xrpld/core/detail/LedgerIndexMap.h>
 
 #include <xrpl/beast/insight/Collector.h>
 #include <xrpl/protocol/RippleLedgerHash.h>
@@ -149,7 +150,8 @@ private:
     ConsensusValidated m_consensus_validated;
 
     // Maps ledger indexes to the corresponding hash.
-    std::map<LedgerIndex, LedgerHash> mLedgersByIndex;  // validated ledgers
+    ripple::LedgerIndexMap<LedgerIndex, LedgerHash>
+        mLedgersByIndex;  // validated ledgers
 
     beast::Journal j_;
 };

src/xrpld/core/detail/LedgerIndexMap.h

@@ -0,0 +1,154 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github.com/ripple/rippled
+    Copyright (c) 2012, 2013 Ripple Labs Inc.
+
+    Permission to use, copy, modify, and/or distribute this software for any
+    purpose  with  or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF  USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+#ifndef RIPPLE_APP_LEDGER_INDEX_MAP_H_INCLUDED
+#define RIPPLE_APP_LEDGER_INDEX_MAP_H_INCLUDED
+
+#include <mutex>
+#include <queue>
+#include <unordered_map>
+
+namespace ripple {
+
+template <class Key, class Mapped>
+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;
+
+    Mapped&
+    operator[](Key const& k)
+    {
+        std::lock_guard lock(mutex_);
+        auto [it, inserted] = data_.try_emplace(k);
+        if (inserted)
+            order_.push(k);
+        return it->second;
+    }
+
+    Mapped&
+    operator[](Key&& k)
+    {
+        std::lock_guard lock(mutex_);
+        auto [it, inserted] = data_.try_emplace(std::move(k));
+        if (inserted)
+            order_.push(it->first);
+        return it->second;
+    }
+
+    [[nodiscard]] Mapped*
+    get(Key const& k)
+    {
+        std::lock_guard lock(mutex_);
+        auto it = data_.find(k);
+        return it == data_.end() ? nullptr : &it->second;
+    }
+
+    [[nodiscard]] Mapped const*
+    get(LedgerIndex const& k) const
+    {
+        std::lock_guard lock(mutex_);
+        auto it = data_.find(k);
+        return it == data_.end() ? nullptr : &it->second;
+    }
+
+    template <class... Args>
+    Mapped&
+    put(Key const& k, Args&&... args)
+    {
+        std::lock_guard lock(mutex_);
+        auto [it, inserted] = data_.try_emplace(k, std::forward<Args>(args)...);
+        if (!inserted)
+            it->second = Mapped(std::forward<Args>(args)...);
+        else
+            order_.push(k);
+        return it->second;
+    }
+
+    bool
+    contains(Key const& k) const
+    {
+        std::lock_guard lock(mutex_);
+        return data_.find(k) != data_.end();
+    }
+
+    std::size_t
+    size() const noexcept
+    {
+        std::lock_guard lock(mutex_);
+        return data_.size();
+    }
+
+    bool
+    empty() const noexcept
+    {
+        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_);
+        std::size_t removed = 0;
+        while (!order_.empty())
+        {
+            Key const& k = order_.front();
+            if (!(k < cutoff))
+                break;
+            removed += (data_.erase(k) != 0);
+            order_.pop();
+        }
+        return removed;
+    }
+
+private:
+    std::unordered_map<Key, Mapped> data_;
+    std::queue<Key> order_;  // assumes non-decreasing inserts for O(k) purge
+    mutable std::mutex mutex_;
+};
+
+}  // namespace ripple
+
+#endif  // RIPPLE_APP_LEDGER_INDEX_MAP_H_INCLUDED