From 12e11721f98c398f197464035b74ff0917f0b6fb Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Tue, 11 Aug 2015 16:10:25 -0400 Subject: [PATCH] Eliminate redundant traversal logic of SHAMap: * Only the const_iterator interface remains. --- src/ripple/app/ledger/Ledger.cpp | 15 +- src/ripple/app/ledger/Ledger.h | 4 - src/ripple/shamap/SHAMap.h | 25 +-- src/ripple/shamap/impl/SHAMap.cpp | 235 ++++++------------------ src/ripple/shamap/tests/SHAMap.test.cpp | 33 ++-- 5 files changed, 88 insertions(+), 224 deletions(-) diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index 39edfef82..1545f1068 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -894,9 +894,8 @@ boost::optional Ledger::succ (uint256 const& key, boost::optional last) const { - auto const item = - stateMap_->peekNextItem(key); - if (! item) + auto item = stateMap_->upper_bound(key); + if (item == stateMap_->end()) return boost::none; if (last && item->key() >= last) return boost::none; @@ -1148,16 +1147,6 @@ void Ledger::visitStateItems (std::function callback) const } } -uint256 Ledger::getNextLedgerIndex (uint256 const& hash, - boost::optional const& last) const -{ - auto const node = - stateMap_->peekNextItem(hash); - if ((! node) || (last && node->key() >= *last)) - return {}; - return node->key(); -} - bool Ledger::walkLedger () const { std::vector missingNodes1; diff --git a/src/ripple/app/ledger/Ledger.h b/src/ripple/app/ledger/Ledger.h index 68f8919e4..b49cc4cca 100644 --- a/src/ripple/app/ledger/Ledger.h +++ b/src/ripple/app/ledger/Ledger.h @@ -317,10 +317,6 @@ public: bool pendSaveValidated (bool isSynchronous, bool isCurrent); - // first node >hash, const& last = boost::none) const; - std::vector getNeededTransactionHashes ( int max, SHAMapSyncFilter* filter) const; diff --git a/src/ripple/shamap/SHAMap.h b/src/ripple/shamap/SHAMap.h index 44cc07c8a..104d06015 100644 --- a/src/ripple/shamap/SHAMap.h +++ b/src/ripple/shamap/SHAMap.h @@ -165,16 +165,7 @@ public: peekItem (uint256 const& id, SHAMapTreeNode::TNType & type) const; // traverse functions - std::shared_ptr const& peekFirstItem () const; - std::shared_ptr const& - peekFirstItem (SHAMapTreeNode::TNType & type) const; - SHAMapItem const* peekFirstItem(NodeStack& stack) const; - std::shared_ptr const& peekLastItem () const; - std::shared_ptr const& peekNextItem (uint256 const& ) const; - std::shared_ptr const& - peekNextItem (uint256 const& , SHAMapTreeNode::TNType & type) const; - SHAMapItem const* peekNextItem(uint256 const& id, NodeStack& stack) const; - std::shared_ptr const& peekPrevItem (uint256 const& ) const; + const_iterator upper_bound(uint256 const& id) const; void visitNodes (std::function const&) const; void @@ -275,9 +266,7 @@ private: writeNode(NodeObjectType t, std::uint32_t seq, std::shared_ptr node) const; - SHAMapTreeNode* firstBelow (SHAMapAbstractNode*) const; SHAMapTreeNode* firstBelow (SHAMapAbstractNode*, NodeStack& stack) const; - SHAMapTreeNode* lastBelow (SHAMapAbstractNode*) const; // Simple descent // Get a child of the specified node @@ -305,6 +294,8 @@ private: bool hasInnerNode (SHAMapNodeID const& nodeID, uint256 const& hash) const; bool hasLeafNode (uint256 const& tag, uint256 const& hash) const; + SHAMapItem const* peekFirstItem(NodeStack& stack) const; + SHAMapItem const* peekNextItem(uint256 const& id, NodeStack& stack) const; bool walkBranch (SHAMapAbstractNode* node, std::shared_ptr const& otherMapItem, bool isFirstMap, Delta & differences, int & maxCount) const; @@ -389,6 +380,7 @@ public: private: explicit const_iterator(SHAMap const* map); const_iterator(SHAMap const* map, pointer item); + const_iterator(SHAMap const* map, pointer item, NodeStack&& stack); friend bool operator==(const_iterator const& x, const_iterator const& y); friend class SHAMap; @@ -408,6 +400,15 @@ SHAMap::const_iterator::const_iterator(SHAMap const* map, pointer item) { } +inline +SHAMap::const_iterator::const_iterator(SHAMap const* map, pointer item, + NodeStack&& stack) + : stack_(std::move(stack)) + , map_(map) + , item_(item) +{ +} + inline SHAMap::const_iterator::reference SHAMap::const_iterator::operator*() const diff --git a/src/ripple/shamap/impl/SHAMap.cpp b/src/ripple/shamap/impl/SHAMap.cpp index 666dd64ef..a6d7f874e 100644 --- a/src/ripple/shamap/impl/SHAMap.cpp +++ b/src/ripple/shamap/impl/SHAMap.cpp @@ -149,7 +149,6 @@ SHAMapTreeNode* SHAMap::walkToPointer (uint256 const& id) const { auto inNode = root_.get(); SHAMapNodeID nodeID; - uint256 nodeHash; while (inNode->isInner ()) { @@ -416,35 +415,6 @@ SHAMap::unshareNode (std::shared_ptr node, SHAMapNodeID const& nodeID) return node; } -SHAMapTreeNode* -SHAMap::firstBelow (SHAMapAbstractNode* node) const -{ - // Return the first item below this node - do - { - assert(node != nullptr); - - if (node->isLeaf ()) - return static_cast(node); - - // Walk down the tree - bool foundNode = false; - auto inner = static_cast(node); - for (int i = 0; i < 16; ++i) - { - if (!inner->isEmptyBranch (i)) - { - node = descendThrow (inner, i); - foundNode = true; - break; - } - } - if (!foundNode) - return nullptr; - } - while (true); -} - SHAMapTreeNode* SHAMap::firstBelow(SHAMapAbstractNode* node, NodeStack& stack) const { @@ -472,32 +442,6 @@ SHAMap::firstBelow(SHAMapAbstractNode* node, NodeStack& stack) const } } -SHAMapTreeNode* -SHAMap::lastBelow (SHAMapAbstractNode* node) const -{ - do - { - if (node->isLeaf ()) - return static_cast(node); - - // Walk down the tree - bool foundNode = false; - auto inner = static_cast(node); - for (int i = 15; i >= 0; --i) - { - if (!inner->isEmptyBranch (i)) - { - node = descendThrow (inner, i); - foundNode = true; - break; - } - } - if (!foundNode) - return nullptr; - } - while (true); -} - static const std::shared_ptr no_item; std::shared_ptr const& @@ -550,17 +494,6 @@ SHAMap::fetch (uint256 const& key) const return leaf->peekItem(); } -std::shared_ptr const& -SHAMap::peekFirstItem () const -{ - SHAMapTreeNode* node = firstBelow (root_.get ()); - - if (!node) - return no_item; - - return node->peekItem (); -} - SHAMapItem const* SHAMap::peekFirstItem(NodeStack& stack) const { @@ -572,81 +505,6 @@ SHAMap::peekFirstItem(NodeStack& stack) const return node->peekItem().get(); } -std::shared_ptr const& -SHAMap::peekFirstItem (SHAMapTreeNode::TNType& type) const -{ - SHAMapTreeNode* node = firstBelow (root_.get ()); - - if (!node) - return no_item; - - type = node->getType (); - return node->peekItem (); -} - -std::shared_ptr const& -SHAMap::peekLastItem () const -{ - SHAMapTreeNode* node = lastBelow (root_.get ()); - - if (!node) - return no_item; - - return node->peekItem (); -} - -std::shared_ptr const& -SHAMap::peekNextItem (uint256 const& id) const -{ - SHAMapTreeNode::TNType type; - return peekNextItem (id, type); -} - -std::shared_ptr const& -SHAMap::peekNextItem (uint256 const& id, SHAMapTreeNode::TNType& type) const -{ - // Get a pointer to the next item in the tree after a given item - item need not be in tree - - auto stack = getStack (id, true); - - while (!stack.empty ()) - { - auto node = stack.top().first.get(); - auto nodeID = stack.top().second; - stack.pop (); - - if (node->isLeaf ()) - { - auto leaf = static_cast(node); - if (leaf->peekItem ()->key() > id) - { - type = leaf->getType (); - return leaf->peekItem (); - } - } - else - { - // breadth-first - auto inner = static_cast(node); - for (int i = nodeID.selectBranch (id) + 1; i < 16; ++i) - if (!inner->isEmptyBranch (i)) - { - node = descendThrow (inner, i); - auto leaf = firstBelow (node); - - if (!leaf) - throw SHAMapMissingNode(type_, id); - - type = leaf->getType (); - return leaf->peekItem (); - } - } - } - - // must be last item - return no_item; -} - SHAMapItem const* SHAMap::peekNextItem(uint256 const& id, NodeStack& stack) const { @@ -679,43 +537,6 @@ SHAMap::peekNextItem(uint256 const& id, NodeStack& stack) const 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 const& -SHAMap::peekPrevItem (uint256 const& id) const -{ - auto stack = getStack (id, true); - - while (!stack.empty ()) - { - auto node = stack.top ().first.get(); - auto nodeID = stack.top ().second; - stack.pop (); - - if (node->isLeaf ()) - { - auto leaf = static_cast(node); - if (leaf->peekItem ()->key() < id) - return leaf->peekItem (); - } - else - { - auto inner = static_cast(node); - for (int i = nodeID.selectBranch (id) - 1; i >= 0; --i) - { - if (!inner->isEmptyBranch (i)) - { - node = descendThrow (inner, i); - auto leaf = lastBelow (node); - return leaf->peekItem (); - } - } - } - } - - // must be first item - return no_item; -} - std::shared_ptr const& SHAMap::peekItem (uint256 const& id) const { @@ -751,6 +572,62 @@ SHAMap::peekItem (uint256 const& id, uint256& hash) const return leaf->peekItem (); } +SHAMap::const_iterator +SHAMap::upper_bound(uint256 const& id) const +{ + // Get a const_iterator to the next item in the tree after a given item + // item need not be in tree + NodeStack stack; + auto node = root_.get(); + auto nodeID = SHAMapNodeID{}; + auto foundLeaf = true; + while (!node->isLeaf()) + { + stack.push ({node, nodeID}); + auto branch = nodeID.selectBranch(id); + auto inner = static_cast(node); + if (inner->isEmptyBranch(branch)) + { + foundLeaf = false; + break; + } + node = descendThrow(inner, branch); + nodeID = nodeID.getChildNodeID(branch); + } + if (foundLeaf) + stack.push({node, nodeID}); + while (!stack.empty()) + { + std::tie(node, nodeID) = stack.top(); + if (node->isLeaf()) + { + auto leaf = static_cast(node); + if (leaf->peekItem()->key() > id) + return const_iterator(this, leaf->peekItem().get(), std::move(stack)); + stack.pop(); + } + else + { + stack.pop(); + auto inner = static_cast(node); + for (auto branch = nodeID.selectBranch(id) + 1; branch < 16; ++branch) + { + if (!inner->isEmptyBranch(branch)) + { + node = descendThrow(inner, branch); + nodeID = nodeID.getChildNodeID(branch); + stack.push({node, nodeID}); + auto leaf = firstBelow(node, stack); + if (!leaf) + throw SHAMapMissingNode(type_, id); + return const_iterator(this, leaf->peekItem().get(), + std::move(stack)); + } + } + } + } + return end(); +} bool SHAMap::hasItem (uint256 const& id) const { diff --git a/src/ripple/shamap/tests/SHAMap.test.cpp b/src/ripple/shamap/tests/SHAMap.test.cpp index 56f739cc8..a35eeaefd 100644 --- a/src/ripple/shamap/tests/SHAMap.test.cpp +++ b/src/ripple/shamap/tests/SHAMap.test.cpp @@ -68,31 +68,32 @@ public: unexpected (!sMap.addItem (i2, true, false), "no add"); unexpected (!sMap.addItem (i1, true, false), "no add"); - std::shared_ptr i; - i = sMap.peekFirstItem (); - unexpected (!i || (*i != i1), "bad traverse"); - i = sMap.peekNextItem (i->key()); - unexpected (!i || (*i != i2), "bad traverse"); - i = sMap.peekNextItem (i->key()); - unexpected (i, "bad traverse"); + auto i = sMap.begin(); + auto e = sMap.end(); + unexpected (i == e || (*i != i1), "bad traverse"); + ++i; + unexpected (i == e || (*i != i2), "bad traverse"); + ++i; + unexpected (i != e, "bad traverse"); sMap.addItem (i4, true, false); sMap.delItem (i2.key()); sMap.addItem (i3, true, false); - i = sMap.peekFirstItem (); - unexpected (!i || (*i != i1), "bad traverse"); - i = sMap.peekNextItem (i->key()); - unexpected (!i || (*i != i3), "bad traverse"); - i = sMap.peekNextItem (i->key()); - unexpected (!i || (*i != i4), "bad traverse"); - i = sMap.peekNextItem (i->key()); - unexpected (i, "bad traverse"); + i = sMap.begin(); + e = sMap.end(); + unexpected (i == e || (*i != i1), "bad traverse"); + ++i; + unexpected (i == e || (*i != i3), "bad traverse"); + ++i; + unexpected (i == e || (*i != i4), "bad traverse"); + ++i; + unexpected (i != e, "bad traverse"); testcase ("snapshot"); uint256 mapHash = sMap.getHash (); std::shared_ptr map2 = sMap.snapShot (false); unexpected (sMap.getHash () != mapHash, "bad snapshot"); unexpected (map2->getHash () != mapHash, "bad snapshot"); - unexpected (!sMap.delItem (sMap.peekFirstItem ()->key()), "bad mod"); + unexpected (!sMap.delItem (sMap.begin()->key()), "bad mod"); unexpected (sMap.getHash () == mapHash, "bad snapshot"); unexpected (map2->getHash () != mapHash, "bad snapshot");