Skip to content

Commit

Permalink
Remove removal of useful {} to bracket the flow of code into meaningf…
Browse files Browse the repository at this point in the history
…ul explicit vs implicit blocks.
  • Loading branch information
Bryan Thompson committed Jan 4, 2025
1 parent f9f50fa commit 852ff2b
Showing 1 changed file with 83 additions and 78 deletions.
161 changes: 83 additions & 78 deletions olc_art.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1383,9 +1383,10 @@ bool olc_db::iterator::try_first() noexcept {
return false;

Check warning on line 1383 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1383

Added line #L1383 was not covered by tests
auto node{db_.root.load()};
if (UNODB_DETAIL_UNLIKELY(node == nullptr)) {
if (UNODB_DETAIL_UNLIKELY(!parent_critical_section.try_read_unlock()))
if (UNODB_DETAIL_UNLIKELY(!parent_critical_section.try_read_unlock())) {
return false; // data race - retry.

Check warning on line 1387 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1387

Added line #L1387 was not covered by tests
return true; // empty tree.
}
return true; // empty tree.
}
return try_left_most_traversal(node, parent_critical_section);
}
Expand All @@ -1400,9 +1401,10 @@ bool olc_db::iterator::try_last() noexcept {
return false;

Check warning on line 1401 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1401

Added line #L1401 was not covered by tests
auto node{db_.root.load()};
if (UNODB_DETAIL_UNLIKELY(node == nullptr)) {
if (UNODB_DETAIL_UNLIKELY(!parent_critical_section.try_read_unlock()))
if (UNODB_DETAIL_UNLIKELY(!parent_critical_section.try_read_unlock())) {
return false; // data race - retry.

Check warning on line 1405 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1405

Added line #L1405 was not covered by tests
return true; // empty tree.
}
return true; // empty tree.
}
return try_right_most_traversal(node, parent_critical_section);
}
Expand Down Expand Up @@ -1511,9 +1513,10 @@ inline bool olc_db::iterator::try_left_most_traversal(
const auto node_type = node.type();
if (node_type == node_type::LEAF) {
push_leaf(node, node_critical_section);
if (UNODB_DETAIL_UNLIKELY(!node_critical_section.try_read_unlock()))
if (UNODB_DETAIL_UNLIKELY(!node_critical_section.try_read_unlock())) {
return false; // LCOV_EXCL_LINE
return true; // done
}
return true; // done
}
// recursive descent.
auto *const inode{node.ptr<detail::olc_inode *>()};

Check warning on line 1522 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1522

Added line #L1522 was not covered by tests
Expand Down Expand Up @@ -1551,9 +1554,10 @@ inline bool olc_db::iterator::try_right_most_traversal(
const auto node_type = node.type();
if (node_type == node_type::LEAF) {
push_leaf(node, node_critical_section);
if (UNODB_DETAIL_UNLIKELY(!node_critical_section.try_read_unlock()))
if (UNODB_DETAIL_UNLIKELY(!node_critical_section.try_read_unlock())) {
return false; // LCOV_EXCL_LINE
return true; // done
}
return true; // done
}
// recursive descent.
auto *const inode{node.ptr<detail::olc_inode *>()};
Expand Down Expand Up @@ -1588,8 +1592,12 @@ static inline bool UNLOCK(optimistic_lock::read_critical_section &cs,
// consider the cases for both forward traversal and reverse traversal
// from a key that is not in the data.
//
// FIXME We could do partial invalidation, in which case caller's
// might need to explicitly unwind the stack to the first valid node.
// TODO(thompsonbry) We could do partial invalidation, in which case
// caller's might need to explicitly unwind the stack to the first
// valid node. This is deferred for now as it would make the logic
// more complicated and there is no data as yet about the importance
// of this (which just optimizes part of the seek away) while the code
// complexity would be definitely increased.
bool olc_db::iterator::try_seek(detail::art_key search_key, bool &match,
bool fwd) noexcept {
invalidate(); // invalidate the iterator (clear the stack).
Expand All @@ -1599,9 +1607,10 @@ bool olc_db::iterator::try_seek(detail::art_key search_key, bool &match,
return false;

Check warning on line 1607 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1607

Added line #L1607 was not covered by tests
auto node{db_.root.load()};
if (UNODB_DETAIL_UNLIKELY(node == nullptr)) {
if (UNODB_DETAIL_UNLIKELY(!parent_critical_section.try_read_unlock()))
if (UNODB_DETAIL_UNLIKELY(!parent_critical_section.try_read_unlock())) {
return false; // data race - retry.

Check warning on line 1611 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1611

Added line #L1611 was not covered by tests
return true; // empty tree.
}
return true; // empty tree.
}
const detail::art_key k = search_key;
auto remaining_key{k};
Expand Down Expand Up @@ -1753,76 +1762,72 @@ bool olc_db::iterator::try_seek(detail::art_key search_key, bool &match,
push(node, tmp.key_byte, child_index, // push the path we took
node_critical_section);
return try_left_most_traversal(child, node_critical_section);
} else {
// REV: Take the prior child_index that is mapped and then do
// a right-most descent to land on the key that is the
// immediate precessor of the desired key in the data.
auto nxt = inode->lte_key_byte(node_type, remaining_key[0]);
if (!nxt) {
// Pop off the current entry until we find one with a
// left-sibling and then do a right-most descent under that
// left-sibling. In the extreme case there is no such
// previous entry and we will wind up with an empty stack.
if (UNODB_DETAIL_UNLIKELY(
!parent_critical_section.try_read_unlock())) // unlock parent
return false; // LCOV_EXCL_LINE
if (UNODB_DETAIL_UNLIKELY(
!node_critical_section.try_read_unlock())) // unlock node
return false; // LCOV_EXCL_LINE
if (!empty()) pop();
while (!empty()) {
const auto centry = top();
const auto cnode{centry.node}; // a possible parent from stack
auto c_critical_section(
node_ptr_lock(cnode).rehydrate_read_lock(centry.version));
if (!UNODB_DETAIL_UNLIKELY(c_critical_section.check()))
return false; // restart check
auto *const icnode{cnode.ptr<detail::olc_inode *>()};
const auto cnxt = icnode->prior(
cnode.type(), centry.child_index); // left-sibling.
if (cnxt) {
auto nchild = icnode->get_child(
cnode.type(), centry.child_index); // get the child
if (UNODB_DETAIL_UNLIKELY(
!c_critical_section.check())) // before using [nchild]
return false; // LCOV_EXCL_LINE
return try_right_most_traversal(nchild, c_critical_section);
}
pop();
if (!UNODB_DETAIL_UNLIKELY(c_critical_section.try_read_unlock()))
return false;
}
// REV: Take the prior child_index that is mapped and then do
// a right-most descent to land on the key that is the
// immediate precessor of the desired key in the data.
auto nxt = inode->lte_key_byte(node_type, remaining_key[0]);
if (!nxt) {
// Pop off the current entry until we find one with a
// left-sibling and then do a right-most descent under that
// left-sibling. In the extreme case there is no such
// previous entry and we will wind up with an empty stack.
if (UNODB_DETAIL_UNLIKELY(
!parent_critical_section.try_read_unlock())) // unlock parent
return false; // LCOV_EXCL_LINE
if (UNODB_DETAIL_UNLIKELY(
!node_critical_section.try_read_unlock())) // unlock node
return false; // LCOV_EXCL_LINE
if (!empty()) pop();
while (!empty()) {
const auto centry = top();
const auto cnode{centry.node}; // a possible parent from stack

Check warning on line 1784 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1783-L1784

Added lines #L1783 - L1784 were not covered by tests
auto c_critical_section(
node_ptr_lock(cnode).rehydrate_read_lock(centry.version));
if (!UNODB_DETAIL_UNLIKELY(c_critical_section.check()))
return false; // restart check
auto *const icnode{cnode.ptr<detail::olc_inode *>()};

Check warning on line 1789 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1786-L1789

Added lines #L1786 - L1789 were not covered by tests
const auto cnxt =
icnode->prior(cnode.type(), centry.child_index); // left-sibling.
if (cnxt) {
auto nchild = icnode->get_child(
cnode.type(), centry.child_index); // get the child
if (UNODB_DETAIL_UNLIKELY(

Check warning on line 1795 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1791-L1795

Added lines #L1791 - L1795 were not covered by tests
!c_critical_section.check())) // before using [nchild]
return false; // LCOV_EXCL_LINE
return try_right_most_traversal(nchild, c_critical_section);

Check warning on line 1798 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1798

Added line #L1798 was not covered by tests
}
return true; // stack is empty (aka end()).
} else {
auto tmp = nxt.value(); // unwrap.
const auto child_index = tmp.child_index;
const auto child =
inode->get_child(node_type, child_index); // get the child
if (UNODB_DETAIL_UNLIKELY(
!node_critical_section.check())) // before using [child]
return false; // LCOV_EXCL_LINE
if (UNODB_DETAIL_UNLIKELY(
!parent_critical_section.try_read_unlock())) // unlock parent
return false; // LCOV_EXCL_LINE
push(node, tmp.key_byte, child_index, // push the path we took
node_critical_section);
return try_right_most_traversal(child, node_critical_section);
pop();
if (!UNODB_DETAIL_UNLIKELY(c_critical_section.try_read_unlock()))
return false;
}

Check warning on line 1803 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1800-L1803

Added lines #L1800 - L1803 were not covered by tests
return true; // stack is empty (aka end()).
}
UNODB_DETAIL_CANNOT_HAPPEN();
} else {
// Simple case. There is a child for the current key byte.
const auto child_index{res.first};
const auto *const child{res.second};
push(node, remaining_key[0], child_index, node_critical_section);
node = *child;
remaining_key.shift_right(1);
// check node before using [child] and before we std::move() the RCS.
if (UNODB_DETAIL_UNLIKELY(!node_critical_section.check()))
return false; // LCOV_EXCL_LINE
// Move RCS (will check invariant at top of loop)
parent_critical_section = std::move(node_critical_section);
auto tmp = nxt.value(); // unwrap.
const auto child_index = tmp.child_index;
const auto child =
inode->get_child(node_type, child_index); // get the child
if (UNODB_DETAIL_UNLIKELY(
!node_critical_section.check())) // before using [child]
return false; // LCOV_EXCL_LINE
if (UNODB_DETAIL_UNLIKELY(
!parent_critical_section.try_read_unlock())) // unlock parent
return false; // LCOV_EXCL_LINE
push(node, tmp.key_byte, child_index, // push the path we took
node_critical_section);
return try_right_most_traversal(child, node_critical_section);
}
// Simple case. There is a child for the current key byte.
const auto child_index{res.first};
const auto *const child{res.second};
push(node, remaining_key[0], child_index, node_critical_section);
node = *child;
remaining_key.shift_right(1);
// check node before using [child] and before we std::move() the RCS.
if (UNODB_DETAIL_UNLIKELY(!node_critical_section.check()))
return false; // LCOV_EXCL_LINE
// Move RCS (will check invariant at top of loop)
parent_critical_section = std::move(node_critical_section);
} // while ( true )
UNODB_DETAIL_CANNOT_HAPPEN();
}
Expand Down

0 comments on commit 852ff2b

Please sign in to comment.