Skip to content

Commit

Permalink
Prevent unnecessary shared_ptr copies by accepting a value in `SHAM…
Browse files Browse the repository at this point in the history
…apInnerNode::setChild` (#4266)

* Do a move instead of a copy in `SHAMapInnerNode::setChild`

* Create the value directly in the call
  • Loading branch information
greg7mdp authored Dec 9, 2022
1 parent 4a5ad4c commit c1e7fe2
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/ripple/shamap/SHAMapInnerNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ class SHAMapInnerNode final : public SHAMapTreeNode,
getChildHash(int m) const;

void
setChild(int m, std::shared_ptr<SHAMapTreeNode> const& child);
setChild(int m, std::shared_ptr<SHAMapTreeNode> child);

void
shareChild(int m, std::shared_ptr<SHAMapTreeNode> const& child);
Expand Down
7 changes: 3 additions & 4 deletions src/ripple/shamap/impl/SHAMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ SHAMap::dirtyUp(
assert(branch >= 0);

node = unshareNode(std::move(node), nodeID);
node->setChild(branch, child);
node->setChild(branch, std::move(child));

child = std::move(node);
}
Expand Down Expand Up @@ -718,7 +718,7 @@ SHAMap::delItem(uint256 const& id)
stack.pop();

node = unshareNode(std::move(node), nodeID);
node->setChild(selectBranch(nodeID, id), prevNode);
node->setChild(selectBranch(nodeID, id), std::move(prevNode));

if (!nodeID.isRoot())
{
Expand Down Expand Up @@ -795,8 +795,7 @@ SHAMap::addGiveItem(SHAMapNodeType type, std::shared_ptr<SHAMapItem const> item)
auto inner = std::static_pointer_cast<SHAMapInnerNode>(node);
int branch = selectBranch(nodeID, tag);
assert(inner->isEmptyBranch(branch));
auto newNode = makeTypedLeaf(type, std::move(item), cowid_);
inner->setChild(branch, newNode);
inner->setChild(branch, makeTypedLeaf(type, std::move(item), cowid_));
}
else
{
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/shamap/impl/SHAMapInnerNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ SHAMapInnerNode::getString(const SHAMapNodeID& id) const

// We are modifying an inner node
void
SHAMapInnerNode::setChild(int m, std::shared_ptr<SHAMapTreeNode> const& child)
SHAMapInnerNode::setChild(int m, std::shared_ptr<SHAMapTreeNode> child)
{
assert((m >= 0) && (m < branchFactor));
assert(cowid_ != 0);
Expand All @@ -310,7 +310,7 @@ SHAMapInnerNode::setChild(int m, std::shared_ptr<SHAMapTreeNode> const& child)
auto const childIndex = *getChildIndex(m);
auto [_, hashes, children] = hashesAndChildren_.getHashesAndChildren();
hashes[childIndex].zero();
children[childIndex] = child;
children[childIndex] = std::move(child);
}

hash_.zero();
Expand Down

0 comments on commit c1e7fe2

Please sign in to comment.