Remove mistakenly checked in printfs

cc #3. Found some non-trivial dead code this time!

ConflictSet.cpp

@@ -472,7 +472,8 @@ inline void Node256::copyChildrenAndKeyFrom(const Node256 &other) {
 
 namespace {
 std::string getSearchPathPrintable(Node *n);
-}
+std::string getSearchPath(Node *n);
+} // namespace
 
 // Bound memory usage following the analysis in the ART paper
 
@@ -676,7 +677,7 @@ template <class NodeT> int getNodeIndex(NodeT *self, uint8_t index) {
 // Precondition - an entry for index must exist in the node
 Node *&getChildExists(Node *self, uint8_t index) {
   switch (self->getType()) {
-  case Type_Node0:
+  case Type_Node0:           // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   case Type_Node3: {
     auto *self3 = static_cast<Node3 *>(self);
@@ -1078,7 +1079,7 @@ void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
 void maybeDecreaseCapacity(Node *&self, NodeAllocators *allocators,
                            ConflictSet::Impl *impl) {
   const int maxCapacity =
-      (self->numChildren + int(self->entryPresent)) * self->partialKeyLen;
+      (self->numChildren + int(self->entryPresent)) * (self->partialKeyLen + 1);
   if (self->getCapacity() <= maxCapacity) {
     return;
   }
@@ -1097,7 +1098,7 @@ void maybeDownsize(Node *self, NodeAllocators *allocators,
   assert(self->getType() == kType);
   static_assert(kType != Type_Node0);
   switch (kType) {
-  case Type_Node0:
+  case Type_Node0:           // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   case Type_Node3: {
     auto *self3 = (Node3 *)self;
@@ -1201,7 +1202,7 @@ Node *erase(Node *self, NodeAllocators *allocators, ConflictSet::Impl *impl,
   if (self->numChildren != 0) {
     const bool update = result == dontInvalidate;
     switch (self->getType()) {
-    case Type_Node0:
+    case Type_Node0:           // GCOVR_EXCL_LINE
       __builtin_unreachable(); // GCOVR_EXCL_LINE
     case Type_Node3:
       maybeDownsize<Type_Node3>(self, allocators, impl, result);
@@ -1224,28 +1225,11 @@ Node *erase(Node *self, NodeAllocators *allocators, ConflictSet::Impl *impl,
     return result;
   }
 
-  switch (self->getType()) {
-  case Type_Node0:
-    allocators->node0.release((Node0 *)self);
-    break;
-  case Type_Node3:
-    allocators->node3.release((Node3 *)self);
-    break;
-  case Type_Node16:
-    allocators->node16.release((Node16 *)self);
-    break;
-  case Type_Node48:
-    allocators->node48.release((Node48 *)self);
-    break;
-  case Type_Node256:
-    allocators->node256.release((Node256 *)self);
-    break;
-  default:                   // GCOVR_EXCL_LINE
-    __builtin_unreachable(); // GCOVR_EXCL_LINE
-  }
+  assert(self->getType() == Type_Node0);
+  allocators->node0.release((Node0 *)self);
 
   switch (parent->getType()) {
-  case Type_Node0:
+  case Type_Node0:           // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   case Type_Node3: {
     auto *parent3 = static_cast<Node3 *>(parent);
@@ -1259,15 +1243,11 @@ Node *erase(Node *self, NodeAllocators *allocators, ConflictSet::Impl *impl,
                 (parent->numChildren - (nodeIndex + 1)));
 
     --parent->numChildren;
-    if (parent->numChildren == 0 && !parent->entryPresent &&
-        parent->parent != nullptr) {
-      return erase(parent, allocators, impl, dontInvalidate);
-    } else {
-      const bool update = result == dontInvalidate;
-      maybeDownsize<Type_Node3>(parent, allocators, impl, result);
-      if (update) {
-        dontInvalidate = result;
-      }
+    assert(parent->numChildren > 0 || parent->entryPresent);
+    const bool update = result == dontInvalidate;
+    maybeDownsize<Type_Node3>(parent, allocators, impl, result);
+    if (update) {
+      dontInvalidate = result;
     }
   } break;
   case Type_Node16: {
@@ -1282,16 +1262,16 @@ Node *erase(Node *self, NodeAllocators *allocators, ConflictSet::Impl *impl,
                 (parent->numChildren - (nodeIndex + 1)));
 
     --parent->numChildren;
-    if (parent->numChildren == 0 && !parent->entryPresent &&
-        parent->parent != nullptr) {
-      return erase(parent, allocators, impl, dontInvalidate);
-    } else {
-      const bool update = result == dontInvalidate;
-      maybeDownsize<Type_Node16>(parent, allocators, impl, result);
-      if (update) {
-        dontInvalidate = result;
-      }
+
+    // By kMinChildrenNode16
+    assert(parent->numChildren > 0);
+
+    const bool update = result == dontInvalidate;
+    maybeDownsize<Type_Node16>(parent, allocators, impl, result);
+    if (update) {
+      dontInvalidate = result;
     }
+
   } break;
   case Type_Node48: {
     auto *parent48 = static_cast<Node48 *>(parent);
@@ -1309,15 +1289,14 @@ Node *erase(Node *self, NodeAllocators *allocators, ConflictSet::Impl *impl,
     }
 
     --parent->numChildren;
-    if (parent->numChildren == 0 && !parent->entryPresent &&
-        parent->parent != nullptr) {
-      return erase(parent, allocators, impl, dontInvalidate);
-    } else {
-      const bool update = result == dontInvalidate;
-      maybeDownsize<Type_Node48>(parent, allocators, impl, result);
-      if (update) {
-        dontInvalidate = result;
-      }
+
+    // By kMinChildrenNode48
+    assert(parent->numChildren > 0);
+
+    const bool update = result == dontInvalidate;
+    maybeDownsize<Type_Node48>(parent, allocators, impl, result);
+    if (update) {
+      dontInvalidate = result;
     }
   } break;
   case Type_Node256: {
@@ -1326,15 +1305,14 @@ Node *erase(Node *self, NodeAllocators *allocators, ConflictSet::Impl *impl,
     parent256->children[parentsIndex].child = nullptr;
 
     --parent->numChildren;
-    if (parent->numChildren == 0 && !parent->entryPresent &&
-        parent->parent != nullptr) {
-      return erase(parent, allocators, impl, dontInvalidate);
-    } else {
-      const bool update = result == dontInvalidate;
-      maybeDownsize<Type_Node256>(parent, allocators, impl, result);
-      if (update) {
-        dontInvalidate = result;
-      }
+
+    // By kMinChildrenNode256
+    assert(parent->numChildren > 0);
+
+    const bool update = result == dontInvalidate;
+    maybeDownsize<Type_Node256>(parent, allocators, impl, result);
+    if (update) {
+      dontInvalidate = result;
     }
   } break;
   default:                   // GCOVR_EXCL_LINE
@@ -1374,7 +1352,7 @@ constexpr int kUnrollFactor = 4;
 bool compareStride(const uint8_t *ap, const uint8_t *bp) {
 #if defined(HAS_ARM_NEON)
   static_assert(kStride == 64);
-  uint8x16_t x[4];
+  uint8x16_t x[4]; // GCOVR_EXCL_LINE
   for (int i = 0; i < 4; ++i) {
     x[i] = vceqq_u8(vld1q_u8(ap + i * 16), vld1q_u8(bp + i * 16));
   }
@@ -1384,7 +1362,7 @@ bool compareStride(const uint8_t *ap, const uint8_t *bp) {
             uint64_t(-1);
 #elif defined(HAS_AVX)
   static_assert(kStride == 64);
-  __m128i x[4];
+  __m128i x[4]; // GCOVR_EXCL_LINE
   for (int i = 0; i < 4; ++i) {
     x[i] = _mm_cmpeq_epi8(_mm_loadu_si128((__m128i *)(ap + i * 16)),
                           _mm_loadu_si128((__m128i *)(bp + i * 16)));
@@ -1403,7 +1381,7 @@ bool compareStride(const uint8_t *ap, const uint8_t *bp) {
 int firstNeqStride(const uint8_t *ap, const uint8_t *bp) {
 #if defined(HAS_AVX)
   static_assert(kStride == 64);
-  uint64_t c[kStride / 16];
+  uint64_t c[kStride / 16]; // GCOVR_EXCL_LINE
   for (int i = 0; i < kStride; i += 16) {
     const auto a = _mm_loadu_si128((__m128i *)(ap + i));
     const auto b = _mm_loadu_si128((__m128i *)(bp + i));
@@ -1445,18 +1423,6 @@ int longestCommonPrefix(const uint8_t *ap, const uint8_t *bp, int cl) {
     goto bytes;
   }
 
-  // Optimistic early return
-  {
-    uint64_t a;
-    uint64_t b;
-    memcpy(&a, ap, 8);
-    memcpy(&b, bp, 8);
-    const auto mismatched = a ^ b;
-    if (mismatched) {
-      return std::countr_zero(mismatched) / 8;
-    }
-  }
-
   // kStride * kUnrollCount at a time
   end = cl & ~(kStride * kUnrollFactor - 1);
   while (i < end) {
@@ -1484,8 +1450,8 @@ int longestCommonPrefix(const uint8_t *ap, const uint8_t *bp, int cl) {
   // word at a time
   end = cl & ~(sizeof(uint64_t) - 1);
   while (i < end) {
-    uint64_t a;
-    uint64_t b;
+    uint64_t a; // GCOVR_EXCL_LINE
+    uint64_t b; // GCOVR_EXCL_LINE
     memcpy(&a, ap, 8);
     memcpy(&b, bp, 8);
     const auto mismatched = a ^ b;
@@ -1640,7 +1606,7 @@ int64_t maxBetweenExclusive(Node *n, int begin, int end) {
     }
   }
   switch (n->getType()) {
-  case Type_Node0:
+  case Type_Node0: // GCOVR_EXCL_LINE
     // We would have returned above, after not finding a child
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   case Type_Node3: {
@@ -1703,7 +1669,7 @@ Vector<uint8_t> getSearchPath(Arena &arena, Node *n) {
   }
   std::reverse(result.begin(), result.end());
   return result;
-}
+} // GCOVR_EXCL_LINE
 
 // Return true if the max version among all keys that start with key + [child],
 // where begin < child < end, is <= readVersion
@@ -2480,11 +2446,11 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     assert(n != nullptr);
     // Don't erase the root
     if (n == root) {
-      n = nextLogical(n);
+      n = nextPhysical(n);
     }
     for (; keyUpdates > 0 && n != nullptr; --keyUpdates) {
-      if (std::max(n->entry.pointVersion, n->entry.rangeVersion) <=
-          oldestVersion) {
+      if (n->entryPresent && std::max(n->entry.pointVersion,
+                                      n->entry.rangeVersion) <= oldestVersion) {
         // Any transaction n would have prevented from committing is
         // going to fail with TooOld anyway.
 
@@ -2495,7 +2461,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         n = erase(n, &allocators, this, dummy);
       } else {
         maybeDecreaseCapacity(n, &allocators, this);
-        n = nextLogical(n);
+        n = nextPhysical(n);
       }
     }
     if (n == nullptr) {
@@ -2839,8 +2805,8 @@ Iterator firstGeq(Node *n, std::string_view key) {
   case Type_Node256:
     minNumChildren = kMinChildrenNode256;
     break;
-  default:                   // GCOVR_EXCL_LINE
-    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  default:
+    abort();
   }
   if (node->numChildren + int(node->entryPresent) < minNumChildren) {
     fprintf(stderr,
@@ -2898,8 +2864,8 @@ int64_t getNodeSize(struct Node *n) {
     return sizeof(Node48);
   case Type_Node256:
     return sizeof(Node256);
-  default:                   // GCOVR_EXCL_LINE
-    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  default:
+    abort();
   }
 }
 

Internal.h

@@ -531,7 +531,7 @@ template <class ConflictSetImpl> struct TestDriver {
   ConflictSetImpl cs{oldestVersion};
   ReferenceImpl refImpl{oldestVersion};
 
-  constexpr static auto kMaxKeyLen = 128;
+  constexpr static auto kMaxKeyLen = 8;
 
   bool ok = true;
 

Jenkinsfile

@@ -97,7 +97,7 @@ pipeline {
             steps {
                 CleanBuildAndTest("-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_C_FLAGS=--coverage -DCMAKE_CXX_FLAGS=--coverage -DCMAKE_BUILD_TYPE=Debug")
                 sh '''
-                gcovr --exclude '.*third_party.*' --cobertura > build/coverage.xml
+                gcovr -f ConflictSet.cpp --cobertura > build/coverage.xml
                 '''
                 cobertura autoUpdateHealth: false, autoUpdateStability: false, coberturaReportFile: 'build/coverage.xml', conditionalCoverageTargets: '70, 0, 0', failUnhealthy: false, failUnstable: false, lineCoverageTargets: '80, 0, 0', maxNumberOfBuilds: 0, methodCoverageTargets: '80, 0, 0', onlyStable: false, sourceEncoding: 'ASCII', zoomCoverageChart: false
             }

corpus/05a79f06cf3f67f726dae68d18a2290f6c9a50c9

@@ -1 +0,0 @@
-: