From bacf2605a46dbc3ec04d444215bc3d76c65b9946 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Thu, 10 Dec 2015 11:51:39 -0500 Subject: [PATCH] Simplify SHAMap::firstBelow: * Add SHAMap iteration test --- src/ripple/shamap/SHAMap.h | 5 +- src/ripple/shamap/impl/SHAMap.cpp | 67 ++++++++++++++----------- src/ripple/shamap/tests/SHAMap.test.cpp | 25 +++++++++ 3 files changed, 66 insertions(+), 31 deletions(-) diff --git a/src/ripple/shamap/SHAMap.h b/src/ripple/shamap/SHAMap.h index a988d09e5..f7b219f52 100644 --- a/src/ripple/shamap/SHAMap.h +++ b/src/ripple/shamap/SHAMap.h @@ -245,7 +245,10 @@ private: /** Walk towards the specified id, returning the node. Caller must check if the return is nullptr, and if not, if the node->peekItem()->key() == id */ SHAMapTreeNode* - walkToKey(uint256 const& id, NodeStack* stack = nullptr) const; + walkTowardsKey(uint256 const& id, NodeStack* stack = nullptr) const; + /** Return nullptr if key not found */ + SHAMapTreeNode* + findKey(uint256 const& id) const; /** Unshare the node, allowing it to be modified */ template diff --git a/src/ripple/shamap/impl/SHAMap.cpp b/src/ripple/shamap/impl/SHAMap.cpp index 13dd970a5..1c48d0262 100644 --- a/src/ripple/shamap/impl/SHAMap.cpp +++ b/src/ripple/shamap/impl/SHAMap.cpp @@ -145,7 +145,7 @@ SHAMap::dirtyUp (SharedPtrNodeStack& stack, } SHAMapTreeNode* -SHAMap::walkToKey(uint256 const& id, NodeStack* stack) const +SHAMap::walkTowardsKey(uint256 const& id, NodeStack* stack) const { assert(stack == nullptr || stack->empty()); auto inNode = root_.get(); @@ -155,8 +155,8 @@ SHAMap::walkToKey(uint256 const& id, NodeStack* stack) const while (inNode->isInner()) { - int branch = nodeID.selectBranch (id); - auto inner = static_cast(inNode); + auto const branch = nodeID.selectBranch (id); + auto const inner = static_cast(inNode); if (inner->isEmptyBranch (branch)) return nullptr; @@ -168,6 +168,15 @@ SHAMap::walkToKey(uint256 const& id, NodeStack* stack) const return static_cast(inNode); } +SHAMapTreeNode* +SHAMap::findKey(uint256 const& id) const +{ + SHAMapTreeNode* leaf = walkTowardsKey(id); + if (leaf && leaf->peekItem()->key() != id) + leaf = nullptr; + return leaf; +} + std::shared_ptr SHAMap::fetchNodeFromDB (SHAMapHash const& hash) const { @@ -423,28 +432,26 @@ SHAMap::unshareNode (std::shared_ptr node, SHAMapNodeID const& nodeID) SHAMapTreeNode* SHAMap::firstBelow(SHAMapAbstractNode* node, NodeStack& stack) const { - // Return the first item below this node - while (true) + // Return the first item at or below this node + if (node->isLeaf()) + return static_cast(node); + auto inner = static_cast(node); + for (int i = 0; i < 16;) { - assert(node != nullptr); - if (node->isLeaf()) - return static_cast(node); - bool foundNode = false; - auto inner = static_cast(node); - for (int i = 0; i < 16; ++i) + if (!inner->isEmptyBranch(i)) { - if (!inner->isEmptyBranch(i)) - { - node = descendThrow(inner, i); - assert(!stack.empty()); - stack.push({node, stack.top().second.getChildNodeID(i)}); - foundNode = true; - break; - } + node = descendThrow(inner, i); + assert(!stack.empty()); + stack.push({node, stack.top().second.getChildNodeID(i)}); + if (node->isLeaf()) + return static_cast(node); + inner = static_cast(node); + i = 0; // scan all 16 branches of this new node } - if (!foundNode) - return nullptr; + else + ++i; // scan next branch } + return nullptr; } static const std::shared_ptr no_item; @@ -539,9 +546,9 @@ SHAMap::peekNextItem(uint256 const& id, NodeStack& stack) const std::shared_ptr const& SHAMap::peekItem (uint256 const& id) const { - SHAMapTreeNode* leaf = walkToKey(id); + SHAMapTreeNode* leaf = findKey(id); - if (!leaf || leaf->peekItem()->key() != id) + if (!leaf) return no_item; return leaf->peekItem (); @@ -550,9 +557,9 @@ SHAMap::peekItem (uint256 const& id) const std::shared_ptr const& SHAMap::peekItem (uint256 const& id, SHAMapTreeNode::TNType& type) const { - SHAMapTreeNode* leaf = walkToKey(id); + SHAMapTreeNode* leaf = findKey(id); - if (!leaf || leaf->peekItem()->key() != id) + if (!leaf) return no_item; type = leaf->getType (); @@ -562,9 +569,9 @@ SHAMap::peekItem (uint256 const& id, SHAMapTreeNode::TNType& type) const std::shared_ptr const& SHAMap::peekItem (uint256 const& id, SHAMapHash& hash) const { - SHAMapTreeNode* leaf = walkToKey(id); + SHAMapTreeNode* leaf = findKey(id); - if (!leaf || leaf->peekItem()->key() != id) + if (!leaf) return no_item; hash = leaf->getNodeHash (); @@ -577,7 +584,7 @@ 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; - walkToKey(id, &stack); + walkTowardsKey(id, &stack); SHAMapAbstractNode* node; SHAMapNodeID nodeID; while (!stack.empty()) @@ -615,8 +622,8 @@ SHAMap::upper_bound(uint256 const& id) const bool SHAMap::hasItem (uint256 const& id) const { // does the tree have an item with this ID - SHAMapTreeNode* leaf = walkToKey(id); - return (leaf != nullptr && leaf->peekItem()->key() == id); + SHAMapTreeNode* leaf = findKey(id); + return (leaf != nullptr); } bool SHAMap::delItem (uint256 const& id) diff --git a/src/ripple/shamap/tests/SHAMap.test.cpp b/src/ripple/shamap/tests/SHAMap.test.cpp index 21fe6c83a..c653fae7b 100644 --- a/src/ripple/shamap/tests/SHAMap.test.cpp +++ b/src/ripple/shamap/tests/SHAMap.test.cpp @@ -133,6 +133,31 @@ public: } expect (map.getHash() == zero, "bad final empty map hash"); } + + testcase ("iterate"); + { + std::vector keys(8); + keys[0].SetHex ("f22891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6ca8"); + keys[1].SetHex ("b99891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6ca8"); + keys[2].SetHex ("b92891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6ca8"); + keys[3].SetHex ("b92881fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6ca8"); + keys[4].SetHex ("b92791fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6ca8"); + keys[5].SetHex ("b92691fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6ca8"); + keys[6].SetHex ("b91891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6ca8"); + keys[7].SetHex ("292891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6ca8"); + + tests::TestFamily f{beast::Journal{}}; + SHAMap map{SHAMapType::FREE, f}; + for (auto const& k : keys) + map.addItem(SHAMapItem{k, IntToVUC(0)}, true, false); + + int i = 7; + for (auto const& k : map) + { + expect(k.key() == keys[i]); + --i; + } + } } };