From 15aad1cb24f68c242a636647e77362a4f3e2da49 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Thu, 21 Aug 2014 16:31:23 -0700 Subject: [PATCH] Optimize pathfinding operations (RIPD-537): * Calculate and cache Account hashes without holding locks. * Fast hash-based path element comparison. * Use emplace instead of find/insert --- src/ripple/module/app/paths/Pathfinder.cpp | 21 ++++---- .../module/app/paths/RippleLineCache.cpp | 22 +++----- src/ripple/module/app/paths/RippleLineCache.h | 51 ++++++++++++++++++- .../module/data/protocol/SerializedTypes.cpp | 23 +++++++++ .../module/data/protocol/SerializedTypes.h | 51 +++++++++++++------ 5 files changed, 126 insertions(+), 42 deletions(-) diff --git a/src/ripple/module/app/paths/Pathfinder.cpp b/src/ripple/module/app/paths/Pathfinder.cpp index 6a90e5008..c47a0c07c 100644 --- a/src/ripple/module/app/paths/Pathfinder.cpp +++ b/src/ripple/module/app/paths/Pathfinder.cpp @@ -500,19 +500,18 @@ int Pathfinder::getPathsOut ( Currency const& currencyID, Account const& accountID, bool isDstCurrency, Account const& dstAccount) { - Issue issue (currencyID, accountID); - auto it = mPathsOutCountMap.find (issue); + Issue const issue (currencyID, accountID); - if (it != mPathsOutCountMap.end ()) - return it->second; + auto it = mPathsOutCountMap.emplace (issue, 0); - auto sleAccount = mLedger->getSLEi ( - Ledger::getAccountRootIndex (accountID)); + // If it was already present, return the stored number of paths + if (!it.second) + return it.first->second; + + auto sleAccount = mLedger->getSLEi (Ledger::getAccountRootIndex (accountID)); + if (!sleAccount) - { - mPathsOutCountMap[issue] = 0; return 0; - } int aFlags = sleAccount->getFieldU32(sfFlags); bool const bAuthRequired = (aFlags & lsfRequireAuth) != 0; @@ -523,7 +522,7 @@ int Pathfinder::getPathsOut ( if (!bFrozen) { - count = getApp().getOrderBookDB().getBookSize({currencyID, accountID}); + count = getApp().getOrderBookDB().getBookSize(issue); for (auto const& item : mRLCache->getRippleLines (accountID)) { @@ -552,7 +551,7 @@ int Pathfinder::getPathsOut ( ++count; } } - mPathsOutCountMap[issue] = count; + it.first->second = count; return count; } diff --git a/src/ripple/module/app/paths/RippleLineCache.cpp b/src/ripple/module/app/paths/RippleLineCache.cpp index 09f359a0e..7374518a5 100644 --- a/src/ripple/module/app/paths/RippleLineCache.cpp +++ b/src/ripple/module/app/paths/RippleLineCache.cpp @@ -24,25 +24,19 @@ RippleLineCache::RippleLineCache (Ledger::ref l) { } -std::vector const& +RippleLineCache::RippleStateVector const& RippleLineCache::getRippleLines (Account const& accountID) { - - { - ScopedLockType sl (mLock); - - auto it = mRLMap.find (accountID); - if (it != mRLMap.end ()) - return it->second; - } - - // It's not in the cache, so build it - auto lines = ripple::getRippleStateItems (accountID, mLedger); + AccountKey key (accountID); ScopedLockType sl (mLock); - // We must return a reference to the cached version - return mRLMap.emplace (accountID, std::move (lines)).first->second; + auto it = mRLMap.emplace (key, RippleStateVector ()); + + if (it.second) + it.first->second = ripple::getRippleStateItems (accountID, mLedger); + + return it.first->second; } } // ripple diff --git a/src/ripple/module/app/paths/RippleLineCache.h b/src/ripple/module/app/paths/RippleLineCache.h index 9e5370f70..6d341ee3d 100644 --- a/src/ripple/module/app/paths/RippleLineCache.h +++ b/src/ripple/module/app/paths/RippleLineCache.h @@ -26,6 +26,7 @@ namespace ripple { class RippleLineCache { public: + typedef std::vector RippleStateVector; typedef std::shared_ptr pointer; typedef pointer const& ref; @@ -46,7 +47,55 @@ private: Ledger::pointer mLedger; - hash_map > mRLMap; + struct AccountKey + { + Account account_; + std::size_t hash_value_; + + AccountKey (Account const& account) + : account_ (account) + , hash_value_ (beast::hardened_hash<>{}(account)) + { + + } + + AccountKey (AccountKey const& other) + : account_ (other.account_) + , hash_value_ (other.hash_value_) + { + + } + + AccountKey& operator=(AccountKey const& other) + { + account_ = other.account_; + hash_value_ = other.hash_value_; + return *this; + } + + bool operator== (AccountKey const& lhs) const + { + return hash_value_ == lhs.hash_value_ && + account_ == lhs.account_; + } + + std::size_t + get_hash () const + { + return hash_value_; + } + + struct Hash + { + std::size_t + operator () (AccountKey const& key) const noexcept + { + return key.get_hash (); + } + }; + }; + + hash_map mRLMap; }; } // ripple diff --git a/src/ripple/module/data/protocol/SerializedTypes.cpp b/src/ripple/module/data/protocol/SerializedTypes.cpp index 56596449c..920d73009 100644 --- a/src/ripple/module/data/protocol/SerializedTypes.cpp +++ b/src/ripple/module/data/protocol/SerializedTypes.cpp @@ -187,6 +187,29 @@ void STAccount::setValueNCA (RippleAddress const& nca) setValueH160 (nca.getAccountID ()); } +std::size_t +STPathElement::get_hash (STPathElement const& element) +{ + std::size_t hash_account = 2654435761; + std::size_t hash_currency = 2654435761; + std::size_t hash_issuer = 2654435761; + + // NIKB NOTE: This doesn't have to be a secure hash as speed is more + // important. We don't even really need to fully hash the whole + // base_uint here, as a few bytes would do for our use. + + for (auto const x : element.getAccountID ()) + hash_account += (hash_account * 257) ^ x; + + for (auto const x : element.getCurrency ()) + hash_currency += (hash_currency * 509) ^ x; + + for (auto const x : element.getIssuerID ()) + hash_issuer += (hash_issuer * 911) ^ x; + + return (hash_account ^ hash_currency ^ hash_issuer); +} + STPathSet* STPathSet::construct (SerializerIterator& s, SField::ref name) { std::vector paths; diff --git a/src/ripple/module/data/protocol/SerializedTypes.h b/src/ripple/module/data/protocol/SerializedTypes.h index 37493039b..22d4883ac 100644 --- a/src/ripple/module/data/protocol/SerializedTypes.h +++ b/src/ripple/module/data/protocol/SerializedTypes.h @@ -192,29 +192,44 @@ public: // Combination of all types. }; +private: + static + std::size_t + get_hash (STPathElement const& element); + public: STPathElement ( Account const& account, Currency const& currency, Account const& issuer, bool forceCurrency = false) - : mAccountID (account), mCurrencyID (currency), mIssuerID (issuer) + : mType (typeNone), mAccountID (account), mCurrencyID (currency) + , mIssuerID (issuer), is_offer_ (isXRP(mAccountID)) { - mType = - (account.isZero () ? 0 : STPathElement::typeAccount) - | ((currency.isZero () && !forceCurrency) ? 0 : - STPathElement::typeCurrency) - | (issuer.isZero () ? 0 : STPathElement::typeIssuer); + if (!is_offer_) + mType |= typeAccount; + + if (forceCurrency || !isXRP(currency)) + mType |= typeCurrency; + + if (!isXRP(issuer)) + mType |= typeIssuer; + + hash_value_ = get_hash (*this); } STPathElement ( unsigned int uType, Account const& account, Currency const& currency, Account const& issuer) - : mType (uType), mAccountID (account), mCurrencyID (currency), - mIssuerID (issuer) - {} + : mType (uType), mAccountID (account), mCurrencyID (currency) + , mIssuerID (issuer), is_offer_ (isXRP(mAccountID)) + { + hash_value_ = get_hash (*this); + } STPathElement () - : mType (0) - {} + : mType (typeNone), is_offer_ (true) + { + hash_value_ = get_hash (*this); + } int getNodeType () const { @@ -222,7 +237,7 @@ public: } bool isOffer () const { - return mAccountID.isZero (); + return is_offer_; } bool isAccount () const { @@ -246,10 +261,11 @@ public: bool operator== (const STPathElement& t) const { - return (mType & typeAccount) == (t.mType & typeAccount) - && mAccountID == t.mAccountID - && mCurrencyID == t.mCurrencyID - && mIssuerID == t.mIssuerID; + return (mType & typeAccount) == (t.mType & typeAccount) && + hash_value_ == t.hash_value_ && + mAccountID == t.mAccountID && + mCurrencyID == t.mCurrencyID && + mIssuerID == t.mIssuerID; } private: @@ -257,6 +273,9 @@ private: Account mAccountID; Currency mCurrencyID; Account mIssuerID; + + bool is_offer_; + std::size_t hash_value_; }; //------------------------------------------------------------------------------