Optimize trust line caching:

The existing trust line caching code was suboptimal in that it stored redundant information, pinned SLEs into memory and required multiple memory allocations per cached object. This commit eliminates redundant data, reducing the size of cached objects and unpinning SLEs from memory, and uses value_types to avoid the need for `std::shared_ptr`. As a result of these changes, the effective size of a cached object, includes the overhead of the memory allocator and the `std::shared_ptr` should be reduced by at least 64 bytes. This is significant, as there can easily be tens of millions of these objects.
XRPLF · Mar 24, 2022 · 4d5459d · 4d5459d · Manuellechner · Apr 16, 2022
1 parent 59f5844
commit 4d5459d
Show file tree

Hide file tree

Showing 13 changed files with 255 additions and 214 deletions.
diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake
@@ -395,7 +395,7 @@ target_sources (rippled PRIVATE
   src/ripple/app/paths/Pathfinder.cpp
   src/ripple/app/paths/RippleCalc.cpp
   src/ripple/app/paths/RippleLineCache.cpp
-  src/ripple/app/paths/RippleState.cpp
+  src/ripple/app/paths/TrustLine.cpp
   src/ripple/app/paths/impl/BookStep.cpp
   src/ripple/app/paths/impl/DirectStep.cpp
   src/ripple/app/paths/impl/PaySteps.cpp

diff --git a/src/ripple/app/paths/AccountCurrencies.cpp b/src/ripple/app/paths/AccountCurrencies.cpp
@@ -33,24 +33,16 @@ accountSourceCurrencies(
     if (includeXRP)
         currencies.insert(xrpCurrency());
 
-    // List of ripple lines.
-    auto& rippleLines = lrCache->getRippleLines(account);
-
-    for (auto const& item : rippleLines)
+    for (auto const& rspEntry : lrCache->getRippleLines(account))
     {
-        auto rspEntry = (RippleState*)item.get();
-        assert(rspEntry);
-        if (!rspEntry)
-            continue;
-
-        auto& saBalance = rspEntry->getBalance();
+        auto& saBalance = rspEntry.getBalance();
 
         // Filter out non
         if (saBalance > beast::zero
             // Have IOUs to send.
-            || (rspEntry->getLimitPeer()
+            || (rspEntry.getLimitPeer()
                 // Peer extends credit.
-                && ((-saBalance) < rspEntry->getLimitPeer())))  // Credit left.
+                && ((-saBalance) < rspEntry.getLimitPeer())))  // Credit left.
         {
             currencies.insert(saBalance.getCurrency());
         }
@@ -72,19 +64,11 @@ accountDestCurrencies(
         currencies.insert(xrpCurrency());
     // Even if account doesn't exist
 
-    // List of ripple lines.
-    auto& rippleLines = lrCache->getRippleLines(account);
-
-    for (auto const& item : rippleLines)
+    for (auto const& rspEntry : lrCache->getRippleLines(account))
     {
-        auto rspEntry = (RippleState*)item.get();
-        assert(rspEntry);
-        if (!rspEntry)
-            continue;
-
-        auto& saBalance = rspEntry->getBalance();
+        auto& saBalance = rspEntry.getBalance();
 
-        if (saBalance < rspEntry->getLimit())  // Can take more
+        if (saBalance < rspEntry.getLimit())  // Can take more
             currencies.insert(saBalance.getCurrency());
     }
 

diff --git a/src/ripple/app/paths/Pathfinder.cpp b/src/ripple/app/paths/Pathfinder.cpp
@@ -717,30 +717,27 @@ Pathfinder::getPathsOut(
     {
         count = app_.getOrderBookDB().getBookSize(issue);
 
-        for (auto const& item : mRLCache->getRippleLines(account))
+        for (auto const& rspEntry : mRLCache->getRippleLines(account))
         {
-            RippleState* rspEntry = (RippleState*)item.get();
-
-            if (currency != rspEntry->getLimit().getCurrency())
+            if (currency != rspEntry.getLimit().getCurrency())
             {
             }
             else if (
-                rspEntry->getBalance() <= beast::zero &&
-                (!rspEntry->getLimitPeer() ||
-                 -rspEntry->getBalance() >= rspEntry->getLimitPeer() ||
-                 (bAuthRequired && !rspEntry->getAuth())))
+                rspEntry.getBalance() <= beast::zero &&
+                (!rspEntry.getLimitPeer() ||
+                 -rspEntry.getBalance() >= rspEntry.getLimitPeer() ||
+                 (bAuthRequired && !rspEntry.getAuth())))
             {
             }
-            else if (
-                isDstCurrency && dstAccount == rspEntry->getAccountIDPeer())
+            else if (isDstCurrency && dstAccount == rspEntry.getAccountIDPeer())
             {
                 count += 10000;  // count a path to the destination extra
             }
-            else if (rspEntry->getNoRipplePeer())
+            else if (rspEntry.getNoRipplePeer())
             {
                 // This probably isn't a useful path out
             }
-            else if (rspEntry->getFreezePeer())
+            else if (rspEntry.getFreezePeer())
             {
                 // Not a useful path out
             }
@@ -940,15 +937,9 @@ Pathfinder::addLink(
                 AccountCandidates candidates;
                 candidates.reserve(rippleLines.size());
 
-                for (auto const& item : rippleLines)
+                for (auto const& rs : rippleLines)
                 {
-                    auto* rs = dynamic_cast<RippleState const*>(item.get());
-                    if (!rs)
-                    {
-                        JLOG(j_.error()) << "Couldn't decipher RippleState";
-                        continue;
-                    }
-                    auto const& acct = rs->getAccountIDPeer();
+                    auto const& acct = rs.getAccountIDPeer();
 
                     if (hasEffectiveDestination && (acct == mDstAccount))
                     {
@@ -963,18 +954,18 @@ Pathfinder::addLink(
                         continue;
                     }
 
-                    if ((uEndCurrency == rs->getLimit().getCurrency()) &&
+                    if ((uEndCurrency == rs.getLimit().getCurrency()) &&
                         !currentPath.hasSeen(acct, uEndCurrency, acct))
                     {
                         // path is for correct currency and has not been seen
-                        if (rs->getBalance() <= beast::zero &&
-                            (!rs->getLimitPeer() ||
-                             -rs->getBalance() >= rs->getLimitPeer() ||
-                             (bRequireAuth && !rs->getAuth())))
+                        if (rs.getBalance() <= beast::zero &&
+                            (!rs.getLimitPeer() ||
+                             -rs.getBalance() >= rs.getLimitPeer() ||
+                             (bRequireAuth && !rs.getAuth())))
                         {
                             // path has no credit
                         }
-                        else if (bIsNoRippleOut && rs->getNoRipple())
+                        else if (bIsNoRippleOut && rs.getNoRipple())
                         {
                             // Can't leave on this path
                         }

diff --git a/src/ripple/app/paths/RippleLineCache.cpp b/src/ripple/app/paths/RippleLineCache.cpp
@@ -18,6 +18,7 @@
 //==============================================================================
 
 #include <ripple/app/paths/RippleLineCache.h>
+#include <ripple/app/paths/TrustLine.h>
 #include <ripple/ledger/OpenView.h>
 
 namespace ripple {
@@ -30,18 +31,17 @@ RippleLineCache::RippleLineCache(std::shared_ptr<ReadView const> const& ledger)
     mLedger = std::make_shared<OpenView>(&*ledger, ledger);
 }
 
-std::vector<RippleState::pointer> const&
+std::vector<PathFindTrustLine> const&
 RippleLineCache::getRippleLines(AccountID const& accountID)
 {
     AccountKey key(accountID, hasher_(accountID));
 
     std::lock_guard sl(mLock);
 
-    auto [it, inserted] =
-        lines_.emplace(key, std::vector<RippleState::pointer>());
+    auto [it, inserted] = lines_.emplace(key, std::vector<PathFindTrustLine>());
 
     if (inserted)
-        it->second = getRippleStateItems(accountID, *mLedger);
+        it->second = PathFindTrustLine::getItems(accountID, *mLedger);
 
     return it->second;
 }

diff --git a/src/ripple/app/paths/RippleLineCache.h b/src/ripple/app/paths/RippleLineCache.h
@@ -21,8 +21,10 @@
 #define RIPPLE_APP_PATHS_RIPPLELINECACHE_H_INCLUDED
 
 #include <ripple/app/ledger/Ledger.h>
-#include <ripple/app/paths/RippleState.h>
+#include <ripple/app/paths/TrustLine.h>
+#include <ripple/basics/CountedObject.h>
 #include <ripple/basics/hardened_hash.h>
+
 #include <cstddef>
 #include <memory>
 #include <mutex>
@@ -31,7 +33,7 @@
 namespace ripple {
 
 // Used by Pathfinder
-class RippleLineCache
+class RippleLineCache : public CountedObject<RippleLineCache>
 {
 public:
     explicit RippleLineCache(std::shared_ptr<ReadView const> const& l);
@@ -42,7 +44,7 @@ class RippleLineCache
         return mLedger;
     }
 
-    std::vector<RippleState::pointer> const&
+    std::vector<PathFindTrustLine> const&
     getRippleLines(AccountID const& accountID);
 
 private:
@@ -90,7 +92,7 @@ class RippleLineCache
         };
     };
 
-    hash_map<AccountKey, std::vector<RippleState::pointer>, AccountKey::Hash>
+    hash_map<AccountKey, std::vector<PathFindTrustLine>, AccountKey::Hash>
         lines_;
 };
 

diff --git a/src/ripple/app/paths/RippleState.cpp b/src/ripple/app/paths/RippleState.cpp