Attempt to fix likely arm bug

It seems all callers ultimately want this

ConflictSet.cpp

@@ -718,15 +718,20 @@ struct ReadContext {
 // A type that's plumbed along the non-const call tree. Same lifetime as
 // ConflictSet::Impl
 struct WriteContext {
-  int64_t entries_erased_accum = 0;
-  int64_t insert_iterations_accum = 0;
-  int64_t entries_inserted_accum = 0;
-  int64_t nodes_allocated_accum = 0;
-  int64_t nodes_released_accum = 0;
-  int64_t point_writes_accum = 0;
-  int64_t range_writes_accum = 0;
+
+  struct Accum {
+    int64_t entries_erased = 0;
+    int64_t insert_iterations = 0;
+    int64_t entries_inserted = 0;
+    int64_t nodes_allocated = 0;
+    int64_t nodes_released = 0;
+    int64_t point_writes = 0;
+    int64_t range_writes = 0;
+    int64_t write_bytes = 0;
+  } accum;
+
   template <class T> T *allocate(int c) {
-    ++nodes_allocated_accum;
+    ++accum.nodes_allocated;
     if constexpr (std::is_same_v<T, Node0>) {
       return node0.allocate(c);
     } else if constexpr (std::is_same_v<T, Node3>) {
@@ -741,7 +746,7 @@ struct WriteContext {
   }
   template <class T> void release(T *c) {
     static_assert(!std::is_same_v<T, Node>);
-    ++nodes_released_accum;
+    ++accum.nodes_released;
     if constexpr (std::is_same_v<T, Node0>) {
       return node0.release(c);
     } else if constexpr (std::is_same_v<T, Node3>) {
@@ -839,28 +844,41 @@ template <class NodeT> int getNodeIndex(NodeT *self, uint8_t index) {
 #endif
 }
 
+// Precondition - an entry for index must exist in the node
+Node *&getChildExists(Node3 *self, uint8_t index) {
+  return self->children[getNodeIndex(self, index)];
+}
+// Precondition - an entry for index must exist in the node
+Node *&getChildExists(Node16 *self, uint8_t index) {
+  return self->children[getNodeIndex(self, index)];
+}
+// Precondition - an entry for index must exist in the node
+Node *&getChildExists(Node48 *self, uint8_t index) {
+  assert(self->bitSet.test(index));
+  return self->children[self->index[index]];
+}
+// Precondition - an entry for index must exist in the node
+Node *&getChildExists(Node256 *self, uint8_t index) {
+  assert(self->bitSet.test(index));
+  return self->children[index];
+}
+
 // Precondition - an entry for index must exist in the node
 Node *&getChildExists(Node *self, uint8_t index) {
   switch (self->getType()) {
   case Type_Node0:           // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   case Type_Node3: {
-    auto *self3 = static_cast<Node3 *>(self);
-    return self3->children[getNodeIndex(self3, index)];
+    return getChildExists(static_cast<Node3 *>(self), index);
   }
   case Type_Node16: {
-    auto *self16 = static_cast<Node16 *>(self);
-    return self16->children[getNodeIndex(self16, index)];
+    return getChildExists(static_cast<Node16 *>(self), index);
   }
   case Type_Node48: {
-    auto *self48 = static_cast<Node48 *>(self);
-    assert(self48->bitSet.test(index));
-    return self48->children[self48->index[index]];
+    return getChildExists(static_cast<Node48 *>(self), index);
   }
   case Type_Node256: {
-    auto *self256 = static_cast<Node256 *>(self);
-    assert(self256->bitSet.test(index));
-    return self256->children[index];
+    return getChildExists(static_cast<Node256 *>(self), index);
   }
   default:                   // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
@@ -873,35 +891,39 @@ void setMaxVersion(Node *n, ConflictSet::Impl *, InternalVersionT maxVersion);
 
 Node *&getInTree(Node *n, ConflictSet::Impl *);
 
+Node *getChild(Node0 *, uint8_t) { return nullptr; }
+Node *getChild(Node3 *self, uint8_t index) {
+  int i = getNodeIndex(self, index);
+  return i < 0 ? nullptr : self->children[i];
+}
+Node *getChild(Node16 *self, uint8_t index) {
+  int i = getNodeIndex(self, index);
+  return i < 0 ? nullptr : self->children[i];
+}
+Node *getChild(Node48 *self, uint8_t index) {
+  int i = self->index[index];
+  return i < 0 ? nullptr : self->children[i];
+}
+Node *getChild(Node256 *self, uint8_t index) { return self->children[index]; }
+
 Node *getChild(Node *self, uint8_t index) {
   switch (self->getType()) {
   case Type_Node0:
-    return nullptr;
-  case Type_Node3: {
-    auto *self3 = static_cast<Node3 *>(self);
-    int i = getNodeIndex(self3, index);
-    return i < 0 ? nullptr : self3->children[i];
-  }
-  case Type_Node16: {
-    auto *self16 = static_cast<Node16 *>(self);
-    int i = getNodeIndex(self16, index);
-    return i < 0 ? nullptr : self16->children[i];
-  }
-  case Type_Node48: {
-    auto *self48 = static_cast<Node48 *>(self);
-    int i = self48->index[index];
-    return i < 0 ? nullptr : self48->children[i];
-  }
-  case Type_Node256: {
-    auto *self256 = static_cast<Node256 *>(self);
-    return self256->children[index];
-  }
+    return getChild(static_cast<Node0 *>(self), index);
+  case Type_Node3:
+    return getChild(static_cast<Node3 *>(self), index);
+  case Type_Node16:
+    return getChild(static_cast<Node16 *>(self), index);
+  case Type_Node48:
+    return getChild(static_cast<Node48 *>(self), index);
+  case Type_Node256:
+    return getChild(static_cast<Node256 *>(self), index);
   default:                   // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   }
 }
 
-template <class NodeT> int getChildGeqSimd(NodeT *self, int child) {
+template <class NodeT> Node *getChildGeqSimd(NodeT *self, int child) {
   static_assert(std::is_same_v<NodeT, Node3> || std::is_same_v<NodeT, Node16>);
 
   // cachegrind says the plain loop is fewer instructions and more mis-predicted
@@ -912,10 +934,13 @@ template <class NodeT> int getChildGeqSimd(NodeT *self, int child) {
     Node3 *n = (Node3 *)self;
     for (int i = 0; i < n->numChildren; ++i) {
       if (n->index[i] >= child) {
-        return n->index[i];
+        return n->children[i];
       }
     }
-    return -1;
+    return nullptr;
+  }
+  if (child > 255) {
+    return nullptr;
   }
 
 #ifdef HAS_AVX
@@ -925,16 +950,7 @@ template <class NodeT> int getChildGeqSimd(NodeT *self, int child) {
   __m128i results = _mm_cmpeq_epi8(key_vec, _mm_min_epu8(key_vec, indices));
   int mask = (1 << self->numChildren) - 1;
   uint32_t bitfield = _mm_movemask_epi8(results) & mask;
-  int result = bitfield == 0 ? -1 : self->index[std::countr_zero(bitfield)];
-  assert(result == [&]() -> int {
-    for (int i = 0; i < self->numChildren; ++i) {
-      if (self->index[i] >= child) {
-        return self->index[i];
-      }
-    }
-    return -1;
-  }());
-  return result;
+  return bitfield == 0 ? nullptr : self->children[std::countr_zero(bitfield)];
 #elif defined(HAS_ARM_NEON)
   uint8x16_t indices;
   memcpy(&indices, self->index, sizeof(self->index));
@@ -951,47 +967,92 @@ template <class NodeT> int getChildGeqSimd(NodeT *self, int child) {
           vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(results), 4)),
           0) &
       mask;
-  int simd = bitfield == 0 ? -1 : self->index[std::countr_zero(bitfield) / 4];
-  assert(simd == [&]() -> int {
-    for (int i = 0; i < self->numChildren; ++i) {
-      if (self->index[i] >= child) {
-        return self->index[i];
-      }
-    }
-    return -1;
-  }());
-  return simd;
+  return bitfield == 0 ? nullptr
+                       : self->children[std::countr_zero(bitfield) / 4];
 #else
   for (int i = 0; i < self->numChildren; ++i) {
     if (i > 0) {
       assert(self->index[i - 1] < self->index[i]);
     }
     if (self->index[i] >= child) {
-      return self->index[i];
+      return self->children[i];
     }
   }
-  return -1;
+  return nullptr;
 #endif
 }
 
-int getChildGeq(Node *self, int child) {
-  if (child > 255) {
-    return -1;
+Node *getChildGeq(Node0 *, int) { return nullptr; }
+Node *getChildGeq(Node3 *self, int child) {
+  return getChildGeqSimd(self, child);
+}
+Node *getChildGeq(Node16 *self, int child) {
+  return getChildGeqSimd(self, child);
+}
+Node *getChildGeq(Node48 *self, int child) {
+  int c = self->bitSet.firstSetGeq(child);
+  if (c < 0) {
+    return nullptr;
   }
+  return self->children[self->index[c]];
+}
+Node *getChildGeq(Node256 *self, int child) {
+  int c = self->bitSet.firstSetGeq(child);
+  if (c < 0) {
+    return nullptr;
+  }
+  return self->children[c];
+}
+
+Node *getChildGeq(Node *self, int child) {
   switch (self->getType()) {
   case Type_Node0:
-    return -1;
+    return getChildGeq(static_cast<Node0 *>(self), child);
   case Type_Node3:
-    return getChildGeqSimd(static_cast<Node3 *>(self), child);
+    return getChildGeq(static_cast<Node3 *>(self), child);
   case Type_Node16:
-    return getChildGeqSimd(static_cast<Node16 *>(self), child);
+    return getChildGeq(static_cast<Node16 *>(self), child);
   case Type_Node48:
-    [[fallthrough]];
-  case Type_Node256: {
-    static_assert(offsetof(Node48, bitSet) == offsetof(Node256, bitSet));
-    auto *self48 = static_cast<Node48 *>(self);
-    return self48->bitSet.firstSetGeq(child);
+    return getChildGeq(static_cast<Node48 *>(self), child);
+  case Type_Node256:
+    return getChildGeq(static_cast<Node256 *>(self), child);
+  default:                   // GCOVR_EXCL_LINE
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
   }
+}
+
+// Precondition: self has a child
+Node *getFirstChildExists(Node3 *self) {
+  assert(self->numChildren > 0);
+  return self->children[0];
+}
+// Precondition: self has a child
+Node *getFirstChildExists(Node16 *self) {
+  assert(self->numChildren > 0);
+  return self->children[0];
+}
+// Precondition: self has a child
+Node *getFirstChildExists(Node48 *self) {
+  return self->children[self->index[self->bitSet.firstSetGeq(0)]];
+}
+// Precondition: self has a child
+Node *getFirstChildExists(Node256 *self) {
+  return self->children[self->bitSet.firstSetGeq(0)];
+}
+
+// Precondition: self has a child
+Node *getFirstChildExists(Node *self) {
+  switch (self->getType()) {
+  case Type_Node0:           // GCOVR_EXCL_LINE
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  case Type_Node3:
+    return getFirstChildExists(static_cast<Node3 *>(self));
+  case Type_Node16:
+    return getFirstChildExists(static_cast<Node16 *>(self));
+  case Type_Node48:
+    return getFirstChildExists(static_cast<Node48 *>(self));
+  case Type_Node256:
+    return getFirstChildExists(static_cast<Node256 *>(self));
   default:                   // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   }
@@ -1197,8 +1258,8 @@ Node *nextPhysical(Node *node) {
   int index = -1;
   for (;;) {
     auto nextChild = getChildGeq(node, index + 1);
-    if (nextChild >= 0) {
-      return getChildExists(node, nextChild);
+    if (nextChild != nullptr) {
+      return nextChild;
     }
     index = node->parentsIndex;
     node = node->parent;
@@ -1454,7 +1515,7 @@ void maybeDownsize(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
 // will update it to its new pointee as well. Precondition: `self->entryPresent`
 Node *erase(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
             bool logical, Node *&dontInvalidate) {
-  ++tls->entries_erased_accum;
+  ++tls->accum.entries_erased;
   assert(self->parent != nullptr);
 
 #if DEBUG_VERBOSE && !defined(NDEBUG)
@@ -1576,21 +1637,16 @@ Node *erase(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
   return result;
 }
 
-struct Iterator {
-  Node *n;
-  int cmp;
-};
-
 Node *nextSibling(Node *node) {
   for (;;) {
     if (node->parent == nullptr) {
       return nullptr;
     }
     auto next = getChildGeq(node->parent, node->parentsIndex + 1);
-    if (next < 0) {
+    if (next == nullptr) {
       node = node->parent;
     } else {
-      return getChildExists(node->parent, next);
+      return next;
     }
   }
 }
@@ -1783,17 +1839,15 @@ bool checkPointRead(Node *n, const std::span<const uint8_t> key,
       if (n->entryPresent) {
         return n->entry.pointVersion <= readVersion;
       }
-      int c = getChildGeq(n, 0);
-      assert(c >= 0);
-      n = getChildExists(n, c);
+      n = getFirstChildExists(n);
       goto downLeftSpine;
     }
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
-        n = getChildExists(n, c);
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
+        n = c;
         goto downLeftSpine;
       } else {
         n = nextSibling(n);
@@ -1833,14 +1887,9 @@ bool checkPointRead(Node *n, const std::span<const uint8_t> key,
     }
   }
 downLeftSpine:
-  for (;;) {
-    if (n->entryPresent) {
-      return n->entry.rangeVersion <= readVersion;
-    }
-    int c = getChildGeq(n, 0);
-    assert(c >= 0);
-    n = getChildExists(n, c);
+  for (; !n->entryPresent; n = getFirstChildExists(n)) {
   }
+  return n->entry.rangeVersion <= readVersion;
 }
 
 // Logically this is the same as performing firstGeq and then checking against
@@ -1867,9 +1916,9 @@ bool checkPrefixRead(Node *n, const std::span<const uint8_t> key,
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
-        n = getChildExists(n, c);
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
+        n = c;
         goto downLeftSpine;
       } else {
         n = nextSibling(n);
@@ -1913,14 +1962,9 @@ bool checkPrefixRead(Node *n, const std::span<const uint8_t> key,
     }
   }
 downLeftSpine:
-  for (;;) {
-    if (n->entryPresent) {
-      return n->entry.rangeVersion <= readVersion;
-    }
-    int c = getChildGeq(n, 0);
-    assert(c >= 0);
-    n = getChildExists(n, c);
+  for (; !n->entryPresent; n = getFirstChildExists(n)) {
   }
+  return n->entry.rangeVersion <= readVersion;
 }
 
 #ifdef HAS_AVX
@@ -2373,9 +2417,9 @@ bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
 
   auto *child = getChild(n, remaining[0]);
   if (child == nullptr) {
-    int c = getChildGeq(n, remaining[0]);
-    if (c >= 0) {
-      n = getChildExists(n, c);
+    auto c = getChildGeq(n, remaining[0]);
+    if (c != nullptr) {
+      n = c;
       goto downLeftSpine;
     } else {
       n = nextSibling(n);
@@ -2419,14 +2463,9 @@ bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
   __builtin_unreachable(); // GCOVR_EXCL_LINE
 
 downLeftSpine:
-  for (;;) {
-    if (n->entryPresent) {
-      return n->entry.rangeVersion <= readVersion;
-    }
-    int c = getChildGeq(n, 0);
-    assert(c >= 0);
-    n = getChildExists(n, c);
+  for (; !n->entryPresent; n = getFirstChildExists(n)) {
   }
+  return n->entry.rangeVersion <= readVersion;
 }
 
 namespace {
@@ -2474,13 +2513,13 @@ template <bool kAVX512> struct CheckRangeLeftSide {
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
         if (searchPathLen < prefixLen) {
-          n = getChildExists(n, c);
+          n = c;
           return downLeftSpine();
         }
-        n = getChildExists(n, c);
+        n = c;
         ok = maxVersion(n, impl) <= readVersion;
         return true;
       } else {
@@ -2539,15 +2578,10 @@ template <bool kAVX512> struct CheckRangeLeftSide {
   }
 
   bool downLeftSpine() {
-    for (;;) {
-      if (n->entryPresent) {
-        ok = n->entry.rangeVersion <= readVersion;
-        return true;
-      }
-      int c = getChildGeq(n, 0);
-      assert(c >= 0);
-      n = getChildExists(n, c);
+    for (; !n->entryPresent; n = getFirstChildExists(n)) {
     }
+    ok = n->entry.rangeVersion <= readVersion;
+    return true;
   }
 };
 
@@ -2611,9 +2645,9 @@ template <bool kAVX512> struct CheckRangeRightSide {
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
-        n = getChildExists(n, c);
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
+        n = c;
         return downLeftSpine();
       } else {
         return backtrack();
@@ -2663,12 +2697,12 @@ template <bool kAVX512> struct CheckRangeRightSide {
         return true;
       }
       auto next = getChildGeq(n->parent, n->parentsIndex + 1);
-      if (next < 0) {
+      if (next == nullptr) {
         searchPathLen -= 1 + n->partialKeyLen;
         n = n->parent;
       } else {
         searchPathLen -= n->partialKeyLen;
-        n = getChildExists(n->parent, next);
+        n = next;
         searchPathLen += n->partialKeyLen;
         return downLeftSpine();
       }
@@ -2676,15 +2710,10 @@ template <bool kAVX512> struct CheckRangeRightSide {
   }
 
   bool downLeftSpine() {
-    for (;;) {
-      if (n->entryPresent) {
-        ok = n->entry.rangeVersion <= readVersion;
-        return true;
-      }
-      int c = getChildGeq(n, 0);
-      assert(c >= 0);
-      n = getChildExists(n, c);
+    for (; !n->entryPresent; n = getFirstChildExists(n)) {
     }
+    ok = n->entry.rangeVersion <= readVersion;
+    return true;
   }
 };
 } // namespace
@@ -2834,7 +2863,7 @@ template <bool kBegin>
                            InternalVersionT writeVersion, WriteContext *tls,
                            ConflictSet::Impl *impl) {
 
-  for (;; ++tls->insert_iterations_accum) {
+  for (;; ++tls->accum.insert_iterations) {
 
     if ((*self)->partialKeyLen > 0) {
       // Handle an existing partial key
@@ -2932,9 +2961,8 @@ void destroyTree(Node *root) {
     auto *n = toFree.back();
     toFree.pop_back();
     // Add all children to toFree
-    for (int child = getChildGeq(n, 0); child >= 0;
-         child = getChildGeq(n, child + 1)) {
-      auto *c = getChildExists(n, child);
+    for (auto c = getChildGeq(n, 0); c != nullptr;
+         c = getChildGeq(n, c->parentsIndex + 1)) {
       assert(c != nullptr);
       toFree.push_back(c);
     }
@@ -2945,10 +2973,10 @@ void destroyTree(Node *root) {
 void addPointWrite(Node *&root, std::span<const uint8_t> key,
                    InternalVersionT writeVersion, WriteContext *tls,
                    ConflictSet::Impl *impl) {
-  ++tls->point_writes_accum;
+  ++tls->accum.point_writes;
   auto *n = insert<true>(&root, key, writeVersion, tls, impl);
   if (!n->entryPresent) {
-    ++tls->entries_inserted_accum;
+    ++tls->accum.entries_inserted;
     auto *p = nextLogical(n);
 
     addKey(n);
@@ -2975,7 +3003,7 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
       end.back() == 0) {
     return addPointWrite(root, begin, writeVersion, tls, impl);
   }
-  ++tls->range_writes_accum;
+  ++tls->accum.range_writes;
   const bool beginIsPrefix = lcp == int(begin.size());
   auto remaining = begin.subspan(0, lcp);
 
@@ -3019,7 +3047,7 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
   beginNode->entryPresent = true;
 
   if (insertedBegin) {
-    ++tls->entries_inserted_accum;
+    ++tls->accum.entries_inserted;
     auto *p = nextLogical(beginNode);
     beginNode->entry.rangeVersion =
         p == nullptr ? InternalVersionT::zero
@@ -3040,7 +3068,7 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
   endNode->entryPresent = true;
 
   if (insertedEnd) {
-    ++tls->entries_inserted_accum;
+    ++tls->accum.entries_inserted;
     auto *p = nextLogical(endNode);
     endNode->entry.pointVersion =
         p == nullptr ? InternalVersionT::zero
@@ -3072,9 +3100,9 @@ Node *firstGeqPhysical(Node *n, const std::span<const uint8_t> key) {
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
-        n = getChildExists(n, c);
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
+        n = c;
         return n;
       } else {
         n = nextSibling(n);
@@ -3169,25 +3197,29 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 
     assert(writeVersion >= newestVersionFullPrecision);
 
-    if (writeVersion > newestVersionFullPrecision + kNominalVersionWindow) {
-      destroyTree(root);
-      init(writeVersion - kNominalVersionWindow);
+    if (oldestExtantVersion < writeVersion - kMaxCorrectVersionWindow)
+        [[unlikely]] {
+      if (writeVersion > newestVersionFullPrecision + kNominalVersionWindow) {
+        destroyTree(root);
+        init(writeVersion - kNominalVersionWindow);
+      }
+
+      newestVersionFullPrecision = writeVersion;
+      newest_version.set(newestVersionFullPrecision);
+      setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
+      while (oldestExtantVersion <
+             newestVersionFullPrecision - kMaxCorrectVersionWindow) {
+        gcScanStep(1000);
+      }
+    } else {
+      newestVersionFullPrecision = writeVersion;
+      newest_version.set(newestVersionFullPrecision);
+      setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
     }
 
-    newestVersionFullPrecision = writeVersion;
-    newest_version.set(newestVersionFullPrecision);
-    setOldestVersion(
-        std::max(oldestVersionFullPrecision,
-                 newestVersionFullPrecision - kNominalVersionWindow));
-    while (oldestExtantVersion <
-           newestVersionFullPrecision - kMaxCorrectVersionWindow) {
-      gcScanStep(1000);
-    }
-
-    int64_t write_byte_accum = 0;
     for (int i = 0; i < count; ++i) {
       const auto &w = writes[i];
-      write_byte_accum += w.begin.len + w.end.len;
+      tls.accum.write_bytes += w.begin.len + w.end.len;
       auto begin = std::span<const uint8_t>(w.begin.p, w.begin.len);
       auto end = std::span<const uint8_t>(w.end.p, w.end.len);
       if (w.end.len > 0) {
@@ -3201,14 +3233,15 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     }
 
     memory_bytes.set(totalBytes);
-    point_writes_total.add(std::exchange(tls.point_writes_accum, 0));
-    range_writes_total.add(std::exchange(tls.range_writes_accum, 0));
-    nodes_allocated_total.add(std::exchange(tls.nodes_allocated_accum, 0));
-    nodes_released_total.add(std::exchange(tls.nodes_released_accum, 0));
-    entries_inserted_total.add(std::exchange(tls.entries_inserted_accum, 0));
-    entries_erased_total.add(std::exchange(tls.entries_erased_accum, 0));
-    insert_iterations_total.add(std::exchange(tls.insert_iterations_accum, 0));
-    write_bytes_total.add(write_byte_accum);
+    point_writes_total.add(tls.accum.point_writes);
+    range_writes_total.add(tls.accum.range_writes);
+    nodes_allocated_total.add(tls.accum.nodes_allocated);
+    nodes_released_total.add(tls.accum.nodes_released);
+    entries_inserted_total.add(tls.accum.entries_inserted);
+    entries_erased_total.add(tls.accum.entries_erased);
+    insert_iterations_total.add(tls.accum.insert_iterations);
+    write_bytes_total.add(tls.accum.write_bytes);
+    memset(&tls.accum, 0, sizeof(tls.accum));
   }
 
   // Spends up to `fuel` gc'ing, and returns its unused fuel. Reclaims memory
@@ -3283,10 +3316,10 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     keyUpdates = gcScanStep(keyUpdates);
 
     memory_bytes.set(totalBytes);
-    nodes_allocated_total.add(std::exchange(tls.nodes_allocated_accum, 0));
-    nodes_released_total.add(std::exchange(tls.nodes_released_accum, 0));
-    entries_inserted_total.add(std::exchange(tls.entries_inserted_accum, 0));
-    entries_erased_total.add(std::exchange(tls.entries_erased_accum, 0));
+    nodes_allocated_total.add(std::exchange(tls.accum.nodes_allocated, 0));
+    nodes_released_total.add(std::exchange(tls.accum.nodes_released, 0));
+    entries_inserted_total.add(std::exchange(tls.accum.entries_inserted, 0));
+    entries_erased_total.add(std::exchange(tls.accum.entries_erased, 0));
     oldest_version.set(oldestVersionFullPrecision);
   }
 
@@ -3583,29 +3616,27 @@ double internal_getMetricValue(const ConflictSet::MetricsV1 *metric) {
 
 // GCOVR_EXCL_START
 
-Iterator firstGeqLogical(Node *n, const std::span<const uint8_t> key) {
+Node *firstGeqLogical(Node *n, const std::span<const uint8_t> key) {
   auto remaining = key;
   for (;;) {
     if (remaining.size() == 0) {
       if (n->entryPresent) {
-        return {n, 0};
+        return n;
       }
-      int c = getChildGeq(n, 0);
-      assert(c >= 0);
-      n = getChildExists(n, c);
+      n = getFirstChildExists(n);
       goto downLeftSpine;
     }
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
-        n = getChildExists(n, c);
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
+        n = c;
         goto downLeftSpine;
       } else {
         n = nextSibling(n);
         if (n == nullptr) {
-          return {nullptr, 1};
+          return nullptr;
         }
         goto downLeftSpine;
       }
@@ -3637,14 +3668,9 @@ Iterator firstGeqLogical(Node *n, const std::span<const uint8_t> key) {
     }
   }
 downLeftSpine:
-  for (;;) {
-    if (n->entryPresent) {
-      return {n, 1};
-    }
-    int c = getChildGeq(n, 0);
-    assert(c >= 0);
-    n = getChildExists(n, c);
+  for (; !n->entryPresent; n = getFirstChildExists(n)) {
   }
+  return n;
 }
 
 void ConflictSet::check(const ReadRange *reads, Result *results,
@@ -3818,9 +3844,8 @@ std::string getSearchPath(Node *n) {
                 getPartialKeyPrintable(n).c_str(), x, y);
       }
       x += kSeparation;
-      for (int child = getChildGeq(n, 0); child >= 0;
-           child = getChildGeq(n, child + 1)) {
-        auto *c = getChildExists(n, child);
+      for (auto c = getChildGeq(n, 0); c != nullptr;
+           c = getChildGeq(n, c->parentsIndex + 1)) {
         fprintf(file, " k_%p -> k_%p;\n", (void *)n, (void *)c);
         print(c, y - kSeparation);
       }
@@ -3839,19 +3864,19 @@ std::string getSearchPath(Node *n) {
 }
 
 void checkParentPointers(Node *node, bool &success) {
-  for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
-    auto *child = getChildExists(node, i);
+  for (auto child = getChildGeq(node, 0); child != nullptr;
+       child = getChildGeq(node, child->parentsIndex + 1)) {
     if (child->parent != node) {
       fprintf(stderr, "%s child %d has parent pointer %p. Expected %p\n",
-              getSearchPathPrintable(node).c_str(), i, (void *)child->parent,
-              (void *)node);
+              getSearchPathPrintable(node).c_str(), child->parentsIndex,
+              (void *)child->parent, (void *)node);
       success = false;
     }
     checkParentPointers(child, success);
   }
 }
 
-Iterator firstGeq(Node *n, std::string_view key) {
+Node *firstGeq(Node *n, std::string_view key) {
   return firstGeqLogical(
       n, std::span<const uint8_t>((const uint8_t *)key.data(), key.size()));
 }
@@ -3909,8 +3934,8 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
   if (node->entryPresent) {
     expected = std::max(expected, node->entry.pointVersion);
   }
-  for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
-    auto *child = getChildExists(node, i);
+  for (auto child = getChildGeq(node, 0); child != nullptr;
+       child = getChildGeq(node, child->parentsIndex + 1)) {
     expected = std::max(
         expected, checkMaxVersion(root, child, oldestVersion, success, impl));
     if (child->entryPresent) {
@@ -3922,8 +3947,8 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
   auto inc = strinc(key, ok);
   if (ok) {
     auto borrowed = firstGeq(root, inc);
-    if (borrowed.n != nullptr) {
-      expected = std::max(expected, borrowed.n->entry.rangeVersion);
+    if (borrowed != nullptr) {
+      expected = std::max(expected, borrowed->entry.rangeVersion);
     }
   }
   if (maxVersion(node, impl) > oldestVersion &&
@@ -3938,14 +3963,14 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
 
 [[maybe_unused]] int64_t checkEntriesExist(Node *node, bool &success) {
   int64_t total = node->entryPresent;
-  for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
-    auto *child = getChildExists(node, i);
+  for (auto child = getChildGeq(node, 0); child != nullptr;
+       child = getChildGeq(node, child->parentsIndex + 1)) {
     int64_t e = checkEntriesExist(child, success);
     total += e;
     if (e == 0) {
       Arena arena;
       fprintf(stderr, "%s has child %02x with no reachable entries\n",
-              getSearchPathPrintable(node).c_str(), i);
+              getSearchPathPrintable(node).c_str(), child->parentsIndex);
       success = false;
     }
   }
@@ -3982,8 +4007,8 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
     success = false;
   }
   // TODO check that the max capacity property eventually holds
-  for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
-    auto *child = getChildExists(node, i);
+  for (auto child = getChildGeq(node, 0); child != nullptr;
+       child = getChildGeq(node, child->parentsIndex + 1)) {
     checkMemoryBoundInvariants(child, success);
   }
 }