Skip to content

Commit

Permalink
Low level coverage exclusions part one of N
Browse files Browse the repository at this point in the history
  • Loading branch information
Bryan Thompson committed Jan 4, 2025
1 parent 852ff2b commit 0b99480
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 35 deletions.
2 changes: 2 additions & 0 deletions art.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ void db::dump(std::ostream &os) const {
///

void db::iterator::dump(std::ostream &os) const {

Check warning on line 457 in art.cpp

View check run for this annotation

Codecov / codecov/patch

art.cpp#L457

Added line #L457 was not covered by tests
// LCOV_EXCL_START
if (empty()) {
os << "iter::stack:: empty\n";
return;
Expand All @@ -477,6 +478,7 @@ void db::iterator::dump(std::ostream &os) const {
tmp.pop();
level--;
}
// LCOV_EXCL_STOP
}

Check warning on line 482 in art.cpp

View check run for this annotation

Codecov / codecov/patch

art.cpp#L482

Added line #L482 was not covered by tests

// Traverse to the left-most leaf. The stack is cleared first and then
Expand Down
6 changes: 6 additions & 0 deletions art.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -564,9 +564,15 @@ inline bool db::iterator::operator==(const iterator& other) const noexcept {
if (stack_.empty() != other.stack_.empty())
return false; // one stack is empty and the other is not?
if (stack_.empty()) return true; // both empty.
// Note: we always test an iterator against end so we never hit the
// code below. And this is always done from within the scan()
// kernels since the iterator is not a public API.
//
// LCOV_EXCL_START
const auto& a = stack_.top();
const auto& b = other.stack_.top();
return a == b; // top of stack is same (inode, key, and child_index).
// LCOV_EXCL_STOP
}

inline bool db::iterator::operator!=(const iterator& other) const noexcept {
Expand Down
16 changes: 9 additions & 7 deletions art_internal_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -714,8 +714,8 @@ class basic_inode_impl : public ArtPolicy::header_type {
case node_type::I256:
return static_cast<inode256_type *>(this)->remove_or_choose_subtree(
std::forward<Args>(args)...);
case node_type::LEAF:
// LCOV_EXCL_START
case node_type::LEAF:
UNODB_DETAIL_CANNOT_HAPPEN();
}
UNODB_DETAIL_CANNOT_HAPPEN();
Expand Down Expand Up @@ -1341,15 +1341,15 @@ class basic_inode_4 : public basic_inode_4_parent<ArtPolicy> {
return children[child_index].load();
}

// N4 - position on the first child (there is always at least one child for
// N4).
// N4 - position on the first child (there is always at least two
// children for N4).
[[nodiscard]] constexpr typename basic_inode_4::iter_result begin() noexcept {
const auto key = keys.byte_array[0].load();
return {node_ptr{this, node_type::I4}, key, static_cast<uint8_t>(0)};
}

// N4 - position on the last child (there is always at least one child for
// N4).
// N4 - position on the last child (there is always at least two
// children for N4).
//
// TODO(laurynas) The iter_result-returning sequences follow the
// same pattern once child_index is known. Look into extracting a
Expand Down Expand Up @@ -2178,7 +2178,8 @@ class basic_inode_48 : public basic_inode_48_parent<ArtPolicy> {
return {node_ptr{this, node_type::I48}, key, child_index};
}
}
UNODB_DETAIL_CANNOT_HAPPEN(); // because we always have at least 17 keys.
// because we always have at least 17 keys.
UNODB_DETAIL_CANNOT_HAPPEN(); // LCOV_EXCL_LINE
}

[[nodiscard]] constexpr typename basic_inode_48::iter_result_opt next(
Expand Down Expand Up @@ -2522,7 +2523,8 @@ class basic_inode_256 : public basic_inode_256_parent<ArtPolicy> {
return {node_ptr{this, node_type::I256}, key, child_index};
}
}
UNODB_DETAIL_CANNOT_HAPPEN(); // because we always have at least 49 keys.
// because we always have at least 49 keys.
UNODB_DETAIL_CANNOT_HAPPEN(); // LCOV_EXCL_LINE
}

[[nodiscard]] constexpr typename basic_inode_256::iter_result_opt next(
Expand Down
39 changes: 23 additions & 16 deletions olc_art.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,7 @@ olc_db::try_get_result_type olc_db::try_get(detail::art_key k) const noexcept {
detail::olc_node_ptr node{root.load()};
if (UNODB_DETAIL_UNLIKELY(node == nullptr)) { // special path if empty tree.
if (UNODB_DETAIL_UNLIKELY(!parent_critical_section.try_read_unlock()))
return {}; // data race - retry.
return {}; // LCOV_EXCL_LINE
// return an empty result (breaks out of caller's while(true) loop)
return std::make_optional<get_result>(std::nullopt);
}
Expand Down Expand Up @@ -1273,6 +1273,7 @@ void olc_db::dump(std::ostream &os) const {
///

void olc_db::iterator::dump(std::ostream &os) const {

Check warning on line 1275 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1275

Added line #L1275 was not covered by tests
// LCOV_EXCL_START
if (empty()) {
os << "iter::stack:: empty\n";
return;
Expand All @@ -1298,6 +1299,7 @@ void olc_db::iterator::dump(std::ostream &os) const {
tmp.pop();
level--;
}
// LCOV_EXCL_STOP
}

Check warning on line 1303 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1303

Added line #L1303 was not covered by tests

olc_db::iterator &olc_db::iterator::first() noexcept {
Expand Down Expand Up @@ -1336,7 +1338,7 @@ olc_db::iterator &olc_db::iterator::next() noexcept {
return *this; // done.
}
}
return *this;
return *this; // LCOV_EXCL_LINE
}

olc_db::iterator &olc_db::iterator::prior() noexcept {
Expand All @@ -1363,7 +1365,7 @@ olc_db::iterator &olc_db::iterator::prior() noexcept {
return *this; // done.
}

Check warning on line 1366 in olc_art.cpp

View check run for this annotation

Codecov / codecov/patch

olc_art.cpp#L1364-L1366

Added lines #L1364 - L1366 were not covered by tests
}
return *this;
return *this; // LCOV_EXCL_LINE
}

olc_db::iterator &olc_db::iterator::seek(detail::art_key search_key,
Expand All @@ -1380,11 +1382,12 @@ bool olc_db::iterator::try_first() noexcept {
invalidate(); // clear the stack
auto parent_critical_section = db_.root_pointer_lock.try_read_lock();
if (UNODB_DETAIL_UNLIKELY(parent_critical_section.must_restart()))
return false;
return false; // LCOV_EXCL_LINE
auto node{db_.root.load()};
if (UNODB_DETAIL_UNLIKELY(node == nullptr)) {
if (UNODB_DETAIL_UNLIKELY(!parent_critical_section.try_read_unlock())) {
return false; // data race - retry.
// data race - retry.
return false; // LCOV_EXCL_LINE
}
return true; // empty tree.
}
Expand All @@ -1402,7 +1405,8 @@ bool olc_db::iterator::try_last() noexcept {
auto node{db_.root.load()};
if (UNODB_DETAIL_UNLIKELY(node == nullptr)) {
if (UNODB_DETAIL_UNLIKELY(!parent_critical_section.try_read_unlock())) {
return false; // data race - retry.
// data race - retry.
return false; // LCOV_EXCL_LINE
}
return true; // empty tree.
}
Expand Down Expand Up @@ -1430,12 +1434,14 @@ bool olc_db::iterator::try_next() noexcept {
auto *inode{node.ptr<detail::olc_inode *>()};
auto nxt = inode->next(node_type,
e.child_index); // next child of that parent.
if (!UNODB_DETAIL_UNLIKELY(node_critical_section.check()))
return false; // restart check
if (!UNODB_DETAIL_UNLIKELY(node_critical_section.check())) {
// restart check
return false; // LCOV_EXCL_LINE
}
if (!nxt) {
pop(); // Nothing more for that inode.
if (!UNODB_DETAIL_UNLIKELY(node_critical_section.try_read_unlock()))
return false; // unlock
return false; // LCOV_EXCL_LINE
continue; // We will look for the right sibling of the parent inode.
}
// Fix up stack for new parent node state and left-most descent.
Expand All @@ -1461,23 +1467,23 @@ bool olc_db::iterator::try_prior() noexcept {
auto node_critical_section(
node_ptr_lock(node).rehydrate_read_lock(e.version));
if (!UNODB_DETAIL_UNLIKELY(node_critical_section.check()))
return false; // restart check
return false; // LCOV_EXCL_LINE
auto node_type = node.type();
if (node_type == node_type::LEAF) {
pop(); // pop off the leaf
if (!UNODB_DETAIL_UNLIKELY(node_critical_section.try_read_unlock()))
return false; // unlock
return false; // LCOV_EXCL_LINE
continue; // falls through loop if just a root leaf since stack now empty
}
auto *inode{node.ptr<detail::olc_inode *>()};
auto nxt = inode->prior(node_type,
e.child_index); // previous child of that parent.
if (!UNODB_DETAIL_UNLIKELY(node_critical_section.check()))
return false; // restart check
return false; // LCOV_EXCL_LINE
if (!nxt) {
pop(); // Nothing more for that inode.
if (!UNODB_DETAIL_UNLIKELY(node_critical_section.try_read_unlock()))
return false; // unlock
return false; // LCOV_EXCL_LINE
continue; // We will look for the left sibling of the parent inode.
}
// Fix up stack for new parent node state and right-most descent.
Expand Down Expand Up @@ -1548,7 +1554,7 @@ inline bool olc_db::iterator::try_right_most_traversal(
// Lock version chaining (node and parent)
auto node_critical_section = node_ptr_lock(node).try_read_lock();
if (UNODB_DETAIL_UNLIKELY(node_critical_section.must_restart()))
return false;
return false; // LCOV_EXCL_LINE
if (UNODB_DETAIL_UNLIKELY(!parent_critical_section.try_read_unlock()))
return false; // LCOV_EXCL_LINE
const auto node_type = node.type();
Expand Down Expand Up @@ -1604,11 +1610,12 @@ bool olc_db::iterator::try_seek(detail::art_key search_key, bool &match,
match = false; // unless we wind up with an exact match.
auto parent_critical_section = db_.root_pointer_lock.try_read_lock();
if (UNODB_DETAIL_UNLIKELY(parent_critical_section.must_restart()))
return false;
return false; // LCOV_EXCL_START
auto node{db_.root.load()};
if (UNODB_DETAIL_UNLIKELY(node == nullptr)) {
if (UNODB_DETAIL_UNLIKELY(!parent_critical_section.try_read_unlock())) {
return false; // data race - retry.
// data race - retry.
return false; // LCOV_EXCL_LINE
}
return true; // empty tree.
}
Expand Down
7 changes: 7 additions & 0 deletions olc_art.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -804,9 +804,16 @@ inline bool olc_db::iterator::operator==(const iterator& other) const noexcept {
//
// This is looking at all components in the element on the top
// of the stack (including the read_critical_section).
//
// Note: we always test an iterator against end so we never hit the
// code below. And this is always done from within the scan()
// kernels since the iterator is not a public API.
//
// LCOV_EXCL_START
const auto& a = stack_.top();
const auto& b = other.stack_.top();
return a == b; // top of stack is same (inode, key, and child_index).
// LCOV_EXCL_STOP
}

inline int olc_db::iterator::cmp(const detail::art_key& akey) const noexcept {
Expand Down
23 changes: 19 additions & 4 deletions test/test_art_concurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,12 @@ class ARTConcurrencyTest : public ::testing::Test {
verifier->get_db().scan_range(to_key, from_key,
fn); // reverse scan
}
// std::cerr<<"scan: from_key="<<from_key<<", to_key="<<to_key<<",
// n="<<n<<", sum="<<sum<<std::endl;
break;
}
// LCOV_EXCL_START
default:
UNODB_DETAIL_CANNOT_HAPPEN();
// LCOV_EXCL_STOP
}
key++;
}
Expand Down Expand Up @@ -174,8 +174,10 @@ class ARTConcurrencyTest : public ::testing::Test {
}
break;
}
// LCOV_EXCL_START
default:
UNODB_DETAIL_CANNOT_HAPPEN();
// LCOV_EXCL_STOP
}
}
}
Expand Down Expand Up @@ -243,8 +245,19 @@ TYPED_TEST(ARTConcurrencyTest, Node256ParallelOps) {
TYPED_TEST(ARTConcurrencyTest, ParallelRandomInsertDeleteGetScan) {
constexpr auto thread_count = 4 * 3;
constexpr auto initial_keys = 2048;
constexpr auto ops_per_thread =
10'000; // configured at 10'000 for normal builds.
constexpr auto ops_per_thread = 10'000;

this->verifier.insert_key_range(0, initial_keys, true);
this->template parallel_test<thread_count, ops_per_thread>(
TestFixture::random_op_thread);
}

// A more challenging test using a smaller key range and the same
// number of threads and operations per thread.
TYPED_TEST(ARTConcurrencyTest, ParallelRandomInsertDeleteGetScan_2) {
constexpr auto thread_count = 4 * 3;
constexpr auto initial_keys = 512;
constexpr auto ops_per_thread = 10'000;

this->verifier.insert_key_range(0, initial_keys, true);
this->template parallel_test<thread_count, ops_per_thread>(
Expand All @@ -255,6 +268,7 @@ TYPED_TEST(ARTConcurrencyTest, ParallelRandomInsertDeleteGetScan) {
// the thread_count for your machine.
TYPED_TEST(ARTConcurrencyTest,

Check warning on line 269 in test/test_art_concurrency.cpp

View check run for this annotation

Codecov / codecov/patch

test/test_art_concurrency.cpp#L269

Added line #L269 was not covered by tests
DISABLED_ParallelRandomInsertDeleteGetScan_StressTest) {
// LCOV_EXCL_START
constexpr auto thread_count = 48;
constexpr auto initial_keys =
1024; // fewer keys and more threads is more challenging
Expand All @@ -264,6 +278,7 @@ TYPED_TEST(ARTConcurrencyTest,
this->verifier.insert_key_range(0, initial_keys, true);
this->template parallel_test<thread_count, ops_per_thread>(
TestFixture::random_op_thread);
// LCOV_EXCL_STOP
}

UNODB_END_TESTS()
Expand Down
24 changes: 16 additions & 8 deletions test/test_art_scan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ class ARTScanTest : public ::testing::Test {

// used with conditional compilation for debug.
[[maybe_unused]] static void dump(const std::vector<unodb::key>& x) {

Check warning on line 41 in test/test_art_scan.cpp

View check run for this annotation

Codecov / codecov/patch

test/test_art_scan.cpp#L41

Added line #L41 was not covered by tests
// LCOV_EXCL_START
std::cerr << "[";
auto it = x.begin();
while (it != x.end()) {
std::cerr << *it << " ";
it++;
}
std::cerr << "]" << std::endl;
// LCOV_EXCL_STOP
}

Check warning on line 51 in test/test_art_scan.cpp

View check run for this annotation

Codecov / codecov/patch

test/test_art_scan.cpp#L51

Added line #L51 was not covered by tests

// Test help creates an index and populates it with the ODD keys in
Expand Down Expand Up @@ -94,8 +96,10 @@ void doScanTest(const unodb::key fromKey, const unodb::key toKey,
auto fn = [&nactual, &eit,
eit2](unodb::visitor<typename TypeParam::iterator>& v) {
if (eit == eit2) {
// LCOV_EXCL_START
EXPECT_TRUE(false) << "ART scan should have halted.";
return true; // halt early.
// LCOV_EXCL_STOP
}
const auto ekey = *eit; // expected key to visit
const auto akey = v.get_key(); // actual key visited.
Expand All @@ -105,19 +109,23 @@ void doScanTest(const unodb::key fromKey, const unodb::key toKey,
v.dump(std::cerr);
}
if (ekey != akey) {
// LCOV_EXCL_START
EXPECT_EQ(ekey, akey);
return true; // halt early.
// LCOV_EXCL_STOP
}
nactual++; // count #of visited keys.
eit++; // advance iterator over the expected keys.
return false; // !halt (aka continue scan).
};
db.scan_range(fromKey, toKey, fn);
// LCOV_EXCL_START
EXPECT_TRUE(eit == eit2)
<< "Expected iterator should have been fully consumed, but was not (ART "
"scan visited too little).";
EXPECT_EQ(nactual, nexpected)
<< ", fromKey=" << fromKey << ", toKey=" << toKey << ", limit=" << limit;
// LCOV_EXCL_STOP
}

//
Expand All @@ -139,26 +147,26 @@ TYPED_TEST(ARTScanTest, scan_forward__empty_tree_keys_and_values) {
{
uint64_t n = 0;
auto fn = [&n](unodb::visitor<typename TypeParam::iterator>&) {
n++;
return false;
n++; // LCOV_EXCL_LINE
return false; // LCOV_EXCL_LINE
};
db.scan(fn);
UNODB_EXPECT_EQ(0, n);
}
{
uint64_t n = 0;
auto fn = [&n](unodb::visitor<typename TypeParam::iterator>&) {
n++;
return false;
n++; // LCOV_EXCL_LINE
return false; // LCOV_EXCL_LINE
};
db.scan_from(0x0000, fn);
UNODB_EXPECT_EQ(0, n);
}
{
uint64_t n = 0;
auto fn = [&n](unodb::visitor<typename TypeParam::iterator>&) {
n++;
return false;
n++; // LCOV_EXCL_LINE
return false; // LCOV_EXCL_LINE
};
db.scan_range(0x0000, 0xffff, fn);
UNODB_EXPECT_EQ(0, n);
Expand Down Expand Up @@ -328,8 +336,8 @@ TYPED_TEST(ARTScanTest, scan_reverse__empty_tree) {
TypeParam& db = verifier.get_db(); // reference to test db instance.
uint64_t n = 0;
auto fn = [&n](unodb::visitor<typename TypeParam::iterator>&) {
n++;
return false;
n++; // LCOV_EXCL_LINE
return false; // LCOV_EXCL_LINE
};
db.scan(fn, false /*fwd*/);
UNODB_EXPECT_EQ(0, n);
Expand Down

0 comments on commit 0b99480

Please sign in to comment.