refactor: Use unsigned int for branch-related operations (#7938)

Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Bart
2026-08-17 10:07:14 +00:00
committed by GitHub
parent 43d842926a
commit 5337d028a2
11 changed files with 195 additions and 184 deletions

View File

@@ -484,31 +484,36 @@ private:
// returns the first item at or below this node
SHAMapLeafNode*
firstBelow(SHAMapTreeNodePtr node, SharedPtrNodeStack& stack, int branch = 0) const;
firstBelow(SHAMapTreeNodePtr node, SharedPtrNodeStack& stack, unsigned int branch = 0u) const;
// returns the last item at or below this node
SHAMapLeafNode*
lastBelow(SHAMapTreeNodePtr node, SharedPtrNodeStack& stack, int branch = kBranchFactor) const;
lastBelow(
SHAMapTreeNodePtr node,
SharedPtrNodeStack& stack,
unsigned int branch = kBranchFactor) const;
// direction in which belowHelper scans an inner node's branches
enum class BelowDirection { First, Last };
// helper function for firstBelow and lastBelow
SHAMapLeafNode*
belowHelper(
SHAMapTreeNodePtr node,
SharedPtrNodeStack& stack,
int branch,
std::tuple<int, std::function<bool(int)>, std::function<void(int&)>> const& loopParams)
const;
unsigned int branch,
BelowDirection direction) const;
// Simple descent
// Get a child of the specified node
SHAMapTreeNode*
descend(SHAMapInnerNode*, int branch) const;
descend(SHAMapInnerNode*, unsigned int branch) const;
SHAMapTreeNode*
descendThrow(SHAMapInnerNode*, int branch) const;
descendThrow(SHAMapInnerNode*, unsigned int branch) const;
SHAMapTreeNodePtr
descend(SHAMapInnerNode&, int branch) const;
descend(SHAMapInnerNode&, unsigned int branch) const;
SHAMapTreeNodePtr
descendThrow(SHAMapInnerNode&, int branch) const;
descendThrow(SHAMapInnerNode&, unsigned int branch) const;
// Descend with filter
// If pending, callback is called as if it called fetchNodeNT
@@ -516,7 +521,7 @@ private:
SHAMapTreeNode*
descendAsync(
SHAMapInnerNode* parent,
int branch,
unsigned int branch,
SHAMapSyncFilter const* filter,
bool& pending,
descendCallback&&) const;
@@ -525,13 +530,13 @@ private:
descend(
SHAMapInnerNode* parent,
SHAMapNodeID const& parentID,
int branch,
unsigned int branch,
SHAMapSyncFilter const* filter) const;
// Non-storing
// Does not hook the returned node to its parent
SHAMapTreeNodePtr
descendNoStore(SHAMapInnerNode&, int branch) const;
descendNoStore(SHAMapInnerNode&, unsigned int branch) const;
/**
* If there is only one leaf below this node, get its contents
@@ -581,8 +586,8 @@ private:
using StackEntry = std::tuple<
SHAMapInnerNode*, // pointer to the node
SHAMapNodeID, // the node's ID
int, // while child we check first
int, // which child we check next
unsigned int, // which child we check first
unsigned int, // which child we check next
bool>; // whether we've found any missing children yet
// We explicitly choose to specify the use of std::deque here, because
@@ -596,7 +601,7 @@ private:
using DeferredNode = std::tuple<
SHAMapInnerNode*, // parent node
SHAMapNodeID, // parent node ID
int, // branch
unsigned int, // branch
SHAMapTreeNodePtr>; // node
int deferred;

View File

@@ -62,8 +62,8 @@ private:
*
* @param i index of the requested child
*/
std::optional<int>
getChildIndex(int i) const;
std::optional<unsigned int>
getChildIndex(unsigned int i) const;
/**
* Call the `f` callback for all 16 (branchFactor) branches - even if
@@ -125,28 +125,28 @@ public:
isEmpty() const;
bool
isEmptyBranch(int m) const;
isEmptyBranch(unsigned int branch) const;
int
unsigned int
getBranchCount() const;
SHAMapHash const&
getChildHash(int m) const;
getChildHash(unsigned int branch) const;
void
setChild(int m, SHAMapTreeNodePtr child);
setChild(unsigned int branch, SHAMapTreeNodePtr child);
void
shareChild(int m, SHAMapTreeNodePtr const& child);
shareChild(unsigned int branch, SHAMapTreeNodePtr const& child);
SHAMapTreeNode*
getChildPointer(int branch);
getChildPointer(unsigned int branch);
SHAMapTreeNodePtr
getChild(int branch);
getChild(unsigned int branch);
SHAMapTreeNodePtr
canonicalizeChild(int branch, SHAMapTreeNodePtr node);
canonicalizeChild(unsigned int branch, SHAMapTreeNodePtr node);
// sync functions
bool
@@ -190,12 +190,12 @@ SHAMapInnerNode::isEmpty() const
}
inline bool
SHAMapInnerNode::isEmptyBranch(int m) const
SHAMapInnerNode::isEmptyBranch(unsigned int branch) const
{
return (isBranch_ & (1 << m)) == 0;
return (isBranch_ & (1u << branch)) == 0u;
}
inline int
inline unsigned int
SHAMapInnerNode::getBranchCount() const
{
return popcnt16(isBranch_);

View File

@@ -53,7 +53,7 @@ public:
}
[[nodiscard]] SHAMapNodeID
getChildNodeID(unsigned int m) const;
getChildNodeID(unsigned int branch) const;
/**
* Create a SHAMapNodeID of a node with the depth of the node and
@@ -64,7 +64,7 @@ public:
* @return SHAMapNodeID of the node
*/
static SHAMapNodeID
createID(int depth, uint256 const& key);
createID(unsigned int depth, uint256 const& key);
/**
* Comparison operators

View File

@@ -219,11 +219,11 @@ public:
*
* @param i index of the requested child
*/
[[nodiscard]] std::optional<int>
getChildIndex(std::uint16_t isBranch, int i) const;
[[nodiscard]] std::optional<unsigned int>
getChildIndex(std::uint16_t isBranch, unsigned int i) const;
};
[[nodiscard]] inline int
[[nodiscard]] inline unsigned int
popcnt16(std::uint16_t a)
{
#if __cpp_lib_bitops
@@ -234,11 +234,11 @@ popcnt16(std::uint16_t a)
// fallback to table lookup
static constexpr auto tbl = []() {
std::array<std::uint8_t, 256> ret{};
for (int i = 0; i != 256; ++i)
for (auto i = 0u; i != 256u; ++i)
{
for (int j = 0; j != 8; ++j)
for (auto j = 0u; j != 8u; ++j)
{
if (i & (1 << j))
if (i & (1u << j))
ret[i]++;
}
}

View File

@@ -22,6 +22,11 @@ static_assert(
static_assert(
kBoundaries.back() == SHAMapInnerNode::kBranchFactor,
"Last element of boundaries must be number of children in a dense array");
static_assert(
kBoundaries.front() >= 1,
"TaggedPointer.ipp subtracts 1 from a numAllocated value derived from "
"kBoundaries, as an unsigned quantity, in several places; the smallest "
"boundary must stay non-zero or those subtractions underflow.");
// Terminology: A chunk is the memory being allocated from a block. A block
// contains multiple chunks. This is the terminology the boost documentation
@@ -148,16 +153,16 @@ TaggedPointer::iterChildren(std::uint16_t isBranch, F&& f) const
if (numAllocated == SHAMapInnerNode::kBranchFactor)
{
// dense case
for (int i = 0; i < SHAMapInnerNode::kBranchFactor; ++i)
for (auto i = 0u; i < SHAMapInnerNode::kBranchFactor; ++i)
f(hashes[i]);
}
else
{
// sparse case
int curHashI = 0;
for (int i = 0; i < SHAMapInnerNode::kBranchFactor; ++i)
auto curHashI = 0u;
for (auto i = 0u; i < SHAMapInnerNode::kBranchFactor; ++i)
{
if ((1 << i) & isBranch)
if ((1u << i) & isBranch)
{
f(hashes[curHashI++]);
}
@@ -176,9 +181,9 @@ TaggedPointer::iterNonEmptyChildIndexes(std::uint16_t isBranch, F&& f) const
if (capacity() == SHAMapInnerNode::kBranchFactor)
{
// dense case
for (int i = 0; i < SHAMapInnerNode::kBranchFactor; ++i)
for (auto i = 0u; i < SHAMapInnerNode::kBranchFactor; ++i)
{
if ((1 << i) & isBranch)
if ((1u << i) & isBranch)
{
f(i, i);
}
@@ -187,10 +192,10 @@ TaggedPointer::iterNonEmptyChildIndexes(std::uint16_t isBranch, F&& f) const
else
{
// sparse case
int curHashI = 0;
for (int i = 0; i < SHAMapInnerNode::kBranchFactor; ++i)
auto curHashI = 0u;
for (auto i = 0u; i < SHAMapInnerNode::kBranchFactor; ++i)
{
if ((1 << i) & isBranch)
if ((1u << i) & isBranch)
{
f(i, curHashI++);
}
@@ -216,14 +221,14 @@ TaggedPointer::destroyHashesAndChildren()
deallocateArrays(tag, ptr);
}
inline std::optional<int>
TaggedPointer::getChildIndex(std::uint16_t isBranch, int i) const
inline std::optional<unsigned int>
TaggedPointer::getChildIndex(std::uint16_t isBranch, unsigned int i) const
{
if (isDense())
return i;
// Sparse case
if ((isBranch & (1 << i)) == 0)
if ((isBranch & (1u << i)) == 0u)
{
// Empty branch. Sparse children do not store empty branches
return {};
@@ -273,10 +278,10 @@ inline TaggedPointer::TaggedPointer(
*this = std::move(other);
auto [srcDstNumAllocated, srcDstHashes, srcDstChildren] = getHashesAndChildren();
bool const srcDstIsDense = isDense();
int srcDstIndex = 0;
for (int i = 0; i < SHAMapInnerNode::kBranchFactor; ++i)
auto srcDstIndex = 0u;
for (auto i = 0u; i < SHAMapInnerNode::kBranchFactor; ++i)
{
auto const mask = (1 << i);
auto const mask = (1u << i);
bool const inSrc = (srcBranches & mask) != 0;
bool const inDst = (dstBranches & mask) != 0;
if (inSrc && inDst)
@@ -298,13 +303,13 @@ inline TaggedPointer::TaggedPointer(
// sparse
// need to shift all the elements to the left by
// one
for (int c = srcDstIndex; c < srcDstNumAllocated - 1; ++c)
for (auto c = srcDstIndex; c + 1 < srcDstNumAllocated; ++c)
{
srcDstHashes[c] = srcDstHashes[c + 1];
srcDstChildren[c] = std::move(srcDstChildren[c + 1]);
}
srcDstHashes[srcDstNumAllocated - 1].zero();
srcDstChildren[srcDstNumAllocated - 1].reset();
srcDstHashes[srcDstNumAllocated - 1u].zero();
srcDstChildren[srcDstNumAllocated - 1u].reset();
// do not increment the index
}
}
@@ -321,7 +326,7 @@ inline TaggedPointer::TaggedPointer(
// sparse
// need to create a hole by shifting all the elements to the
// right by one
for (int c = srcDstNumAllocated - 1; c > srcDstIndex; --c)
for (auto c = srcDstNumAllocated - 1u; c > srcDstIndex; --c)
{
srcDstHashes[c] = srcDstHashes[c - 1];
srcDstChildren[c] = std::move(srcDstChildren[c - 1]);
@@ -352,10 +357,10 @@ inline TaggedPointer::TaggedPointer(
auto [srcNumAllocated, srcHashes, srcChildren] = src.getHashesAndChildren();
bool const srcIsDense = src.isDense();
bool const dstIsDense = dst.isDense();
int srcIndex = 0, dstIndex = 0;
for (int i = 0; i < SHAMapInnerNode::kBranchFactor; ++i)
auto srcIndex = 0u, dstIndex = 0u;
for (auto i = 0u; i < SHAMapInnerNode::kBranchFactor; ++i)
{
auto const mask = (1 << i);
auto const mask = (1u << i);
bool const inSrc = (srcBranches & mask) != 0;
bool const inDst = (dstBranches & mask) != 0;
if (inSrc && inDst)
@@ -409,7 +414,7 @@ inline TaggedPointer::TaggedPointer(
!dstIsDense || dstIndex == dstNumAllocated,
"xrpl::TaggedPointer::TaggedPointer(TaggedPointer&& ...) : "
"non-sparse or valid sparse");
for (int i = dstIndex; i < dstNumAllocated; ++i)
for (auto i = dstIndex; i < dstNumAllocated; ++i)
{
new (&dstHashes[i]) SHAMapHash{};
new (&dstChildren[i]) SHAMapTreeNodePtr{};
@@ -448,9 +453,9 @@ inline TaggedPointer::TaggedPointer(
new (&newChildren[branchNum]) SHAMapTreeNodePtr{std::move(oldChildren[indexNum])};
});
// Run the constructors for the remaining elements
for (int i = 0; i < SHAMapInnerNode::kBranchFactor; ++i)
for (auto i = 0u; i < SHAMapInnerNode::kBranchFactor; ++i)
{
if (((1 << i) & isBranch) != 0)
if (((1u << i) & isBranch) != 0u)
continue;
new (&newHashes[i]) SHAMapHash{};
new (&newChildren[i]) SHAMapTreeNodePtr{};
@@ -459,7 +464,7 @@ inline TaggedPointer::TaggedPointer(
else
{
// new arrays are sparse, old arrays may be sparse or dense
int curCompressedIndex = 0;
auto curCompressedIndex = 0u;
iterNonEmptyChildIndexes(isBranch, [&](auto branchNum, auto indexNum) {
new (&newHashes[curCompressedIndex]) SHAMapHash{oldHashes[indexNum]};
new (&newChildren[curCompressedIndex])
@@ -467,7 +472,7 @@ inline TaggedPointer::TaggedPointer(
++curCompressedIndex;
});
// Run the constructors for the remaining elements
for (int i = curCompressedIndex; i < newNumAllocated; ++i)
for (auto i = curCompressedIndex; i < newNumAllocated; ++i)
{
new (&newHashes[i]) SHAMapHash{};
new (&newChildren[i]) SHAMapTreeNodePtr{};