Optimize account_lines and account_offers (RIPD-587)

Conflicts:
	src/ripple/app/ledger/Ledger.h

src/ripple/app/ledger/Ledger.cpp

@@ -1382,6 +1382,89 @@ void Ledger::visitAccountItems (
 
 }
 
+bool Ledger::visitAccountItems (
+    Account const& accountID,
+    uint256 const& startAfter,
+    std::uint64_t const hint,
+    unsigned int limit,
+    std::function <bool (SLE::ref)> func) const
+{
+    // Visit each item in this account's owner directory
+    uint256 const rootIndex (Ledger::getOwnerDirIndex (accountID));
+    uint256 currentIndex (rootIndex);
+
+    // If startAfter is not zero try jumping to that page using the hint
+    if (startAfter.isNonZero ())
+    {
+        uint256 const hintIndex (getDirNodeIndex (rootIndex, hint));
+        SLE::pointer hintDir (getSLEi (hintIndex));
+        if (hintDir != nullptr)
+        {
+            for (auto const& node : hintDir->getFieldV256 (sfIndexes))
+            {
+                if (node == startAfter)
+                {
+                    // We found the hint, we can start here
+                    currentIndex = hintIndex;
+                    break;
+                }
+            }
+        }
+        
+        bool found (false);
+        for (;;)
+        {
+            SLE::pointer ownerDir (getSLEi (currentIndex));
+
+            if (! ownerDir || ownerDir->getType () != ltDIR_NODE)
+                return found;
+
+            for (auto const& node : ownerDir->getFieldV256 (sfIndexes))
+            {
+                if (!found)
+                {
+                    if (node == startAfter)
+                        found = true;
+                }
+                else if (func (getSLEi (node)) && limit-- <= 1)
+                {
+                    return found;
+                }
+            }
+
+            std::uint64_t const uNodeNext (ownerDir->getFieldU64 (sfIndexNext));
+
+            if (uNodeNext == 0)
+                return found;
+
+            currentIndex = Ledger::getDirNodeIndex (rootIndex, uNodeNext);
+        }
+    }
+    else
+    {
+        for (;;)
+        {
+            SLE::pointer ownerDir (getSLEi (currentIndex));
+
+            if (! ownerDir || ownerDir->getType () != ltDIR_NODE)
+                return true;
+
+            for (auto const& node : ownerDir->getFieldV256 (sfIndexes))
+            {
+                if (func (getSLEi (node)) && limit-- <= 1)
+                    return true;
+            }
+
+            std::uint64_t const uNodeNext (ownerDir->getFieldU64 (sfIndexNext));
+
+            if (uNodeNext == 0)
+                return true;
+
+            currentIndex = Ledger::getDirNodeIndex (rootIndex, uNodeNext);
+        }
+    }
+}
+
 static void visitHelper (
     std::function<void (SLE::ref)>& function, SHAMapItem::ref item)
 {
@@ -1785,12 +1868,20 @@ uint256 Ledger::getRippleStateIndex (
 {
     Serializer  s (62);
 
-    bool const bAltB = a < b;
+    s.add16 (spaceRipple);  //  2
 
-    s.add16 (spaceRipple);          //  2
-    s.add160 (bAltB ? a : b);       // 20
-    s.add160 (bAltB ? b : a);       // 20
-    s.add160 (currency);            // 20
+    if (a < b)
+    {
+        s.add160 (a);       // 20
+        s.add160 (b);       // 20
+    }
+    else
+    {
+        s.add160 (b);       // 20
+        s.add160 (a);       // 20
+    }
+
+    s.add160 (currency);    // 20
 
     return s.getSHA512Half ();
 }

src/ripple/app/ledger/Ledger.h

@@ -288,8 +288,15 @@ public:
     SLE::pointer getAccountRoot (Account const& accountID) const;
     SLE::pointer getAccountRoot (const RippleAddress & naAccountID) const;
     void updateSkipList ();
+        
     void visitAccountItems (
-        Account const& acctID, std::function<void (SLE::ref)>) const;
+        Account const& accountID, std::function<void (SLE::ref)>) const;
+    bool visitAccountItems (
+        Account const& accountID,
+        uint256 const& startAfter, // Entry to start after
+        std::uint64_t const hint,  // Hint which page to start at
+        unsigned int limit,
+        std::function <bool (SLE::ref)>) const;
     void visitStateItems (std::function<void (SLE::ref)>) const;
 
     // database functions (low-level)

src/ripple/rpc/handlers/AccountLines.cpp

@@ -21,6 +21,49 @@
 
 namespace ripple {
 
+struct VisitData
+{
+    std::vector <RippleState::pointer> items;
+    Account const& accountID;
+    RippleAddress const& rippleAddressPeer;
+    Account const& raPeerAccount;
+};
+
+void addLine (Json::Value& jsonLines, RippleState const& line)
+{
+    STAmount const& saBalance (line.getBalance ());
+    STAmount const& saLimit (line.getLimit ());
+    STAmount const& saLimitPeer (line.getLimitPeer ());
+    Json::Value& jPeer (jsonLines.append (Json::objectValue));
+
+    jPeer[jss::account] = to_string (line.getAccountIDPeer ());
+    // Amount reported is positive if current account holds other
+    // account's IOUs.
+    //
+    // Amount reported is negative if other account holds current
+    // account's IOUs.
+    jPeer[jss::balance] = saBalance.getText ();
+    jPeer[jss::currency] = saBalance.getHumanCurrency ();
+    jPeer[jss::limit] = saLimit.getText ();
+    jPeer[jss::limit_peer] = saLimitPeer.getText ();
+    jPeer[jss::quality_in]
+        = static_cast<Json::UInt> (line.getQualityIn ());
+    jPeer[jss::quality_out]
+        = static_cast<Json::UInt> (line.getQualityOut ());
+    if (line.getAuth ())
+        jPeer[jss::authorized] = true;
+    if (line.getAuthPeer ())
+        jPeer[jss::peer_authorized] = true;
+    if (line.getNoRipple ())
+        jPeer[jss::no_ripple] = true;
+    if (line.getNoRipplePeer ())
+        jPeer[jss::no_ripple_peer] = true;
+    if (line.getFreeze ())
+        jPeer[jss::freeze] = true;
+    if (line.getFreezePeer ())
+        jPeer[jss::freeze_peer] = true;
+}
+
 // {
 //   account: <account>|<account_public_key>
 //   account_index: <number>        // optional, defaults to 0.
@@ -34,48 +77,53 @@ Json::Value doAccountLines (RPC::Context& context)
     auto& params = context.params_;
 
     Ledger::pointer ledger;
-    Json::Value result = RPC::lookupLedger (params, ledger, context.netOps_);
+    Json::Value result (RPC::lookupLedger (params, ledger, context.netOps_));
 
-    if (!ledger)
+    if (! ledger)
         return result;
 
-    if (!params.isMember (jss::account))
+    if (! params.isMember (jss::account))
         return RPC::missing_field_error ("account");
 
-    std::string strIdent = params[jss::account].asString ();
-    bool bIndex = params.isMember (jss::account_index);
-    int iIndex = bIndex ? params[jss::account_index].asUInt () : 0;
-
-    RippleAddress   raAccount;
+    std::string strIdent (params[jss::account].asString ());
+    bool bIndex (params.isMember (jss::account_index));
+    int iIndex (bIndex ? params[jss::account_index].asUInt () : 0);
+    RippleAddress rippleAddress;
 
     result = RPC::accountFromString (
-        ledger, raAccount, bIndex, strIdent, iIndex, false, context.netOps_);
+        ledger, rippleAddress, bIndex, strIdent, iIndex, false, context.netOps_);
 
-    if (!result.empty ())
+    if (! result.empty ())
         return result;
 
-    std::string strPeer = params.isMember (jss::peer)
-            ? params[jss::peer].asString () : "";
-    bool bPeerIndex = params.isMember (jss::peer_index);
-    int iPeerIndex = bIndex ? params[jss::peer_index].asUInt () : 0;
+    if (! ledger->hasAccount (rippleAddress))
+        return rpcError (rpcACT_NOT_FOUND);
 
-    RippleAddress   raPeer;
+    std::string strPeer (params.isMember (jss::peer)
+        ? params[jss::peer].asString () : "");
+    bool bPeerIndex (params.isMember (jss::peer_index));
+    int iPeerIndex (bIndex ? params[jss::peer_index].asUInt () : 0);
 
-    if (!strPeer.empty ())
+    RippleAddress rippleAddressPeer;
+
+    if (! strPeer.empty ())
     {
-        result[jss::peer]    = raAccount.humanAccountID ();
+        result[jss::peer] = rippleAddress.humanAccountID ();
 
         if (bPeerIndex)
-            result[jss::peer_index]  = iPeerIndex;
+            result[jss::peer_index] = iPeerIndex;
 
-        result = RPC::accountFromString (
-            ledger, raPeer, bPeerIndex, strPeer, iPeerIndex, false,
-            context.netOps_);
+        result = RPC::accountFromString (ledger, rippleAddressPeer, bPeerIndex, strPeer,
+            iPeerIndex, false, context.netOps_);
 
-        if (!result.empty ())
+        if (! result.empty ())
             return result;
     }
 
+    Account raPeerAccount;
+    if (rippleAddressPeer.isValid ())
+        raPeerAccount = rippleAddressPeer.getAccountID ();
+
     unsigned int limit;
     if (params.isMember (jss::limit))
     {
@@ -88,88 +136,83 @@ Json::Value doAccountLines (RPC::Context& context)
         limit = RPC::Tuning::defaultLinesPerRequest;
     }
 
-    RippleAddress resumeAddress;    
+    Json::Value& jsonLines (result[jss::lines] = Json::arrayValue);   
+    Account const& raAccount(rippleAddress.getAccountID ());
+    VisitData visitData = { {}, raAccount, rippleAddressPeer, raPeerAccount };
+    unsigned int reserve (limit);
+    uint256 startAfter;
+    std::uint64_t startHint;
+
     if (params.isMember (jss::marker))
     {
-        if (!resumeAddress.setAccountID (params[jss::marker].asString ()))
-            return rpcError (rpcACT_MALFORMED);
-    }
+        // We have a start point. Use limit - 1 from the result and use the
+        // very last one for the resume.        
+        Json::Value const& marker (params[jss::marker]);
 
-    if (ledger->hasAccount (raAccount))
-    {
-        result[jss::account] = raAccount.humanAccountID ();
-        Json::Value& jsonLines = (result[jss::lines] = Json::arrayValue);
-
-        bool resume (! resumeAddress.isValid ());
-        unsigned int i (0);
-
-        for (auto const& item : getRippleStateItems (raAccount.getAccountID (), ledger))
-        {
-            RippleState const& line (*item.get ());
-            Account const& lineAccount (line.getAccountIDPeer ());
-            
-            if (! resume && resumeAddress.getAccountID () == lineAccount)
-                resume = true;
-
-            if (resume &&
-                (!raPeer.isValid () || raPeer.getAccountID () == lineAccount))
-            {
-                if (i < limit)
-                {
-                    STAmount const& saBalance = line.getBalance ();
-                    STAmount const& saLimit = line.getLimit ();
-                    STAmount const& saLimitPeer = line.getLimitPeer ();
-
-                    Json::Value& jPeer = jsonLines.append (Json::objectValue);
-
-                    jPeer[jss::account] = to_string (lineAccount);
-                    // Amount reported is positive if current account holds other
-                    // account's IOUs.
-                    //
-                    // Amount reported is negative if other account holds current
-                    // account's IOUs.
-                    jPeer[jss::balance] = saBalance.getText ();
-                    jPeer[jss::currency] = saBalance.getHumanCurrency ();
-                    jPeer[jss::limit] = saLimit.getText ();
-                    jPeer[jss::limit_peer] = saLimitPeer.getText ();
-                    jPeer[jss::quality_in]
-                        = static_cast<Json::UInt> (line.getQualityIn ());
-                    jPeer[jss::quality_out]
-                        = static_cast<Json::UInt> (line.getQualityOut ());
-                    if (line.getAuth ())
-                        jPeer[jss::authorized] = true;
-                    if (line.getAuthPeer ())
-                        jPeer[jss::peer_authorized] = true;
-                    if (line.getNoRipple ())
-                        jPeer[jss::no_ripple] = true;
-                    if (line.getNoRipplePeer ())
-                        jPeer[jss::no_ripple_peer] = true;
-                    if (line.getFreeze ())
-                        jPeer[jss::freeze] = true;
-                    if (line.getFreezePeer ())
-                        jPeer[jss::freeze_peer] = true;
-                    
-                    ++i;
-                }
-                else
-                {
-                    result[jss::limit] = limit;
-                    result[jss::marker] = to_string (lineAccount);
-                    break;
-                }
-            }
-        }
-
-        if (! resume)
+        if (! marker.isString ())
             return rpcError (rpcACT_MALFORMED);
 
-        context.loadType_ = Resource::feeMediumBurdenRPC;
+        startAfter.SetHex (marker.asString ());
+        SLE::pointer sleLine (ledger->getSLEi (startAfter));
+
+        if (sleLine == nullptr || sleLine->getType () != ltRIPPLE_STATE)
+            return rpcError (rpcINVALID_PARAMS);
+        
+        if (sleLine->getFieldAmount (sfLowLimit).getIssuer () == raAccount)
+            startHint = sleLine->getFieldU64 (sfLowNode);
+        else if (sleLine->getFieldAmount (sfHighLimit).getIssuer () == raAccount)
+            startHint = sleLine->getFieldU64 (sfHighNode);
+        else
+            return rpcError (rpcINVALID_PARAMS);
+
+        // Caller provided the first line (startAfter), add it as first result
+        auto const line (RippleState::makeItem (raAccount, sleLine));
+        if (line == nullptr)
+            return rpcError (rpcINVALID_PARAMS);
+
+        addLine (jsonLines, *line);
+        visitData.items.reserve (reserve);
     }
     else
     {
-        result    = rpcError (rpcACT_NOT_FOUND);
+        startHint = 0;
+        // We have no start point, limit should be one higher than requested.
+        visitData.items.reserve (++reserve);
     }
 
+    if (! ledger->visitAccountItems (raAccount, startAfter, startHint, reserve,
+        [&visitData](SLE::ref sleCur)
+        {
+            auto const line (RippleState::makeItem (visitData.accountID, sleCur));
+            if (line != nullptr &&
+                (! visitData.rippleAddressPeer.isValid () ||
+                visitData.raPeerAccount == line->getAccountIDPeer ()))
+            {
+                visitData.items.emplace_back (line);
+                return true;
+            }
+
+            return false;
+        }))
+    {
+        return rpcError (rpcINVALID_PARAMS);
+    }
+    
+    if (visitData.items.size () == reserve)
+    {
+        result[jss::limit] = limit;
+
+        RippleState::pointer line (visitData.items.back ());
+        result[jss::marker] = to_string (line->peekSLE ().getIndex ());
+        visitData.items.pop_back ();
+    }
+
+    result[jss::account] = rippleAddress.humanAccountID ();
+
+    for (auto const& item : visitData.items)
+        addLine (jsonLines, *item.get ());
+
+    context.loadType_ = Resource::feeMediumBurdenRPC;
     return result;
 }
 

src/ripple/rpc/handlers/AccountOffers.cpp

@@ -44,24 +44,23 @@ Json::Value doAccountOffers (RPC::Context& context)
 
     std::string strIdent (params[jss::account].asString ());
     bool bIndex (params.isMember (jss::account_index));
-    int iIndex (bIndex ? params[jss::account_index].asUInt () : 0);
+    int const iIndex (bIndex ? params[jss::account_index].asUInt () : 0);
 
-    RippleAddress raAccount;
+    RippleAddress rippleAddress;
 
-    result = RPC::accountFromString (
-        ledger, raAccount, bIndex, strIdent, iIndex, false, context.netOps_);
+    result = RPC::accountFromString (ledger, rippleAddress, bIndex, strIdent,
+        iIndex, false, context.netOps_);
 
     if (! result.empty ())
         return result;
 
     // Get info on account.
-
-    result[jss::account] = raAccount.humanAccountID ();
+    result[jss::account] = rippleAddress.humanAccountID ();
 
     if (bIndex)
-        result[jss::account_index]   = iIndex;
+        result[jss::account_index] = iIndex;
 
-    if (! ledger->hasAccount (raAccount))
+    if (! ledger->hasAccount (rippleAddress))
         return rpcError (rpcACT_NOT_FOUND);
 
     unsigned int limit;
@@ -75,100 +74,82 @@ Json::Value doAccountOffers (RPC::Context& context)
     {
         limit = RPC::Tuning::defaultOffersPerRequest;
     }
+    
+    Account const& raAccount (rippleAddress.getAccountID ());
+    Json::Value& jsonOffers (result[jss::offers] = Json::arrayValue); 
+    std::vector <SLE::pointer> offers;
+    unsigned int reserve (limit);
+    uint256 startAfter;
+    std::uint64_t startHint;
 
-    uint256 const rootIndex (Ledger::getOwnerDirIndex (raAccount.getAccountID ()));
-    std::uint32_t resumeSeq = 0;
-    uint256 currentIndex;
-    bool resume (true);
-
-    if (params.isMember (jss::marker))
+    if (params.isMember(jss::marker))
     {
+        // We have a start point. Use limit - 1 from the result and use the
+        // very last one for the resume.        
         Json::Value const& marker (params[jss::marker]);
 
-        if (! marker.isObject () || marker.size () != 2 ||
-            ! marker.isMember (jss::seq) || ! marker[jss::seq].isIntegral () ||
-            ! marker.isMember (jss::account_index) ||
-            ! marker[jss::account_index].isString ())
-        {
+        if (! marker.isString ())
             return rpcError (rpcACT_MALFORMED);
+
+        startAfter.SetHex (marker.asString ());
+        SLE::pointer sleOffer (ledger->getSLEi (startAfter));
+
+        if (sleOffer == nullptr ||
+            sleOffer->getType () != ltOFFER ||
+            raAccount != sleOffer->getFieldAccount160 (sfAccount))
+        {
+            return rpcError (rpcINVALID_PARAMS);
         }
 
-        resumeSeq = marker[jss::seq].asUInt ();
-        currentIndex = Ledger::getDirNodeIndex (rootIndex,
-            uintFromHex (marker[jss::account_index].asString ()));
+        startHint = sleOffer->getFieldU64(sfOwnerNode);
 
-        resume = false;
+        // Caller provided the first offer (startAfter), add it as first result
+        Json::Value& obj (jsonOffers.append (Json::objectValue));
+        sleOffer->getFieldAmount (sfTakerPays).setJson (obj[jss::taker_pays]);
+        sleOffer->getFieldAmount (sfTakerGets).setJson (obj[jss::taker_gets]);
+        obj[jss::seq] = sleOffer->getFieldU32 (sfSequence);
+        obj[jss::flags] = sleOffer->getFieldU32 (sfFlags);            
+
+        offers.reserve (reserve);
     }
     else
     {
-        currentIndex = rootIndex;
+        startHint = 0;
+        // We have no start point, limit should be one higher than requested.            
+        offers.reserve (++reserve);
     }
 
-    Json::Value& jvsOffers(result[jss::offers] = Json::arrayValue);
-    unsigned int i (0);
-    bool process (true);
-
-    while (process)
-    {
-        SLE::pointer ownerDir (ledger->getSLEi (currentIndex));
-
-        if (!ownerDir || ownerDir->getType () != ltDIR_NODE)
-            break;
-
-        for (auto const& node : ownerDir->getFieldV256 (sfIndexes).peekValue ())
+    if (! ledger->visitAccountItems (raAccount, startAfter, startHint, reserve,
+        [&offers](SLE::ref offer)
         {
-            SLE::ref offer (ledger->getSLEi (node));
-
             if (offer->getType () == ltOFFER)
             {
-                std::uint32_t const seq (offer->getFieldU32 (sfSequence));
-
-                if (!resume && resumeSeq == seq)
-                    resume = true;
-
-                if (resume)
-                {
-                    if (i < limit)
-                    {
-                        Json::Value& obj (jvsOffers.append (Json::objectValue));
-                        offer->getFieldAmount (sfTakerPays).setJson (
-                            obj[jss::taker_pays]);
-                        offer->getFieldAmount (sfTakerGets).setJson (
-                            obj[jss::taker_gets]);
-                        obj[jss::seq] = seq;
-                        obj[jss::flags] = offer->getFieldU32 (sfFlags);
-
-                        ++i;
-                    }
-                    else
-                    {
-                        result[jss::limit] = limit;
-
-                        Json::Value& marker (result[jss::marker] = Json::objectValue);
-                        marker[jss::seq] = seq;
-                        marker[jss::account_index] = strHex(
-                            ownerDir->getFieldU64 (sfIndexPrevious));
-
-                        process = false;
-                        break;
-                    }
-                }
+                offers.emplace_back (offer);
+                return true;
             }
-        }
 
-        if (process)
-        {
-            std::uint64_t const uNodeNext(ownerDir->getFieldU64(sfIndexNext));
-
-            if (!uNodeNext)
-                break;
-
-            currentIndex = Ledger::getDirNodeIndex(rootIndex, uNodeNext);
-        }
+            return false;
+        }))
+    {
+        return rpcError (rpcINVALID_PARAMS);
     }
 
-    if (!resume)
-        return rpcError (rpcACT_MALFORMED);
+    if (offers.size () == reserve)
+    {
+        result[jss::limit] = limit;
+
+        result[jss::marker] = to_string (offers.back ()->getIndex ());        
+        offers.pop_back ();
+    }
+
+    for (auto const& offer : offers)
+    {
+        Json::Value& obj (jsonOffers.append (Json::objectValue));
+        offer->getFieldAmount (sfTakerPays).setJson (obj[jss::taker_pays]);
+        offer->getFieldAmount (sfTakerGets).setJson (obj[jss::taker_gets]);
+        obj[jss::seq] = offer->getFieldU32 (sfSequence);
+        obj[jss::flags] = offer->getFieldU32 (sfFlags);            
+    }
 
     context.loadType_ = Resource::feeMediumBurdenRPC;
     return result;