From dde89da5563d0c339546cdb58293373ba706168b Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Sun, 10 Jul 2016 08:38:02 +0200 Subject: [PATCH] Improve traverseLeaves --- .../onblocks/datatreestore/DataTree.cpp | 93 +++++++++++-------- .../onblocks/datatreestore/DataTree.h | 6 +- 2 files changed, 59 insertions(+), 40 deletions(-) diff --git a/src/blobstore/implementations/onblocks/datatreestore/DataTree.cpp b/src/blobstore/implementations/onblocks/datatreestore/DataTree.cpp index c32a3536..7279d2e9 100644 --- a/src/blobstore/implementations/onblocks/datatreestore/DataTree.cpp +++ b/src/blobstore/implementations/onblocks/datatreestore/DataTree.cpp @@ -32,6 +32,7 @@ using cpputils::optional_ownership_ptr; using cpputils::WithOwnership; using cpputils::WithoutOwnership; using cpputils::unique_ref; +using cpputils::Data; namespace blobstore { namespace onblocks { @@ -169,7 +170,7 @@ uint32_t DataTree::_computeNumLeaves(const DataNode &node) const { return numLeavesInLeftChildren + numLeavesInRightChild; } -void DataTree::traverseLeaves(uint32_t beginIndex, uint32_t endIndex, function func) { +void DataTree::traverseLeaves(uint32_t beginIndex, uint32_t endIndex, std::function onExistingLeaf, std::function onCreateLeaf) { //TODO Can we traverse in parallel? boost::upgrade_lock lock(_mutex); //TODO Rethink locking here. We probably need locking when the traverse resizes the blob. Otherwise, parallel traverse should be possible. We already allow it below by freeing the upgrade_lock, but we currently only allow it if ALL traverses are entirely inside the valid region. Can we allow more parallelity? auto exclusiveLock = std::make_unique>(lock); @@ -189,40 +190,49 @@ void DataTree::traverseLeaves(uint32_t beginIndex, uint32_t endIndex, function= beginIndex // There is a gap between the current size and the begin of the traversal - _traverseLeaves(_rootNode.get(), 0, numLeaves-1, endIndex, [beginIndex, numLeaves, &func, this](DataLeafNode* node, uint32_t index) { - if (index >= beginIndex) { - func(node, index); - } else if (index == numLeaves - 1) { - // It is the old last leaf - resize it to maximum - node->resize(_nodeStore->layout().maxBytesPerLeaf()); - } - }); + auto _onExistingLeaf = [numLeaves, &onExistingLeaf, this](uint32_t index, DataLeafNode* node) { + if (index == numLeaves - 1) { + // It is the old last leaf - resize it to maximum + node->resize(_nodeStore->layout().maxBytesPerLeaf()); + } + onExistingLeaf(index, node); + }; + auto _onCreateLeaf = [beginIndex, &onCreateLeaf, this](uint32_t index) { + if (index < beginIndex) { + // Create empty leaves in the gap + return Data(_nodeStore->layout().maxBytesPerLeaf()).FillWithZeroes(); + } else { + return onCreateLeaf(index); + } + }; + _traverseLeaves(_rootNode.get(), 0, numLeaves-1, endIndex, _onExistingLeaf, _onCreateLeaf); ASSERT(endIndex >= _numLeavesCache.value(), "We should be outside of the valid region, i.e. outside of the old size"); _numLeavesCache = endIndex; } else if (numLeaves < endIndex) { // We are starting traversal in the valid region, but traverse until after it (we grow new leaves) - _traverseLeaves(_rootNode.get(), 0, beginIndex, endIndex, [numLeaves, &func, this] (DataLeafNode *node, uint32_t index) { - if (index == numLeaves - 1) { - // It is the old last leaf - resize it to maximum - node->resize(_nodeStore->layout().maxBytesPerLeaf()); - } - func(node, index); - }); + auto _onExistingLeaf = [numLeaves, &onExistingLeaf, this] (uint32_t index, DataLeafNode *node) { + if (index == numLeaves - 1) { + // It is the old last leaf - resize it to maximum + node->resize(_nodeStore->layout().maxBytesPerLeaf()); + } + onExistingLeaf(index, node); + }; + _traverseLeaves(_rootNode.get(), 0, beginIndex, endIndex, _onExistingLeaf, onCreateLeaf); ASSERT(endIndex >= _numLeavesCache.value(), "We should be outside of the valid region, i.e. outside of the old size"); _numLeavesCache = endIndex; } else { //We are traversing entirely inside the valid region exclusiveLock.reset(); // we can allow parallel traverses, if all are entirely inside the valid region. - _traverseLeaves(_rootNode.get(), 0, beginIndex, endIndex, func); + _traverseLeaves(_rootNode.get(), 0, beginIndex, endIndex, onExistingLeaf, onCreateLeaf); } } -void DataTree::_traverseLeaves(DataNode *root, uint32_t leafOffset, uint32_t beginIndex, uint32_t endIndex, function func) { +void DataTree::_traverseLeaves(DataNode *root, uint32_t leafOffset, uint32_t beginIndex, uint32_t endIndex, std::function onExistingLeaf, std::function onCreateLeaf) { DataLeafNode *leaf = dynamic_cast(root); if (leaf != nullptr) { ASSERT(beginIndex <= 1 && endIndex <= 1, "If root node is a leaf, the (sub)tree has only one leaf - access indices must be 0 or 1."); if (beginIndex == 0 && endIndex == 1) { - func(leaf, leafOffset); + onExistingLeaf(leafOffset, leaf); } return; } @@ -231,32 +241,41 @@ void DataTree::_traverseLeaves(DataNode *root, uint32_t leafOffset, uint32_t beg uint32_t leavesPerChild = leavesPerFullChild(*inner); uint32_t beginChild = beginIndex/leavesPerChild; uint32_t endChild = utils::ceilDivision(endIndex, leavesPerChild); - vector> children = getOrCreateChildren(inner, beginChild, endChild); - for (uint32_t childIndex = beginChild; childIndex < endChild; ++childIndex) { + for (uint32_t childIndex = beginChild; childIndex < std::min(inner->numChildren(), endChild); ++childIndex) { + auto child = _nodeStore->load(inner->getChild(childIndex)->key()); + ASSERT(child != none, "Couldn't load child node"); uint32_t childOffset = childIndex * leavesPerChild; uint32_t localBeginIndex = utils::maxZeroSubtraction(beginIndex, childOffset); uint32_t localEndIndex = std::min(leavesPerChild, endIndex - childOffset); - auto child = std::move(children[childIndex-beginChild]); - _traverseLeaves(child.get(), leafOffset + childOffset, localBeginIndex, localEndIndex, func); + _traverseLeaves(child->get(), leafOffset + childOffset, localBeginIndex, localEndIndex, onExistingLeaf, onCreateLeaf); + } + for (uint32_t childIndex = inner->numChildren(); childIndex < endChild; ++childIndex) { + uint32_t childOffset = childIndex * leavesPerChild; + uint32_t localEndIndex = std::min(leavesPerChild, endIndex - childOffset); + auto child = _createSubtree(leafOffset, localEndIndex, onCreateLeaf); + inner->addChild(child.key()); } } -vector> DataTree::getOrCreateChildren(DataInnerNode *node, uint32_t begin, uint32_t end) { - vector> children; - children.reserve(end-begin); - for (uint32_t childIndex = begin; childIndex < std::min(node->numChildren(), end); ++childIndex) { - auto child = _nodeStore->load(node->getChild(childIndex)->key()); - ASSERT(child != none, "Couldn't load child node"); - children.emplace_back(std::move(*child)); +unique_ref DataTree::_createSubtree(uint32_t leafOffset, uint32_t numLeaves, std::function onCreateLeaf) { + if (numLeaves == 1) { + auto data = onCreateLeaf(leafOffset); + ASSERT(data.size() <= _nodeStore->layout().maxBytesPerLeaf(), "Too much data for a leaf"); + //TODO More efficient by _nodeStore->createNewLeafNode(data); + auto leaf = _nodeStore->createNewLeafNode(); + leaf->resize(data.size()); + leaf->write(data.data(), 0, data.size()); + return leaf; + } else { + uint32_t numLeafGroups = utils::ceilDivision(numLeaves, _nodeStore->layout().maxChildrenPerInnerNode()); + vector> children; + children.reserve(numLeafGroups); + for (uint32_t i = 0; i < numLeafGroups; ++i) { + children.push_back(_createSubtree()) + ... + } } - for (uint32_t childIndex = node->numChildren(); childIndex < end; ++childIndex) { - //TODO This creates each child with one chain to one leaf only, and then on the next lower level it - // has to create the children for the child. Would be faster to directly create full trees if necessary. - children.emplace_back(addChildTo(node)); - } - ASSERT(children.size() == end-begin, "Number of children in the result is wrong"); - return children; } unique_ref DataTree::addChildTo(DataInnerNode *node) { diff --git a/src/blobstore/implementations/onblocks/datatreestore/DataTree.h b/src/blobstore/implementations/onblocks/datatreestore/DataTree.h index 8fe74b62..c8fbfa3f 100644 --- a/src/blobstore/implementations/onblocks/datatreestore/DataTree.h +++ b/src/blobstore/implementations/onblocks/datatreestore/DataTree.h @@ -30,7 +30,7 @@ public: //Returning uint64_t, because calculations handling this probably need to be done in 64bit to support >4GB blobs. uint64_t maxBytesPerLeaf() const; - void traverseLeaves(uint32_t beginIndex, uint32_t endIndex, std::function func); + void traverseLeaves(uint32_t beginIndex, uint32_t endIndex, std::function onExistingLeaf, std::function onCreateLeaf); void resizeNumBytes(uint64_t newNumBytes); uint32_t numLeaves() const; @@ -62,7 +62,7 @@ private: void ifRootHasOnlyOneChildReplaceRootWithItsChild(); //TODO Use underscore for private methods - void _traverseLeaves(datanodestore::DataNode *root, uint32_t leafOffset, uint32_t beginIndex, uint32_t endIndex, std::function func); + void _traverseLeaves(datanodestore::DataNode *root, uint32_t leafOffset, uint32_t beginIndex, uint32_t endIndex, std::function onExistingLeaf, std::function onCreateLeaf); uint32_t leavesPerFullChild(const datanodestore::DataInnerNode &root) const; uint64_t _numStoredBytes() const; uint64_t _numStoredBytes(const datanodestore::DataNode &root) const; @@ -71,7 +71,7 @@ private: cpputils::optional_ownership_ptr LastLeaf(datanodestore::DataNode *root); cpputils::unique_ref LastLeaf(cpputils::unique_ref root); datanodestore::DataInnerNode* increaseTreeDepth(unsigned int levels); - std::vector> getOrCreateChildren(datanodestore::DataInnerNode *node, uint32_t begin, uint32_t end); + cpputils::unique_ref _createSubtree(uint32_t leafOffset, uint32_t numLeaves, std::function onCreateLeaf); cpputils::unique_ref addChildTo(datanodestore::DataInnerNode *node); DISALLOW_COPY_AND_ASSIGN(DataTree);