Optimize and clean up SHAMap::iterator:

* Remove dependence on boost::iterator_facade.
* Rename iterator to const_iterator.
* Change value_type from shared_ptr<SHAMapItem const> to SHAMapItem.
* Install a stack-path to the current SHAMapItem in the const_iterator.

src/ripple/app/ledger/AcceptedLedger.cpp

@@ -35,7 +35,7 @@ AcceptedLedger::AcceptedLedger (Ledger::ref ledger) : mLedger (ledger)
 {
     for (auto const& item : ledger->txMap())
     {
-        SerialIter sit (item->slice());
+        SerialIter sit (item.slice());
         insert (std::make_shared<AcceptedLedgerTx>(
             ledger, std::ref (sit)));
     }

src/ripple/app/ledger/Ledger.cpp

@@ -59,7 +59,7 @@ class Ledger::txs_iter_impl
 private:
     bool metadata_;
     ReadView const* view_;
-    SHAMap::iterator iter_;
+    SHAMap::const_iterator iter_;
 
 public:
     txs_iter_impl() = delete;
@@ -68,7 +68,7 @@ public:
     txs_iter_impl (txs_iter_impl const&) = default;
 
     txs_iter_impl (bool metadata,
-        SHAMap::iterator iter,
+        SHAMap::const_iterator iter,
             ReadView const& view)
         : metadata_ (metadata)
         , view_ (&view)
@@ -102,8 +102,8 @@ public:
     {
         auto const item = *iter_;
         if (metadata_)
-            return deserializeTxPlusMeta(*item);
+            return deserializeTxPlusMeta(item);
-        return { deserializeTx(*item), nullptr };
+        return { deserializeTx(item), nullptr };
     }
 };
 

src/ripple/app/ledger/LedgerHistory.cpp

@@ -275,16 +275,14 @@ log_metadata_difference(Ledger::pointer builtLedger, Ledger::pointer validLedger
 
 // Return list of leaves sorted by key
 static
-std::vector<std::shared_ptr<SHAMapItem const>>
+std::vector<SHAMapItem const*>
 leaves (SHAMap const& sm)
 {
-    std::vector<std::shared_ptr<
+    std::vector<SHAMapItem const*> v;
-        SHAMapItem const>> v;
     for (auto const& item : sm)
-        v.push_back(item);
+        v.push_back(&item);
     std::sort(v.begin(), v.end(),
-        [](std::shared_ptr<SHAMapItem const> const& lhs,
+        [](SHAMapItem const* lhs, SHAMapItem const* rhs)
-           std::shared_ptr<SHAMapItem const> const& rhs)
                 { return lhs->key() < rhs->key(); });
     return v;
 }

src/ripple/app/ledger/impl/LedgerConsensusImp.cpp

@@ -1812,20 +1812,19 @@ void applyTransactions (
 {
     if (set)
     {
-        for (auto const item : *set)
+        for (auto const& item : *set)
         {
-            if (checkLedger->txExists (item->key()))
+            if (checkLedger->txExists (item.key()))
                 continue;
 
             // The transaction isn't in the check ledger, try to apply it
             WriteLog (lsDEBUG, LedgerConsensus) <<
-                "Processing candidate transaction: " << item->key();
+                "Processing candidate transaction: " << item.key();
             
             std::shared_ptr<STTx const> txn;
             try
             {
-                SerialIter sit (item->slice());
+                txn = std::make_shared<STTx const>(SerialIter{item.slice()});
-                txn = std::make_shared<STTx const>(sit);
             }
             catch (...)
             {

src/ripple/app/ledger/impl/OpenLedger.cpp

@@ -207,7 +207,7 @@ debugTostr (SHAMap const& set)
     {
         try
         {
-            SerialIter sit(item->slice());
+            SerialIter sit(item.slice());
             auto const tx = std::make_shared<
                 STTx const>(sit);
             ss << debugTxstr(tx) << ", ";

src/ripple/app/main/Application.cpp

@@ -1334,16 +1334,16 @@ bool ApplicationImp::loadOldLedger (
             for (auto const& item : txns)
             {
                 auto const txn =
-                    getTransaction(*replayLedger, item->key(),
+                    getTransaction(*replayLedger, item.key(),
                         getApp().getMasterTransaction());
                 if (m_journal.info) m_journal.info <<
                     txn->getJson(0);
                 Serializer s;
                 txn->getSTransaction()->add(s);
-                cur->rawTxInsert(item->key(),
+                cur->rawTxInsert(item.key(),
                     std::make_shared<Serializer const>(
                         std::move(s)), nullptr);
-                getApp().getHashRouter().setFlag (item->key(), SF_SIGGOOD);
+                getApp().getHashRouter().setFlag (item.key(), SF_SIGGOOD);
             }
 
             // Switch to the mutable snapshot

src/ripple/shamap/SHAMap.h

@@ -33,12 +33,12 @@
 #include <ripple/nodestore/Database.h>
 #include <ripple/nodestore/NodeObject.h>
 #include <beast/utility/Journal.h>
-#include <boost/iterator/iterator_facade.hpp>
 #include <boost/thread/mutex.hpp>
 #include <boost/thread/shared_lock_guard.hpp>
 #include <boost/thread/shared_mutex.hpp>
 #include <cassert>
 #include <stack>
+#include <vector>
 
 namespace ripple {
 
@@ -80,6 +80,8 @@ class SHAMap
 {
 private:
     using Family = shamap::Family;
+    using NodeStack = std::stack<std::pair<SHAMapAbstractNode*, SHAMapNodeID>,
+                    std::vector<std::pair<SHAMapAbstractNode*, SHAMapNodeID>>>;
 
     Family&                         f_;
     beast::Journal                  journal_;
@@ -119,10 +121,10 @@ public:
         This is always a const iterator.
         Meets the requirements of ForwardRange.
     */
-    class iterator;
+    class const_iterator;
 
-    iterator begin() const;
+    const_iterator begin() const;
-    iterator end() const;
+    const_iterator end() const;
 
     //--------------------------------------------------------------------------
 
@@ -166,10 +168,12 @@ public:
     std::shared_ptr<SHAMapItem const> const& peekFirstItem () const;
     std::shared_ptr<SHAMapItem const> const&
         peekFirstItem (SHAMapTreeNode::TNType & type) const;
+    SHAMapItem const* peekFirstItem(NodeStack& stack) const;
     std::shared_ptr<SHAMapItem const> const& peekLastItem () const;
     std::shared_ptr<SHAMapItem const> const& peekNextItem (uint256 const& ) const;
     std::shared_ptr<SHAMapItem const> const&
         peekNextItem (uint256 const& , SHAMapTreeNode::TNType & type) const;
+    SHAMapItem const* peekNextItem(uint256 const& id, NodeStack& stack) const;
     std::shared_ptr<SHAMapItem const> const& peekPrevItem (uint256 const& ) const;
 
     void visitNodes (std::function<bool (SHAMapAbstractNode&)> const&) const;
@@ -272,6 +276,7 @@ private:
                   std::shared_ptr<SHAMapAbstractNode> node) const;
 
     SHAMapTreeNode* firstBelow (SHAMapAbstractNode*) const;
+    SHAMapTreeNode* firstBelow (SHAMapAbstractNode*, NodeStack& stack) const;
     SHAMapTreeNode* lastBelow (SHAMapAbstractNode*) const;
 
     // Simple descent
@@ -358,66 +363,109 @@ SHAMap::setUnbacked ()
 
 //------------------------------------------------------------------------------
 
-class SHAMap::iterator
+class SHAMap::const_iterator
-    : public boost::iterator_facade<
-        SHAMap::iterator,
-            std::shared_ptr<SHAMapItem const> const,
-                std::forward_iterator_tag>
 {
-private:
+public:
-    friend class boost::iterator_core_access;
+    using iterator_category = std::forward_iterator_tag;
+    using difference_type   = std::ptrdiff_t;
+    using value_type        = SHAMapItem;
+    using reference         = value_type const&;
+    using pointer           = value_type const*;
 
-    SHAMap const* map_ = nullptr;
+private:
-    std::shared_ptr<
+    NodeStack     stack_;
-        SHAMapItem const> item_;
+    SHAMap const* map_  = nullptr;
+    pointer       item_ = nullptr;
 
 public:
-    iterator() = default;
+    const_iterator() = default;
-    iterator (iterator const&) = default;
-    iterator& operator= (iterator const&) = default;
 
-    iterator (SHAMap const& map,
+    reference operator*()  const;
-        std::shared_ptr<SHAMapItem const> const& item)
+    pointer   operator->() const;
-        : map_(&map)
+
-        , item_(item)
+    const_iterator& operator++();
-    {
+    const_iterator  operator++(int);
-    }
 
 private:
-    void
+    explicit const_iterator(SHAMap const* map);
-    increment()
+    const_iterator(SHAMap const* map, pointer item);
-    {
-        item_ = map_->peekNextItem(
-            item_->key());
-    }
 
-    bool
+    friend bool operator==(const_iterator const& x, const_iterator const& y);
-    equal (iterator const& other) const
+    friend class SHAMap;
-    {
-        assert(map_ == other.map_);
-        return item_ == other.item_;
-    }
-
-    std::shared_ptr<
-        SHAMapItem const> const&
-    dereference() const
-    {
-        return item_;
-    }
 };
 
 inline
-SHAMap::iterator
+SHAMap::const_iterator::const_iterator(SHAMap const* map)
-SHAMap::begin() const
+    : map_(map)
+    , item_(map_->peekFirstItem(stack_))
 {
-    return iterator(*this, peekFirstItem());
 }
 
 inline
-SHAMap::iterator
+SHAMap::const_iterator::const_iterator(SHAMap const* map, pointer item)
+    : map_(map)
+    , item_(item)
+{
+}
+
+inline
+SHAMap::const_iterator::reference
+SHAMap::const_iterator::operator*() const
+{
+    return *item_;
+}
+
+inline
+SHAMap::const_iterator::pointer
+SHAMap::const_iterator::operator->() const
+{
+    return item_;
+}
+
+inline
+SHAMap::const_iterator&
+SHAMap::const_iterator::operator++()
+{
+    item_ = map_->peekNextItem(item_->key(), stack_);
+    return *this;
+}
+
+inline
+SHAMap::const_iterator
+SHAMap::const_iterator::operator++(int)
+{
+    auto tmp = *this;
+    ++(*this);
+    return tmp;
+}
+
+inline
+bool
+operator==(SHAMap::const_iterator const& x, SHAMap::const_iterator const& y)
+{
+    assert(x.map_ == y.map_);
+    return x.item_ == y.item_;
+}
+
+inline
+bool
+operator!=(SHAMap::const_iterator const& x, SHAMap::const_iterator const& y)
+{
+    return !(x == y);
+}
+
+inline
+SHAMap::const_iterator
+SHAMap::begin() const
+{
+    return const_iterator(this);
+}
+
+inline
+SHAMap::const_iterator
 SHAMap::end() const
 {
-    return iterator(*this, nullptr);
+    return const_iterator(this, nullptr);
 }
 
 }

src/ripple/shamap/impl/SHAMap.cpp

@@ -445,6 +445,33 @@ SHAMap::firstBelow (SHAMapAbstractNode* node) const
     while (true);
 }
 
+SHAMapTreeNode*
+SHAMap::firstBelow(SHAMapAbstractNode* node, NodeStack& stack) const
+{
+    // Return the first item below this node
+    while (true)
+    {
+        assert(node != nullptr);
+        if (node->isLeaf())
+            return static_cast<SHAMapTreeNode*>(node);
+        bool foundNode = false;
+        auto inner = static_cast<SHAMapInnerNode*>(node);
+        for (int i = 0; i < 16; ++i)
+        {
+            if (!inner->isEmptyBranch(i))
+            {
+                node = descendThrow(inner, i);
+                assert(!stack.empty());
+                stack.push({node, stack.top().second.getChildNodeID(i)});
+                foundNode = true;
+                break;
+            }
+        }
+        if (!foundNode)
+            return nullptr;
+    }
+}
+
 SHAMapTreeNode*
 SHAMap::lastBelow (SHAMapAbstractNode* node) const
 {
@@ -534,6 +561,17 @@ SHAMap::peekFirstItem () const
     return node->peekItem ();
 }
 
+SHAMapItem const*
+SHAMap::peekFirstItem(NodeStack& stack) const
+{
+    assert(stack.empty());
+    stack.push({root_.get(), SHAMapNodeID{}});
+    SHAMapTreeNode* node = firstBelow(root_.get(), stack);
+    if (!node)
+        return nullptr;
+    return node->peekItem().get();
+}
+
 std::shared_ptr<SHAMapItem const> const&
 SHAMap::peekFirstItem (SHAMapTreeNode::TNType& type) const
 {
@@ -597,7 +635,7 @@ SHAMap::peekNextItem (uint256 const& id, SHAMapTreeNode::TNType& type) const
                     auto leaf = firstBelow (node);
 
                     if (!leaf)
-                        throw (std::runtime_error ("missing/corrupt node"));
+                        throw SHAMapMissingNode(type_, id);
 
                     type = leaf->getType ();
                     return leaf->peekItem ();
@@ -609,6 +647,38 @@ SHAMap::peekNextItem (uint256 const& id, SHAMapTreeNode::TNType& type) const
     return no_item;
 }
 
+SHAMapItem const*
+SHAMap::peekNextItem(uint256 const& id, NodeStack& stack) const
+{
+    assert(!stack.empty());
+    assert(stack.top().first->isLeaf());
+    stack.pop();
+    while (!stack.empty())
+    {
+        auto node = stack.top().first;
+        auto nodeID = stack.top().second;
+        assert(!node->isLeaf());
+        auto inner = static_cast<SHAMapInnerNode*>(node);
+        for (auto i = nodeID.selectBranch(id) + 1; i < 16; ++i)
+        {
+            if (!inner->isEmptyBranch(i))
+            {
+                node = descendThrow(inner, i);
+                nodeID = nodeID.getChildNodeID(i);
+                stack.push({node, nodeID});
+                auto leaf = firstBelow(node, stack);
+                if (!leaf)
+                    throw SHAMapMissingNode(type_, id);
+                assert(leaf->isLeaf());
+                return leaf->peekItem().get();
+            }
+        }
+        stack.pop();
+    }
+    // must be last item
+    return nullptr;
+}
+
 // Get a pointer to the previous item in the tree after a given item - item need not be in tree
 std::shared_ptr<SHAMapItem const> const&
 SHAMap::peekPrevItem (uint256 const& id) const
@@ -697,7 +767,7 @@ bool SHAMap::delItem (uint256 const& id)
     auto stack = getStack (id, true);
 
     if (stack.empty ())
-        throw (std::runtime_error ("missing node"));
+        throw SHAMapMissingNode(type_, id);
 
     auto leaf = std::dynamic_pointer_cast<SHAMapTreeNode>(stack.top ().first);
     stack.pop ();
@@ -785,7 +855,7 @@ SHAMap::addGiveItem (std::shared_ptr<SHAMapItem const> const& item,
     auto stack = getStack (tag, true);
 
     if (stack.empty ())
-        throw (std::runtime_error ("missing node"));
+        throw SHAMapMissingNode(type_, tag);
 
     auto node = stack.top ().first;
     auto nodeID = stack.top ().second;
@@ -875,7 +945,7 @@ SHAMap::updateGiveItem (std::shared_ptr<SHAMapItem const> const& item,
     auto stack = getStack (tag, true);
 
     if (stack.empty ())
-        throw (std::runtime_error ("missing node"));
+        throw SHAMapMissingNode(type_, tag);
 
     auto node = std::dynamic_pointer_cast<SHAMapTreeNode>(stack.top().first);
     auto nodeID = stack.top ().second;