Fix condition checking for erasing the root

Do it in a simpler, more robust/type safe way

CMakeLists.txt

@@ -102,8 +102,6 @@ option(DISABLE_TSAN "Disable TSAN" OFF)
 # https://valgrind.org/docs/manual/manual-core-adv.html#manual-core-adv.clientreq
 include_directories(SYSTEM ${CMAKE_CURRENT_SOURCE_DIR}/third_party/valgrind)
 
-add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Wno-invalid-offsetof>)
-
 if(APPLE)
   add_link_options(-Wl,-dead_strip)
 else()

ConflictSet.cpp

@@ -281,7 +281,6 @@ private:
 
 struct Node {
 
-  /* begin section that's copied to the next node */
   union {
     Entry entry;
     /* Set to the forwarding point for this node if releaseDeferred is set */
@@ -291,10 +290,7 @@ struct Node {
   int32_t partialKeyLen;
   int16_t numChildren;
   bool entryPresent;
-  // Temp variable used to signal the end of the range during addWriteRange
-  bool endOfRange;
   uint8_t parentsIndex;
-  /* end section that's copied to the next node */
 
   /* If set, this node has been replaced and the next node in the forwarding
    * chain is `forwardTo`*/
@@ -324,10 +320,6 @@ Type TaggedNodePointer::getType() {
   return Type(p & uintptr_t(7));
 }
 
-constexpr int kNodeCopyBegin = offsetof(Node, entry);
-constexpr int kNodeCopySize =
-    offsetof(Node, parentsIndex) + sizeof(Node::parentsIndex) - kNodeCopyBegin;
-
 // copyChildrenAndKeyFrom is responsible for copying all
 // public members of Node, copying the partial key, logically copying the
 // children (converting representation if necessary), and updating all the
@@ -433,27 +425,35 @@ struct Node256 : Node {
   size_t size() const { return sizeof(Node256) + getCapacity(); }
 };
 
+template <class To, class From>
+void copyCommon(To &to, const From &from)
+  requires(std::is_base_of_v<Node, To> && std::is_base_of_v<Node, From>)
+{
+  memcpy(&to.entry, &from.entry, sizeof(from.entry));
+  to.parent = from.parent;
+  to.partialKeyLen = from.partialKeyLen;
+  to.numChildren = from.numChildren;
+  to.entryPresent = from.entryPresent;
+  to.parentsIndex = from.parentsIndex;
+}
+
 inline void Node0::copyChildrenAndKeyFrom(const Node0 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   memcpy(partialKey(), &other + 1, partialKeyLen);
 }
 
 inline void Node0::copyChildrenAndKeyFrom(const Node3 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   memcpy(partialKey(), &other + 1, partialKeyLen);
 }
 
 inline void Node3::copyChildrenAndKeyFrom(const Node0 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   memcpy(partialKey(), &other + 1, partialKeyLen);
 }
 
 inline void Node3::copyChildrenAndKeyFrom(const Node3 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   memcpy(index, other.index, kMaxNodes);
   memcpy(children, other.children, kMaxNodes * sizeof(children[0])); // NOLINT
   memcpy(childMaxVersion, other.childMaxVersion,
@@ -466,8 +466,7 @@ inline void Node3::copyChildrenAndKeyFrom(const Node3 &other) {
 }
 
 inline void Node3::copyChildrenAndKeyFrom(const Node16 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   memcpy(index, other.index, kMaxNodes);
   memcpy(children, other.children, kMaxNodes * sizeof(children[0])); // NOLINT
   memcpy(childMaxVersion, other.childMaxVersion,
@@ -480,8 +479,7 @@ inline void Node3::copyChildrenAndKeyFrom(const Node16 &other) {
 }
 
 inline void Node16::copyChildrenAndKeyFrom(const Node3 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   memcpy(index, other.index, Node3::kMaxNodes);
   memcpy(children, other.children,
          Node3::kMaxNodes * sizeof(children[0])); // NOLINT
@@ -496,8 +494,7 @@ inline void Node16::copyChildrenAndKeyFrom(const Node3 &other) {
 }
 
 inline void Node16::copyChildrenAndKeyFrom(const Node16 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   memcpy(index, other.index, sizeof(index));
   for (int i = 0; i < numChildren; ++i) {
     children[i] = other.children[i];
@@ -509,8 +506,7 @@ inline void Node16::copyChildrenAndKeyFrom(const Node16 &other) {
 }
 
 inline void Node16::copyChildrenAndKeyFrom(const Node48 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   int i = 0;
   other.bitSet.forEachSet([&](int c) {
     // Suppress a false positive -Waggressive-loop-optimizations warning
@@ -527,8 +523,7 @@ inline void Node16::copyChildrenAndKeyFrom(const Node48 &other) {
 }
 
 inline void Node48::copyChildrenAndKeyFrom(const Node16 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   assert(numChildren == Node16::kMaxNodes);
   memset(index, -1, sizeof(index));
   memset(children, 0, sizeof(children));
@@ -554,8 +549,7 @@ inline void Node48::copyChildrenAndKeyFrom(const Node16 &other) {
 }
 
 inline void Node48::copyChildrenAndKeyFrom(const Node48 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   bitSet = other.bitSet;
   memcpy(index, other.index, sizeof(index));
   memset(children, 0, sizeof(children));
@@ -575,8 +569,7 @@ inline void Node48::copyChildrenAndKeyFrom(const Node48 &other) {
 }
 
 inline void Node48::copyChildrenAndKeyFrom(const Node256 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   memset(index, -1, sizeof(index));
   memset(children, 0, sizeof(children));
   const auto z = InternalVersionT::zero;
@@ -603,8 +596,7 @@ inline void Node48::copyChildrenAndKeyFrom(const Node256 &other) {
 }
 
 inline void Node256::copyChildrenAndKeyFrom(const Node48 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   bitSet = other.bitSet;
   memset(children, 0, sizeof(children));
   const auto z = InternalVersionT::zero;
@@ -626,8 +618,7 @@ inline void Node256::copyChildrenAndKeyFrom(const Node48 &other) {
 }
 
 inline void Node256::copyChildrenAndKeyFrom(const Node256 &other) {
-  memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
+  copyCommon(*this, other);
-         kNodeCopySize);
   memset(children, 0, sizeof(children));
   const auto z = InternalVersionT::zero;
   for (auto &v : childMaxVersion) {
@@ -699,7 +690,6 @@ template <class T> struct NodeAllocator {
 
   T *allocate(int capacity) {
     T *result = allocate_helper(capacity);
-    result->endOfRange = false;
     result->releaseDeferred = false;
     if constexpr (!std::is_same_v<T, Node0>) {
       const auto z = InternalVersionT::zero;
@@ -793,7 +783,6 @@ struct ReadContext {
   int64_t commits_accum = 0;
   int64_t conflicts_accum = 0;
   int64_t too_olds_accum = 0;
-  ConflictSet::Impl *impl;
   bool operator==(const ReadContext &) const = default; // GCOVR_EXCL_LINE
 };
 
@@ -1117,8 +1106,9 @@ InternalVersionT exchangeMaxVersion(Node *n, InternalVersionT newMax) {
   }
 }
 
-// Precondition `n` is not the root
 void setMaxVersion(Node *n, InternalVersionT newMax) {
+  // This should not be the rootParent
+  assert(n->parent != nullptr);
   assert(newMax >= InternalVersionT::zero);
   int index = n->parentsIndex;
   n = n->parent;
@@ -1161,8 +1151,7 @@ void setMaxVersion(Node *n, InternalVersionT newMax) {
   }
 }
 
-// If impl is nullptr, then n->parent must not be nullptr
+TaggedNodePointer &getInTree(Node *n);
-TaggedNodePointer &getInTree(Node *n, ConflictSet::Impl *impl);
 
 TaggedNodePointer getChild(Node0 *, uint8_t) { return nullptr; }
 TaggedNodePointer getChild(Node3 *self, uint8_t index) {
@@ -1378,13 +1367,11 @@ bool checkRangeVersionOfFirstGeq(Node *node, InternalVersionT readVersion) {
 }
 
 // self must not be the root
-void maybeDecreaseCapacity(Node *&self, WriteContext *writeContext,
+void maybeDecreaseCapacity(Node *&self, WriteContext *writeContext);
-                           ConflictSet::Impl *impl);
 
 void consumePartialKeyFull(TaggedNodePointer &self, TrivialSpan &key,
                            InternalVersionT writeVersion,
-                           WriteContext *writeContext,
+                           WriteContext *writeContext) {
-                           ConflictSet::Impl *impl) {
   // Handle an existing partial key
   int commonLen = std::min<int>(self->partialKeyLen, key.size());
   int partialKeyIndex =
@@ -1422,7 +1409,7 @@ void consumePartialKeyFull(TaggedNodePointer &self, TrivialSpan &key,
                                 partialKeyIndex + 1);
 
     // Maintain memory capacity invariant
-    maybeDecreaseCapacity(old, writeContext, impl);
+    maybeDecreaseCapacity(old, writeContext);
   }
   key = key.subspan(partialKeyIndex, key.size() - partialKeyIndex);
 }
@@ -1431,10 +1418,9 @@ void consumePartialKeyFull(TaggedNodePointer &self, TrivialSpan &key,
 // `key` such that `self` is along the search path of `key`
 inline __attribute__((always_inline)) void
 consumePartialKey(TaggedNodePointer &self, TrivialSpan &key,
-                  InternalVersionT writeVersion, WriteContext *writeContext,
+                  InternalVersionT writeVersion, WriteContext *writeContext) {
-                  ConflictSet::Impl *impl) {
   if (self->partialKeyLen > 0) {
-    consumePartialKeyFull(self, key, writeVersion, writeContext, impl);
+    consumePartialKeyFull(self, key, writeVersion, writeContext);
   }
 }
 
@@ -1444,8 +1430,7 @@ consumePartialKey(TaggedNodePointer &self, TrivialSpan &key,
 // `maxVersion` for result.
 TaggedNodePointer &getOrCreateChild(TaggedNodePointer &self, TrivialSpan &key,
                                     InternalVersionT newMaxVersion,
-                                    WriteContext *writeContext,
+                                    WriteContext *writeContext) {
-                                    ConflictSet::Impl *impl) {
 
   int index = key.front();
   key = key.subspan(1, key.size() - 1);
@@ -1458,8 +1443,7 @@ TaggedNodePointer &getOrCreateChild(TaggedNodePointer &self, TrivialSpan &key,
     auto *self3 = static_cast<Node3 *>(self);
     int i = getNodeIndex(self3, index);
     if (i >= 0) {
-      consumePartialKey(self3->children[i], key, newMaxVersion, writeContext,
+      consumePartialKey(self3->children[i], key, newMaxVersion, writeContext);
-                        impl);
       self3->childMaxVersion[i] = newMaxVersion;
       return self3->children[i];
     }
@@ -1468,8 +1452,7 @@ TaggedNodePointer &getOrCreateChild(TaggedNodePointer &self, TrivialSpan &key,
     auto *self16 = static_cast<Node16 *>(self);
     int i = getNodeIndex(self16, index);
     if (i >= 0) {
-      consumePartialKey(self16->children[i], key, newMaxVersion, writeContext,
+      consumePartialKey(self16->children[i], key, newMaxVersion, writeContext);
-                        impl);
       self16->childMaxVersion[i] = newMaxVersion;
       return self16->children[i];
     }
@@ -1479,7 +1462,7 @@ TaggedNodePointer &getOrCreateChild(TaggedNodePointer &self, TrivialSpan &key,
     int secondIndex = self48->index[index];
     if (secondIndex >= 0) {
       consumePartialKey(self48->children[secondIndex], key, newMaxVersion,
-                        writeContext, impl);
+                        writeContext);
       self48->childMaxVersion[secondIndex] = newMaxVersion;
       self48->maxOfMax[secondIndex >> Node48::kMaxOfMaxShift] =
           std::max(self48->maxOfMax[secondIndex >> Node48::kMaxOfMaxShift],
@@ -1490,7 +1473,7 @@ TaggedNodePointer &getOrCreateChild(TaggedNodePointer &self, TrivialSpan &key,
   case Type_Node256: {
     auto *self256 = static_cast<Node256 *>(self);
     if (auto &result = self256->children[index]; result != nullptr) {
-      consumePartialKey(result, key, newMaxVersion, writeContext, impl);
+      consumePartialKey(result, key, newMaxVersion, writeContext);
       self256->childMaxVersion[index] = newMaxVersion;
       self256->maxOfMax[index >> Node256::kMaxOfMaxShift] = std::max(
           self256->maxOfMax[index >> Node256::kMaxOfMaxShift], newMaxVersion);
@@ -1669,15 +1652,15 @@ downLeftSpine:
   return node;
 }
 
-void freeAndMakeCapacity(Node *&self, int capacity, WriteContext *writeContext,
+void freeAndMakeCapacity(Node *&self, int capacity,
-                         ConflictSet::Impl *impl) {
+                         WriteContext *writeContext) {
   ++writeContext->accum.nodes_resized;
   switch (self->getType()) {
   case Type_Node0: {
     auto *self0 = (Node0 *)self;
     auto *newSelf = writeContext->allocate<Node0>(capacity);
     newSelf->copyChildrenAndKeyFrom(*self0);
-    getInTree(self, impl) = newSelf;
+    getInTree(self) = newSelf;
     writeContext->deferRelease(self0, newSelf);
     self = newSelf;
   } break;
@@ -1685,7 +1668,7 @@ void freeAndMakeCapacity(Node *&self, int capacity, WriteContext *writeContext,
     auto *self3 = (Node3 *)self;
     auto *newSelf = writeContext->allocate<Node3>(capacity);
     newSelf->copyChildrenAndKeyFrom(*self3);
-    getInTree(self, impl) = newSelf;
+    getInTree(self) = newSelf;
     writeContext->deferRelease(self3, newSelf);
     self = newSelf;
   } break;
@@ -1693,7 +1676,7 @@ void freeAndMakeCapacity(Node *&self, int capacity, WriteContext *writeContext,
     auto *self16 = (Node16 *)self;
     auto *newSelf = writeContext->allocate<Node16>(capacity);
     newSelf->copyChildrenAndKeyFrom(*self16);
-    getInTree(self, impl) = newSelf;
+    getInTree(self) = newSelf;
     writeContext->deferRelease(self16, newSelf);
     self = newSelf;
   } break;
@@ -1701,7 +1684,7 @@ void freeAndMakeCapacity(Node *&self, int capacity, WriteContext *writeContext,
     auto *self48 = (Node48 *)self;
     auto *newSelf = writeContext->allocate<Node48>(capacity);
     newSelf->copyChildrenAndKeyFrom(*self48);
-    getInTree(self, impl) = newSelf;
+    getInTree(self) = newSelf;
     writeContext->deferRelease(self48, newSelf);
     self = newSelf;
   } break;
@@ -1709,7 +1692,7 @@ void freeAndMakeCapacity(Node *&self, int capacity, WriteContext *writeContext,
     auto *self256 = (Node256 *)self;
     auto *newSelf = writeContext->allocate<Node256>(capacity);
     newSelf->copyChildrenAndKeyFrom(*self256);
-    getInTree(self, impl) = newSelf;
+    getInTree(self) = newSelf;
     writeContext->deferRelease(self256, newSelf);
     self = newSelf;
   } break;
@@ -1719,8 +1702,7 @@ void freeAndMakeCapacity(Node *&self, int capacity, WriteContext *writeContext,
 }
 
 // Fix larger-than-desired capacities. self must not be the root
-void maybeDecreaseCapacity(Node *&self, WriteContext *writeContext,
+void maybeDecreaseCapacity(Node *&self, WriteContext *writeContext) {
-                           ConflictSet::Impl *impl) {
 
   const int maxCapacity =
       (self->numChildren + int(self->entryPresent)) * (self->partialKeyLen + 1);
@@ -1732,7 +1714,7 @@ void maybeDecreaseCapacity(Node *&self, WriteContext *writeContext,
   if (self->getCapacity() <= maxCapacity) {
     return;
   }
-  freeAndMakeCapacity(self, self->partialKeyLen, writeContext, impl);
+  freeAndMakeCapacity(self, self->partialKeyLen, writeContext);
 }
 
 #if defined(HAS_AVX) && !defined(__SANITIZE_THREAD__)
@@ -1802,13 +1784,13 @@ void rezero(Node *n, InternalVersionT z) {
 #endif
 
 void mergeWithChild(TaggedNodePointer &self, WriteContext *writeContext,
-                    Node3 *self3, ConflictSet::Impl *impl) {
+                    Node3 *self3) {
   assert(!self3->entryPresent);
   Node *child = self3->children[0];
   const int minCapacity = self3->partialKeyLen + 1 + child->partialKeyLen;
 
   if (minCapacity > child->getCapacity()) {
-    freeAndMakeCapacity(child, minCapacity, writeContext, impl);
+    freeAndMakeCapacity(child, minCapacity, writeContext);
   }
 
   // Merge partial key with child
@@ -1829,9 +1811,6 @@ void mergeWithChild(TaggedNodePointer &self, WriteContext *writeContext,
   child->parent = self->parent;
   child->parentsIndex = self->parentsIndex;
 
-  // Max versions are stored in the parent, so we need to update it now
-  // that we have a new parent. Safe to call since the root never has a partial
-  // key.
   setMaxVersion(child, std::max(childMaxVersion, writeContext->zero));
 
   self = child;
@@ -1844,63 +1823,59 @@ bool needsDownsize(Node *n) {
   return n->numChildren + n->entryPresent < minTable[n->getType()];
 }
 
-void downsize(Node3 *self, WriteContext *writeContext,
+void downsize(Node3 *self, WriteContext *writeContext) {
-              ConflictSet::Impl *impl) {
   if (self->numChildren == 0) {
     auto *newSelf = writeContext->allocate<Node0>(self->partialKeyLen);
     newSelf->copyChildrenAndKeyFrom(*self);
-    getInTree(self, impl) = newSelf;
+    getInTree(self) = newSelf;
     writeContext->deferRelease(self, newSelf);
   } else {
     assert(self->numChildren == 1 && !self->entryPresent);
-    mergeWithChild(getInTree(self, impl), writeContext, self, impl);
+    mergeWithChild(getInTree(self), writeContext, self);
   }
 }
 
-void downsize(Node16 *self, WriteContext *writeContext,
+void downsize(Node16 *self, WriteContext *writeContext) {
-              ConflictSet::Impl *impl) {
   assert(self->numChildren + int(self->entryPresent) < kMinChildrenNode16);
   auto *newSelf = writeContext->allocate<Node3>(self->partialKeyLen);
   newSelf->copyChildrenAndKeyFrom(*self);
-  getInTree(self, impl) = newSelf;
+  getInTree(self) = newSelf;
   writeContext->deferRelease(self, newSelf);
 }
 
-void downsize(Node48 *self, WriteContext *writeContext,
+void downsize(Node48 *self, WriteContext *writeContext) {
-              ConflictSet::Impl *impl) {
   assert(self->numChildren + int(self->entryPresent) < kMinChildrenNode48);
   auto *newSelf = writeContext->allocate<Node16>(self->partialKeyLen);
   newSelf->copyChildrenAndKeyFrom(*self);
-  getInTree(self, impl) = newSelf;
+  getInTree(self) = newSelf;
   writeContext->deferRelease(self, newSelf);
 }
 
-void downsize(Node256 *self, WriteContext *writeContext,
+void downsize(Node256 *self, WriteContext *writeContext) {
-              ConflictSet::Impl *impl) {
   assert(self->numChildren + int(self->entryPresent) < kMinChildrenNode256);
   auto *self256 = (Node256 *)self;
   auto *newSelf = writeContext->allocate<Node48>(self->partialKeyLen);
   newSelf->copyChildrenAndKeyFrom(*self256);
-  getInTree(self, impl) = newSelf;
+  getInTree(self) = newSelf;
   writeContext->deferRelease(self256, newSelf);
 }
 
-void downsize(Node *self, WriteContext *writeContext, ConflictSet::Impl *impl) {
+void downsize(Node *self, WriteContext *writeContext) {
 
   switch (self->getType()) {
   case Type_Node0:           // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   case Type_Node3:
-    downsize(static_cast<Node3 *>(self), writeContext, impl);
+    downsize(static_cast<Node3 *>(self), writeContext);
     break;
   case Type_Node16:
-    downsize(static_cast<Node16 *>(self), writeContext, impl);
+    downsize(static_cast<Node16 *>(self), writeContext);
     break;
   case Type_Node48:
-    downsize(static_cast<Node48 *>(self), writeContext, impl);
+    downsize(static_cast<Node48 *>(self), writeContext);
     break;
   case Type_Node256:
-    downsize(static_cast<Node256 *>(self), writeContext, impl);
+    downsize(static_cast<Node256 *>(self), writeContext);
     break;
   default:                   // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
@@ -1910,11 +1885,9 @@ void downsize(Node *self, WriteContext *writeContext, ConflictSet::Impl *impl) {
 // Precondition: self is not the root. May invalidate nodes along the search
 // path to self. May invalidate children of self->parent. Returns a pointer to
 // the node after self. Precondition: `self->entryPresent`
-
+Node *erase(Node *self, WriteContext *writeContext, bool logical) {
-Node *erase(Node *self, WriteContext *writeContext, ConflictSet::Impl *impl,
-            bool logical) {
   ++writeContext->accum.entries_erased;
-  assert(self->parent != nullptr);
+  assert(self->parent->parent != nullptr);
 
 #if DEBUG_VERBOSE && !defined(NDEBUG)
   fprintf(stderr, "Erase: %s\n", getSearchPathPrintable(self).c_str());
@@ -1931,12 +1904,12 @@ Node *erase(Node *self, WriteContext *writeContext, ConflictSet::Impl *impl,
 
   if (self->numChildren != 0) {
     if (needsDownsize(self)) {
-      downsize(self, writeContext, impl);
+      downsize(self, writeContext);
     }
     while (self->releaseDeferred) {
       self = self->forwardTo;
     }
-    maybeDecreaseCapacity(self, writeContext, impl);
+    maybeDecreaseCapacity(self, writeContext);
     if (result != nullptr) {
       while (result->releaseDeferred) {
         result = result->forwardTo;
@@ -1967,7 +1940,7 @@ Node *erase(Node *self, WriteContext *writeContext, ConflictSet::Impl *impl,
                                 sizeof(parent3->children[0]));
 
     if (needsDownsize(parent3)) {
-      downsize(parent3, writeContext, impl);
+      downsize(parent3, writeContext);
     }
   } break;
   case Type_Node16: {
@@ -1986,7 +1959,7 @@ Node *erase(Node *self, WriteContext *writeContext, ConflictSet::Impl *impl,
                                 sizeof(parent16->children[0]));
 
     if (needsDownsize(parent16)) {
-      downsize(parent16, writeContext, impl);
+      downsize(parent16, writeContext);
     }
   } break;
   case Type_Node48: {
@@ -2016,7 +1989,7 @@ Node *erase(Node *self, WriteContext *writeContext, ConflictSet::Impl *impl,
                                 sizeof(parent48->children[0]));
 
     if (needsDownsize(parent48)) {
-      downsize(parent48, writeContext, impl);
+      downsize(parent48, writeContext);
     }
   } break;
   case Type_Node256: {
@@ -2027,7 +2000,7 @@ Node *erase(Node *self, WriteContext *writeContext, ConflictSet::Impl *impl,
     --parent->numChildren;
 
     if (needsDownsize(parent256)) {
-      downsize(parent256, writeContext, impl);
+      downsize(parent256, writeContext);
     }
   } break;
   default:                   // GCOVR_EXCL_LINE
@@ -2037,7 +2010,7 @@ Node *erase(Node *self, WriteContext *writeContext, ConflictSet::Impl *impl,
   while (parent->releaseDeferred) {
     parent = parent->forwardTo;
   }
-  maybeDecreaseCapacity(parent, writeContext, impl);
+  maybeDecreaseCapacity(parent, writeContext);
 
   if (result != nullptr) {
     while (result->releaseDeferred) {
@@ -2634,11 +2607,13 @@ TrivialSpan getSearchPath(Arena &arena, Node *n) {
     for (int i = n->partialKeyLen - 1; i >= 0; --i) {
       result.push_back(n->partialKey()[i]);
     }
-    if (n->parent == nullptr) {
-      break;
-    }
     result.push_back(n->parentsIndex);
     n = n->parent;
+    if (n->parent == nullptr) {
+      // Implicit byte from rootParent
+      result.pop_back();
+      break;
+    }
   }
   std::reverse(result.begin(), result.end());
   return {result.begin(), result.size()};
@@ -2738,12 +2713,13 @@ checkMaxBetweenExclusiveImpl<true>(Node256 *n, int begin, int end,
 // of the result will have `maxVersion` set to `writeVersion` as a
 // postcondition. Nodes along the search path may be invalidated. Callers must
 // ensure that the max version of the self argument is updated.
-[[nodiscard]] TaggedNodePointer *
+[[nodiscard]] TaggedNodePointer *insert(TaggedNodePointer *self,
-insert(TaggedNodePointer *self, TrivialSpan key, InternalVersionT writeVersion,
+                                        TrivialSpan key,
-       WriteContext *writeContext, ConflictSet::Impl *impl) {
+                                        InternalVersionT writeVersion,
+                                        WriteContext *writeContext) {
 
   for (; key.size() != 0; ++writeContext->accum.insert_iterations) {
-    self = &getOrCreateChild(*self, key, writeVersion, writeContext, impl);
+    self = &getOrCreateChild(*self, key, writeVersion, writeContext);
   }
   return self;
 }
@@ -2801,10 +2777,9 @@ void eraseTree(Node *root, WriteContext *writeContext) {
 }
 
 void addPointWrite(TaggedNodePointer &root, TrivialSpan key,
-                   InternalVersionT writeVersion, WriteContext *writeContext,
+                   InternalVersionT writeVersion, WriteContext *writeContext) {
-                   ConflictSet::Impl *impl) {
   ++writeContext->accum.point_writes;
-  auto n = *insert(&root, key, writeVersion, writeContext, impl);
+  auto n = *insert(&root, key, writeVersion, writeContext);
   if (!n->entryPresent) {
     ++writeContext->accum.entries_inserted;
     auto *p = nextLogical(n);
@@ -2933,13 +2908,12 @@ struct AddedWriteRange {
 
 AddedWriteRange addWriteRange(Node *beginRoot, TrivialSpan begin, Node *endRoot,
                               TrivialSpan end, InternalVersionT writeVersion,
-                              WriteContext *writeContext,
+                              WriteContext *writeContext) {
-                              ConflictSet::Impl *impl) {
 
   ++writeContext->accum.range_writes;
 
-  Node *beginNode = *insert(&getInTree(beginRoot, impl), begin, writeVersion,
+  Node *beginNode =
-                            writeContext, impl);
+      *insert(&getInTree(beginRoot), begin, writeVersion, writeContext);
   addKey(beginNode);
   if (!beginNode->entryPresent) {
     ++writeContext->accum.entries_inserted;
@@ -2954,8 +2928,7 @@ AddedWriteRange addWriteRange(Node *beginRoot, TrivialSpan begin, Node *endRoot,
   }
   beginNode->entry.pointVersion = writeVersion;
 
-  Node *endNode =
+  Node *endNode = *insert(&getInTree(endRoot), end, writeVersion, writeContext);
-      *insert(&getInTree(endRoot, impl), end, writeVersion, writeContext, impl);
 
   addKey(endNode);
   if (!endNode->entryPresent) {
@@ -2974,20 +2947,20 @@ AddedWriteRange addWriteRange(Node *beginRoot, TrivialSpan begin, Node *endRoot,
   return {beginNode, endNode};
 }
 
-void eraseInRange(Node *beginNode, Node *endNode, WriteContext *writeContext,
+void eraseInRange(Node *beginNode, Node *endNode, WriteContext *writeContext) {
-                  ConflictSet::Impl *impl) {
+  auto checkNodesEq = [](Node *lhs, Node *&rhs) {
+    assert(!lhs->releaseDeferred);
+    while (rhs->releaseDeferred) [[unlikely]] {
+      rhs = rhs->forwardTo;
+    }
+    return lhs == rhs;
+  };
+
   // Erase nodes in range
-  assert(!beginNode->endOfRange);
+  Node *iter = nextLogical(beginNode);
-  assert(!endNode->endOfRange);
+  for (; !checkNodesEq(iter, endNode);
-  endNode->endOfRange = true;
+       iter = erase(iter, writeContext, /*logical*/ true)) {
-  Node *iter = beginNode;
-  for (iter = nextLogical(iter); !iter->endOfRange;
-       iter = erase(iter, writeContext, impl, /*logical*/ true)) {
-    assert(!iter->endOfRange);
-    assert(!iter->releaseDeferred);
   }
-  assert(iter->endOfRange);
-  iter->endOfRange = false;
 
   // Inserting end trashed the last node's maxVersion. Fix that. Safe to call
   // since the end key always has non-zero size.
@@ -2995,22 +2968,21 @@ void eraseInRange(Node *beginNode, Node *endNode, WriteContext *writeContext,
 }
 
 void addWriteRange(TaggedNodePointer &root, TrivialSpan begin, TrivialSpan end,
-                   InternalVersionT writeVersion, WriteContext *writeContext,
+                   InternalVersionT writeVersion, WriteContext *writeContext) {
-                   ConflictSet::Impl *impl) {
   int lcp = longestCommonPrefix(begin.data(), end.data(),
                                 std::min(begin.size(), end.size()));
   if (lcp == begin.size() && end.size() == begin.size() + 1 &&
       end.back() == 0) {
-    return addPointWrite(root, begin, writeVersion, writeContext, impl);
+    return addPointWrite(root, begin, writeVersion, writeContext);
   }
   auto useAsRoot =
-      insert(&root, begin.subspan(0, lcp), writeVersion, writeContext, impl);
+      insert(&root, begin.subspan(0, lcp), writeVersion, writeContext);
 
   auto [beginNode, endNode] = addWriteRange(
       *useAsRoot, begin.subspan(lcp, begin.size() - lcp), *useAsRoot,
-      end.subspan(lcp, end.size() - lcp), writeVersion, writeContext, impl);
+      end.subspan(lcp, end.size() - lcp), writeVersion, writeContext);
 
-  eraseInRange(beginNode, endNode, writeContext, impl);
+  eraseInRange(beginNode, endNode, writeContext);
 }
 
 Node *firstGeqPhysical(Node *n, const TrivialSpan key) {
@@ -4782,10 +4754,6 @@ bool checkRangeRead(Node *n, TrivialSpan begin, TrivialSpan end,
     return checkRangeRightSide(n, end, lcp, readVersion, readContext);
   }
 
-  // This makes it safe to check maxVersion within checkRangeLeftSide. If this
-  // were false, then we would have returned above since lcp == begin.size().
-  assert(!(n->parent == nullptr && begin.size() == 0));
-
   return checkRangeStartsWith(n, begin.subspan(0, lcp), begin[lcp], end[lcp],
                               readVersion, readContext) &&
          checkRangeLeftSide(n, begin, lcp + 1, readVersion, readContext) &&
@@ -4805,14 +4773,16 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
       } else {
         bool ok;
         if (reads[i].end.len == 0) {
-          ok = checkPointRead(
+          ok = checkPointRead(rootParent->children[0],
-              root, TrivialSpan(reads[i].begin.p, reads[i].begin.len),
+                              TrivialSpan(reads[i].begin.p, reads[i].begin.len),
-              InternalVersionT(reads[i].readVersion), &context.readContext);
+                              InternalVersionT(reads[i].readVersion),
+                              &context.readContext);
         } else {
-          ok = checkRangeRead(
+          ok = checkRangeRead(rootParent->children[0],
-              root, TrivialSpan(reads[i].begin.p, reads[i].begin.len),
+                              TrivialSpan(reads[i].begin.p, reads[i].begin.len),
-              TrivialSpan(reads[i].end.p, reads[i].end.len),
+                              TrivialSpan(reads[i].end.p, reads[i].end.len),
-              InternalVersionT(reads[i].readVersion), &context.readContext);
+                              InternalVersionT(reads[i].readVersion),
+                              &context.readContext);
         }
         result[i] = ok ? Commit : Conflict;
       }
@@ -4829,7 +4799,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 
     int64_t check_byte_accum = 0;
     check::Context context;
-    context.readContext.impl = this;
 
     if constexpr (kEnableInterleaved) {
       if (count == 1) {
@@ -4839,13 +4808,13 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         check::Job inProgress[kConcurrent];
         context.count = count;
         context.oldestVersionFullPrecision = oldestVersionFullPrecision;
-        context.root = root;
+        context.root = rootParent->children[0];
         context.queries = reads;
         context.results = result;
         int64_t started = std::min(kConcurrent, count);
         context.started = started;
         for (int i = 0; i < started; i++) {
-          inProgress[i].init(reads + i, result + i, root,
+          inProgress[i].init(reads + i, result + i, rootParent->children[0],
                              oldestVersionFullPrecision);
         }
         for (int i = 0; i < started - 1; i++) {
@@ -4865,7 +4834,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         Arena arena;
         auto *results2 = new (arena) Result[count];
         check::Context context2;
-        context2.readContext.impl = this;
         useSequential(reads, results2, count, context2);
         assert(memcmp(result, results2, count) == 0);
         assert(context.readContext == context2.readContext);
@@ -4923,7 +4891,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     interleaved_insert::Context context;
     context.writeVersion = writeVersion;
     context.count = count;
-    context.root = root;
+    context.root = rootParent->children[0];
     context.writes = writes;
     context.results = stackResults;
     if (count > kStackResultMax) [[unlikely]] {
@@ -4985,9 +4953,9 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
             context.results[i].insertionPoint->forwardTo;
       }
       if (context.results[i].endInsertionPoint == nullptr) {
-        addPointWrite(getInTree(context.results[i].insertionPoint, this),
+        addPointWrite(getInTree(context.results[i].insertionPoint),
-                      context.results[i].remaining, writeVersion, &writeContext,
+                      context.results[i].remaining, writeVersion,
-                      this);
+                      &writeContext);
       } else {
         if (firstRangeWrite == nullptr) {
           firstRangeWrite = context.results + i;
@@ -5002,7 +4970,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         auto [beginNode, endNode] = addWriteRange(
             context.results[i].insertionPoint, context.results[i].remaining,
             context.results[i].endInsertionPoint,
-            context.results[i].endRemaining, writeVersion, &writeContext, this);
+            context.results[i].endRemaining, writeVersion, &writeContext);
         context.results[i].insertionPoint = beginNode;
         context.results[i].endInsertionPoint = endNode;
       }
@@ -5024,7 +4992,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
           iter->endInsertionPoint = iter->endInsertionPoint->forwardTo;
         }
         eraseInRange(iter->insertionPoint, iter->endInsertionPoint,
-                     &writeContext, this);
+                     &writeContext);
       }
     }
 
@@ -5055,11 +5023,11 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         auto begin = TrivialSpan(w.begin.p, w.begin.len);
         auto end = TrivialSpan(w.end.p, w.end.len);
         if (w.end.len > 0) {
-          addWriteRange(root, begin, end, InternalVersionT(writeVersion),
+          addWriteRange(rootParent->children[0], begin, end,
-                        &writeContext, this);
+                        InternalVersionT(writeVersion), &writeContext);
         } else {
-          addPointWrite(root, begin, InternalVersionT(writeVersion),
+          addPointWrite(rootParent->children[0], begin,
-                        &writeContext, this);
+                        InternalVersionT(writeVersion), &writeContext);
         }
       }
     }
@@ -5080,7 +5048,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     if (oldestExtantVersion < writeVersion - kMaxCorrectVersionWindow)
         [[unlikely]] {
       if (writeVersion > newestVersionFullPrecision + kNominalVersionWindow) {
-        eraseTree(root, &writeContext);
+        eraseTree(rootParent->children[0], &writeContext);
         init(writeVersion - kNominalVersionWindow);
       }
 
@@ -5108,6 +5076,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     }
 
     if (count > 0) {
+      rootParent->childMaxVersion[0] = InternalVersionT(writeVersion);
       int firstNotInserted = 0;
       bool batchHasOnlyPointWrites = writes[0].end.len == 0;
       bool batchIsSorted = true;
@@ -5155,11 +5124,14 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   // Spends up to `fuel` gc'ing, and returns its unused fuel. Reclaims memory
   // and updates oldestExtantVersion after spending enough fuel.
   int64_t gcScanStep(int64_t fuel) {
-    Node *n = firstGeqPhysical(root, removalKey);
+    Node *n = firstGeqPhysical(rootParent->children[0], removalKey);
     // There's no way to erase removalKey without introducing a key after it
     assert(n != nullptr);
     // Don't erase the root
-    if (n == root) {
+    if (n == rootParent->children[0]) {
+      for (auto &v : rootParent->childMaxVersion) {
+        v = std::max(v, oldestVersion);
+      }
       rezero(n, oldestVersion);
       n = nextPhysical(n);
     }
@@ -5178,7 +5150,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         // There's no way to insert a range such that range version of the
         // right node is greater than the point version of the left node
         assert(n->entry.rangeVersion <= oldestVersion);
-        n = erase(n, &writeContext, this, /*logical*/ false);
+        n = erase(n, &writeContext, /*logical*/ false);
       } else {
         n = nextPhysical(n);
       }
@@ -5229,7 +5201,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
       cursor -= partialKey.size();
       memcpy(cursor, partialKey.data(), partialKey.size());
 
-      if (n->parent == nullptr) {
+      if (n->parent == rootParent) {
         break;
       }
 
@@ -5299,17 +5271,28 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     keyUpdates = 10;
 
     // Insert ""
-    root = writeContext.allocate<Node0>(0);
+    rootParent->children[0] = writeContext.allocate<Node0>(0);
-    root->numChildren = 0;
+    rootParent->children[0]->numChildren = 0;
-    root->parent = nullptr;
+    rootParent->children[0]->parent = rootParent;
-    root->entryPresent = false;
+    rootParent->children[0]->parentsIndex = 0;
-    root->partialKeyLen = 0;
+    rootParent->children[0]->entryPresent = false;
+    rootParent->children[0]->partialKeyLen = 0;
+    rootParent->entryPresent = false;
+    rootParent->numChildren = 1;
+    rootParent->parent = nullptr;
+    rootParent->parentsIndex = 0;
+    for (auto &v : rootParent->childMaxVersion) {
+      v = this->oldestVersion;
+    }
+    rootParent->index[0] = 0;
+    rootParent->partialKeyLen = 0;
+    rootParent->releaseDeferred = false;
 
-    addKey(root);
+    addKey(rootParent->children[0]);
 
-    root->entryPresent = true;
+    rootParent->children[0]->entryPresent = true;
-    root->entry.pointVersion = this->oldestVersion;
+    rootParent->children[0]->entry.pointVersion = this->oldestVersion;
-    root->entry.rangeVersion = this->oldestVersion;
+    rootParent->children[0]->entry.rangeVersion = this->oldestVersion;
 
 #if !USE_64_BIT
     InternalVersionT::zero = writeContext.zero = this->oldestVersion;
@@ -5320,13 +5303,15 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 
   explicit Impl(int64_t oldestVersion) {
     assert(oldestVersion >= 0);
+    rootParent = writeContext.allocate<Node3>(0);
     init(oldestVersion);
     metrics = initMetrics(metricsList, metricsCount);
   }
   ~Impl() {
-    eraseTree(root, &writeContext);
+    eraseTree(rootParent->children[0], &writeContext);
     safe_free(metrics, metricsCount * sizeof(metrics[0]));
     safe_free(removalBuffer, removalBufferSize);
+    writeContext.release(rootParent);
   }
 
   WriteContext writeContext;
@@ -5339,7 +5324,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   TrivialSpan removalKey;
   int64_t keyUpdates;
 
-  TaggedNodePointer root;
+  Node3 *rootParent;
   InternalVersionT oldestVersion;
   int64_t oldestVersionFullPrecision;
   int64_t oldestExtantVersion;
@@ -5421,9 +5406,8 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   }
 };
 
-TaggedNodePointer &getInTree(Node *n, ConflictSet::Impl *impl) {
+TaggedNodePointer &getInTree(Node *n) {
-  return n->parent == nullptr ? impl->root
+  return getChildExists(n->parent, n->parentsIndex);
-                              : getChildExists(n->parent, n->parentsIndex);
 }
 
 // Internal entry points. Public entry points should just delegate to these
@@ -5636,11 +5620,13 @@ std::string getSearchPathPrintable(Node *n) {
     for (int i = n->partialKeyLen - 1; i >= 0; --i) {
       result.push_back(n->partialKey()[i]);
     }
-    if (n->parent == nullptr) {
-      break;
-    }
     result.push_back(n->parentsIndex);
     n = n->parent;
+    if (n->parent == nullptr) {
+      // Implicit byte from rootParent
+      result.pop_back();
+      break;
+    }
   }
   std::reverse(result.begin(), result.end());
   if (result.size() > 0) {
@@ -5688,15 +5674,13 @@ std::string getSearchPath(Node *n) {
   return std::string((const char *)result.data(), result.size());
 }
 
-[[maybe_unused]] void debugPrintDot(FILE *file, Node *node,
+[[maybe_unused]] void debugPrintDot(FILE *file, Node *node) {
-                                    ConflictSet::Impl *impl) {
 
   constexpr int kSeparation = 3;
 
   struct DebugDotPrinter {
 
-    explicit DebugDotPrinter(FILE *file, ConflictSet::Impl *impl)
+    explicit DebugDotPrinter(FILE *file) : file(file) {}
-        : file(file), impl(impl) {}
 
     void print(Node *n, int y = 0) {
       assert(n != nullptr);
@@ -5722,13 +5706,12 @@ std::string getSearchPath(Node *n) {
     }
     int x = 0;
     FILE *file;
-    ConflictSet::Impl *impl;
   };
 
   fprintf(file, "digraph ConflictSet {\n");
   fprintf(file, " node [shape = box];\n");
   assert(node != nullptr);
-  DebugDotPrinter printer{file, impl};
+  DebugDotPrinter printer{file};
   printer.print(node);
   fprintf(file, "}\n");
 }
@@ -6116,7 +6099,7 @@ void printTree() {
   write.begin = "art"_s;
   write.end = ""_s;
   cs.addWrites(&write, 1, ++writeVersion);
-  debugPrintDot(stdout, cs.root, &cs);
+  debugPrintDot(stdout, cs.rootParent->children[0]);
 }
 
 int main(void) { benchHorizontal16(); }
@@ -6134,13 +6117,13 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
     if (!done1) {
       done1 = driver1.next();
       if (!driver1.ok) {
-        debugPrintDot(stdout, driver1.cs.root, &driver1.cs);
+        debugPrintDot(stdout, driver1.cs.rootParent->children[0]);
         fflush(stdout);
         abort();
       }
-      if (!checkCorrectness(driver1.cs.root, driver1.cs.oldestVersion,
+      if (!checkCorrectness(driver1.cs.rootParent->children[0],
-                            &driver1.cs)) {
+                            driver1.cs.oldestVersion, &driver1.cs)) {
-        debugPrintDot(stdout, driver1.cs.root, &driver1.cs);
+        debugPrintDot(stdout, driver1.cs.rootParent->children[0]);
         fflush(stdout);
         abort();
       }
@@ -6148,13 +6131,13 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
     if (!done2) {
       done2 = driver2.next();
       if (!driver2.ok) {
-        debugPrintDot(stdout, driver2.cs.root, &driver2.cs);
+        debugPrintDot(stdout, driver2.cs.rootParent->children[0]);
         fflush(stdout);
         abort();
       }
-      if (!checkCorrectness(driver2.cs.root, driver2.cs.oldestVersion,
+      if (!checkCorrectness(driver2.cs.rootParent->children[0],
-                            &driver2.cs)) {
+                            driver2.cs.oldestVersion, &driver2.cs)) {
-        debugPrintDot(stdout, driver2.cs.root, &driver2.cs);
+        debugPrintDot(stdout, driver2.cs.rootParent->children[0]);
         fflush(stdout);
         abort();
       }