Avoid branching on type twice in erase

Use a new node member "endOfRange" instead

ConflictSet.cpp

@@ -173,7 +173,7 @@ int BitSet::firstSetGeq(int i) const {
   return -1;
 }
 
-enum Type {
+enum Type : int8_t {
   Type_Node0,
   Type_Node3,
   Type_Node16,
@@ -191,11 +191,12 @@ 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 */
 
   uint8_t *partialKey();
-  size_t size() const;
 
   Type getType() const { return type; }
   int32_t getCapacity() const { return partialKeyCapacity; }
@@ -626,6 +627,7 @@ template <class T> struct BoundedFreeListAllocator {
 
   T *allocate(int partialKeyCapacity) {
     T *result = allocate_helper(partialKeyCapacity);
+    result->endOfRange = false;
     if constexpr (!std::is_same_v<T, Node0>) {
       memset(result->children, 0, sizeof(result->children));
       const auto z = InternalVersionT::zero;
@@ -693,23 +695,6 @@ uint8_t *Node::partialKey() {
   }
 }
 
-size_t Node::size() const {
-  switch (type) {
-  case Type_Node0:
-    return ((Node0 *)this)->size();
-  case Type_Node3:
-    return ((Node3 *)this)->size();
-  case Type_Node16:
-    return ((Node16 *)this)->size();
-  case Type_Node48:
-    return ((Node48 *)this)->size();
-  case Type_Node256:
-    return ((Node256 *)this)->size();
-  default:                   // GCOVR_EXCL_LINE
-    __builtin_unreachable(); // GCOVR_EXCL_LINE
-  }
-}
-
 // A type that's plumbed along the check call tree. Lifetime ends after each
 // check call.
 struct ReadContext {
@@ -905,10 +890,115 @@ Node *&getChildExists(Node *self, uint8_t index) {
   }
 }
 
-InternalVersionT maxVersion(Node *n, ConflictSet::Impl *);
+// Precondition `n` is not the root
-InternalVersionT exchangeMaxVersion(Node *n, InternalVersionT newMax);
+InternalVersionT maxVersion(Node *n) {
+  int index = n->parentsIndex;
+  n = n->parent;
+  assert(n != nullptr);
+  switch (n->getType()) {
+  case Type_Node0:           // GCOVR_EXCL_LINE
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  case Type_Node3: {
+    auto *n3 = static_cast<Node3 *>(n);
+    int i = getNodeIndex(n3, index);
+    return n3->childMaxVersion[i];
+  }
+  case Type_Node16: {
+    auto *n16 = static_cast<Node16 *>(n);
+    int i = getNodeIndex(n16, index);
+    return n16->childMaxVersion[i];
+  }
+  case Type_Node48: {
+    auto *n48 = static_cast<Node48 *>(n);
+    assert(n48->bitSet.test(index));
+    return n48->childMaxVersion[n48->index[index]];
+  }
+  case Type_Node256: {
+    auto *n256 = static_cast<Node256 *>(n);
+    assert(n256->bitSet.test(index));
+    return n256->childMaxVersion[index];
+  }
+  default:                   // GCOVR_EXCL_LINE
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  }
+}
 
-void setMaxVersion(Node *n, ConflictSet::Impl *, InternalVersionT maxVersion);
+// Precondition `n` is not the root
+InternalVersionT exchangeMaxVersion(Node *n, InternalVersionT newMax) {
+  int index = n->parentsIndex;
+  n = n->parent;
+  assert(n != nullptr);
+  switch (n->getType()) {
+  case Type_Node0:           // GCOVR_EXCL_LINE
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  case Type_Node3: {
+    auto *n3 = static_cast<Node3 *>(n);
+    int i = getNodeIndex(n3, index);
+    return std::exchange(n3->childMaxVersion[i], newMax);
+  }
+  case Type_Node16: {
+    auto *n16 = static_cast<Node16 *>(n);
+    int i = getNodeIndex(n16, index);
+    return std::exchange(n16->childMaxVersion[i], newMax);
+  }
+  case Type_Node48: {
+    auto *n48 = static_cast<Node48 *>(n);
+    assert(n48->bitSet.test(index));
+    return std::exchange(n48->childMaxVersion[n48->index[index]], newMax);
+  }
+  case Type_Node256: {
+    auto *n256 = static_cast<Node256 *>(n);
+    assert(n256->bitSet.test(index));
+    return std::exchange(n256->childMaxVersion[index], newMax);
+  }
+  default:                   // GCOVR_EXCL_LINE
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  }
+}
+
+// Precondition `n` is not the root
+void setMaxVersion(Node *n, InternalVersionT newMax) {
+  assert(newMax >= InternalVersionT::zero);
+  int index = n->parentsIndex;
+  n = n->parent;
+  assert(n != nullptr);
+  switch (n->getType()) {
+  case Type_Node0:           // GCOVR_EXCL_LINE
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  case Type_Node3: {
+    auto *n3 = static_cast<Node3 *>(n);
+    int i = getNodeIndex(n3, index);
+    n3->childMaxVersion[i] = newMax;
+    return;
+  }
+  case Type_Node16: {
+    auto *n16 = static_cast<Node16 *>(n);
+    int i = getNodeIndex(n16, index);
+    n16->childMaxVersion[i] = newMax;
+    return;
+  }
+  case Type_Node48: {
+    auto *n48 = static_cast<Node48 *>(n);
+    assert(n48->bitSet.test(index));
+    int i = n48->index[index];
+    n48->childMaxVersion[i] = newMax;
+    n48->maxOfMax[i >> Node48::kMaxOfMaxShift] = std::max<InternalVersionT>(
+        n48->maxOfMax[i >> Node48::kMaxOfMaxShift], newMax);
+    return;
+  }
+  case Type_Node256: {
+    auto *n256 = static_cast<Node256 *>(n);
+    assert(n256->bitSet.test(index));
+    n256->childMaxVersion[index] = newMax;
+    n256->maxOfMax[index >> Node256::kMaxOfMaxShift] =
+        std::max<InternalVersionT>(
+            n256->maxOfMax[index >> Node256::kMaxOfMaxShift], newMax);
+    return;
+  }
+  default:                   // GCOVR_EXCL_LINE
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  }
+}
 
 Node *&getInTree(Node *n, ConflictSet::Impl *);
 
@@ -1544,91 +1634,108 @@ void rezero(Node *n, InternalVersionT z) {
   }
 }
 
-void maybeDownsize(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
+void mergeWithChild(Node *&self, WriteContext *tls, ConflictSet::Impl *impl,
-                   Node *&dontInvalidate) {
+                    Node *&dontInvalidate, Node3 *self3) {
+  assert(!self3->entryPresent);
+  auto *child = self3->children[0];
+  int minCapacity = self3->partialKeyLen + 1 + child->partialKeyLen;
 
+  if (minCapacity > child->getCapacity()) {
+    const bool update = child == dontInvalidate;
+    freeAndMakeCapacityAtLeast(child, minCapacity, tls, impl, true);
+    if (update) {
+      dontInvalidate = child;
+    }
+  }
+
+  // Merge partial key with child
 #if DEBUG_VERBOSE && !defined(NDEBUG)
-  fprintf(stderr, "maybeDownsize: %s\n", getSearchPathPrintable(self).c_str());
+  fprintf(stderr, "Merge %s into %s\n", getSearchPathPrintable(self).c_str(),
+          getSearchPathPrintable(child).c_str());
 #endif
 
+  InternalVersionT childMaxVersion = self3->childMaxVersion[0];
+
+  // Construct new partial key for child
+  memmove(child->partialKey() + self3->partialKeyLen + 1, child->partialKey(),
+          child->partialKeyLen);
+  memcpy(child->partialKey(), self3->partialKey(), self->partialKeyLen);
+  child->partialKey()[self3->partialKeyLen] = self3->index[0];
+  child->partialKeyLen += 1 + self3->partialKeyLen;
+
+  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 we call since the root never has a partial
+  // key.
+  setMaxVersion(child, std::max(childMaxVersion, tls->zero));
+
+  self = child;
+  tls->release(self3);
+}
+
+bool needsDownsize(Node *n) {
+  static int minTable[] = {0, kMinChildrenNode3, kMinChildrenNode16,
+                           kMinChildrenNode48, kMinChildrenNode256};
+  return n->numChildren + n->entryPresent < minTable[n->getType()];
+}
+
+void downsize(Node3 *self, WriteContext *tls, ConflictSet::Impl *impl,
+              Node *&dontInvalidate) {
+  if (self->numChildren == 0) {
+    auto *newSelf = tls->allocate<Node0>(self->partialKeyLen);
+    newSelf->copyChildrenAndKeyFrom(*self);
+    getInTree(self, impl) = newSelf;
+    tls->release(self);
+  } else {
+    assert(self->numChildren == 1 && !self->entryPresent);
+    mergeWithChild(getInTree(self, impl), tls, impl, dontInvalidate, self);
+  }
+}
+
+void downsize(Node16 *self, WriteContext *tls, ConflictSet::Impl *impl) {
+  assert(self->numChildren + int(self->entryPresent) < kMinChildrenNode16);
+  auto *newSelf = tls->allocate<Node3>(self->partialKeyLen);
+  newSelf->copyChildrenAndKeyFrom(*self);
+  getInTree(self, impl) = newSelf;
+  tls->release(self);
+}
+
+void downsize(Node48 *self, WriteContext *tls, ConflictSet::Impl *impl) {
+  assert(self->numChildren + int(self->entryPresent) < kMinChildrenNode48);
+  auto *newSelf = tls->allocate<Node16>(self->partialKeyLen);
+  newSelf->copyChildrenAndKeyFrom(*self);
+  getInTree(self, impl) = newSelf;
+  tls->release(self);
+}
+
+void downsize(Node256 *self, WriteContext *tls, ConflictSet::Impl *impl) {
+  assert(self->numChildren + int(self->entryPresent) < kMinChildrenNode256);
+  auto *self256 = (Node256 *)self;
+  auto *newSelf = tls->allocate<Node48>(self->partialKeyLen);
+  newSelf->copyChildrenAndKeyFrom(*self256);
+  getInTree(self, impl) = newSelf;
+  tls->release(self256);
+}
+
+void downsize(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
+              Node *&dontInvalidate) {
+
   switch (self->getType()) {
   case Type_Node0:           // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
-  case Type_Node3: {
+  case Type_Node3:
-    auto *self3 = (Node3 *)self;
+    downsize(static_cast<Node3 *>(self), tls, impl, dontInvalidate);
-    if (self->numChildren == 0) {
+    break;
-      auto *newSelf = tls->allocate<Node0>(self->partialKeyLen);
-      newSelf->copyChildrenAndKeyFrom(*self3);
-      getInTree(self, impl) = newSelf;
-      tls->release(self3);
-    } else if (self->numChildren == 1 && !self->entryPresent) {
-      auto *child = self3->children[0];
-      int minCapacity = self3->partialKeyLen + 1 + child->partialKeyLen;
-
-      if (minCapacity > child->getCapacity()) {
-        const bool update = child == dontInvalidate;
-        freeAndMakeCapacityAtLeast(child, minCapacity, tls, impl, true);
-        if (update) {
-          dontInvalidate = child;
-        }
-      }
-
-      // Merge partial key with child
-#if DEBUG_VERBOSE && !defined(NDEBUG)
-      fprintf(stderr, "Merge %s into %s\n",
-              getSearchPathPrintable(self).c_str(),
-              getSearchPathPrintable(child).c_str());
-#endif
-
-      InternalVersionT childMaxVersion = maxVersion(child, impl);
-
-      // Construct new partial key for child
-      memmove(child->partialKey() + self3->partialKeyLen + 1,
-              child->partialKey(), child->partialKeyLen);
-      memcpy(child->partialKey(), self3->partialKey(), self->partialKeyLen);
-      child->partialKey()[self3->partialKeyLen] = self3->index[0];
-      child->partialKeyLen += 1 + self3->partialKeyLen;
-
-      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.
-      setMaxVersion(child, impl, childMaxVersion);
-      if (child->parent) {
-        rezero(child->parent, tls->zero);
-      }
-
-      getInTree(self, impl) = child;
-      tls->release(self3);
-    }
-  } break;
   case Type_Node16:
-    if (self->numChildren + int(self->entryPresent) < kMinChildrenNode16) {
+    downsize(static_cast<Node16 *>(self), tls, impl);
-      auto *self16 = (Node16 *)self;
-      auto *newSelf = tls->allocate<Node3>(self->partialKeyLen);
-      newSelf->copyChildrenAndKeyFrom(*self16);
-      getInTree(self, impl) = newSelf;
-      tls->release(self16);
-    }
     break;
   case Type_Node48:
-    if (self->numChildren + int(self->entryPresent) < kMinChildrenNode48) {
+    downsize(static_cast<Node48 *>(self), tls, impl);
-      auto *self48 = (Node48 *)self;
-      auto *newSelf = tls->allocate<Node16>(self->partialKeyLen);
-      newSelf->copyChildrenAndKeyFrom(*self48);
-      getInTree(self, impl) = newSelf;
-      tls->release(self48);
-    }
     break;
   case Type_Node256:
-    if (self->numChildren + int(self->entryPresent) < kMinChildrenNode256) {
+    downsize(static_cast<Node256 *>(self), tls, impl);
-      auto *self256 = (Node256 *)self;
-      auto *newSelf = tls->allocate<Node48>(self->partialKeyLen);
-      newSelf->copyChildrenAndKeyFrom(*self256);
-      getInTree(self, impl) = newSelf;
-      tls->release(self256);
-    }
     break;
   default:                   // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
@@ -1637,10 +1744,10 @@ void maybeDownsize(Node *self, WriteContext *tls, 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. If erase invalidates the pointee of `dontInvalidate`, it
+// the node after self. Precondition: `self->entryPresent`
-// 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) {
+            bool logical) {
   ++tls->accum.entries_erased;
   assert(self->parent != nullptr);
 
@@ -1658,10 +1765,8 @@ Node *erase(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
   self->entryPresent = false;
 
   if (self->numChildren != 0) {
-    const bool update = result == dontInvalidate;
+    if (needsDownsize(self)) {
-    maybeDownsize(self, tls, impl, result);
+      downsize(self, tls, impl, result);
-    if (update) {
-      dontInvalidate = result;
     }
     return result;
   }
@@ -1682,7 +1787,10 @@ Node *erase(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
       parent3->children[i] = parent3->children[i + 1];
       parent3->childMaxVersion[i] = parent3->childMaxVersion[i + 1];
     }
-    assert(parent->numChildren > 0 || parent->entryPresent);
+
+    if (needsDownsize(parent3)) {
+      downsize(parent3, tls, impl, result);
+    }
   } break;
   case Type_Node16: {
     auto *parent16 = static_cast<Node16 *>(parent);
@@ -1695,8 +1803,9 @@ Node *erase(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
       parent16->childMaxVersion[i] = parent16->childMaxVersion[i + 1];
     }
 
-    // By kMinChildrenNode16
+    if (needsDownsize(parent16)) {
-    assert(parent->numChildren > 0);
+      downsize(parent16, tls, impl, result);
+    }
   } break;
   case Type_Node48: {
     auto *parent48 = static_cast<Node48 *>(parent);
@@ -1724,8 +1833,9 @@ Node *erase(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
 
     --parent->numChildren;
 
-    // By kMinChildrenNode48
+    if (needsDownsize(parent48)) {
-    assert(parent->numChildren > 0);
+      downsize(parent48, tls, impl, result);
+    }
   } break;
   case Type_Node256: {
     auto *parent256 = static_cast<Node256 *>(parent);
@@ -1734,20 +1844,14 @@ Node *erase(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
 
     --parent->numChildren;
 
-    // By kMinChildrenNode256
+    if (needsDownsize(parent256)) {
-    assert(parent->numChildren > 0);
+      downsize(parent256, tls, impl, result);
-
+    }
   } break;
   default:                   // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   }
 
-  const bool update = result == dontInvalidate;
-  maybeDownsize(parent, tls, impl, result);
-  if (update) {
-    dontInvalidate = result;
-  }
-
   return result;
 }
 
@@ -1848,10 +1952,10 @@ bool checkPrefixRead(Node *n, const std::span<const uint8_t> key,
   fprintf(stderr, "Check prefix read: %s\n", printable(key).c_str());
 #endif
   auto remaining = key;
-  auto *impl = tls->impl;
   for (;; ++tls->prefix_read_iterations_accum) {
     if (remaining.size() == 0) {
-      return maxVersion(n, impl) <= readVersion;
+      // There's no way to encode a prefix read of "", so n is not the root
+      return maxVersion(n) <= readVersion;
     }
 
     auto [child, maxV] = getChildAndMaxVersion(n, remaining[0]);
@@ -1894,7 +1998,7 @@ bool checkPrefixRead(Node *n, const std::span<const uint8_t> key,
         // n is the first physical node greater than remaining, and there's no
         // eq node. All physical nodes that start with prefix are reachable from
         // n.
-        if (maxVersion(n, impl) > readVersion) {
+        if (maxVersion(n) > readVersion) {
           return false;
         }
         goto downLeftSpine;
@@ -2373,7 +2477,6 @@ bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
   fprintf(stderr, "%s(%02x,%02x)*\n", printable(key).c_str(), begin, end);
 #endif
   auto remaining = key;
-  auto *impl = tls->impl;
   if (remaining.size() == 0) {
     return checkMaxBetweenExclusive(n, begin, end, readVersion, tls);
   }
@@ -2418,7 +2521,7 @@ bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
       if (n->entryPresent && n->entry.rangeVersion > readVersion) {
         return false;
       }
-      return maxVersion(n, impl) <= readVersion;
+      return maxVersion(n) <= readVersion;
     }
     return true;
   }
@@ -2458,7 +2561,7 @@ struct CheckRangeLeftSide {
   bool step() {
     if (remaining.size() == 0) {
       assert(searchPathLen >= prefixLen);
-      ok = maxVersion(n, impl) <= readVersion;
+      ok = maxVersion(n) <= readVersion;
       return true;
     }
 
@@ -2478,7 +2581,7 @@ struct CheckRangeLeftSide {
           return downLeftSpine();
         }
         n = c;
-        ok = maxVersion(n, impl) <= readVersion;
+        ok = maxVersion(n) <= readVersion;
         return true;
       } else {
         n = nextSibling(n);
@@ -2508,7 +2611,7 @@ struct CheckRangeLeftSide {
             ok = false;
             return true;
           }
-          ok = maxVersion(n, impl) <= readVersion;
+          ok = maxVersion(n) <= readVersion;
           return true;
         } else {
           n = nextSibling(n);
@@ -2528,7 +2631,7 @@ struct CheckRangeLeftSide {
           ok = false;
           return true;
         }
-        ok = maxVersion(n, impl) <= readVersion;
+        ok = maxVersion(n) <= readVersion;
         return true;
       }
     }
@@ -2649,7 +2752,8 @@ struct CheckRangeRightSide {
 
   bool backtrack() {
     for (;;) {
-      if (searchPathLen > prefixLen && maxVersion(n, impl) > readVersion) {
+      // searchPathLen > prefixLen implies n is not the root
+      if (searchPathLen > prefixLen && maxVersion(n) > readVersion) {
         ok = false;
         return true;
       }
@@ -2750,6 +2854,9 @@ bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
     return false;
   }
 
+  // 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));
   CheckRangeLeftSide checkRangeLeftSide{n, begin, lcp + 1, readVersion, tls};
   CheckRangeRightSide checkRangeRightSide{n, end, lcp + 1, readVersion, tls};
 
@@ -2797,14 +2904,11 @@ checkMaxBetweenExclusiveImpl<true>(Node *n, int begin, int end,
 // Returns a pointer the pointer to the newly inserted node in the tree.  Caller
 // must set `entryPresent`, and `entry` fields.  All nodes along the search path
 // of the result will have `maxVersion` set to `writeVersion` as a
-// postcondition. Nodes along the search path may be invalidated.
+// postcondition. Nodes along the search path may be invalidated. Callers must
+// ensure that the max version of the self argument is updated.
 [[nodiscard]]
 Node **insert(Node **self, std::span<const uint8_t> key,
-              InternalVersionT writeVersion, WriteContext *tls,
+              InternalVersionT writeVersion, WriteContext *tls) {
-              ConflictSet::Impl *impl) {
-
-  assert(maxVersion(*self, impl) <= writeVersion);
-  setMaxVersion(*self, impl, writeVersion);
 
   for (; key.size() != 0; ++tls->accum.insert_iterations) {
     self = &getOrCreateChild(*self, key, writeVersion, tls);
@@ -2812,36 +2916,57 @@ Node **insert(Node **self, std::span<const uint8_t> key,
   return self;
 }
 
-void destroyTree(Node *root) {
+void eraseTree(Node *root, WriteContext *tls) {
   Arena arena;
   auto toFree = vector<Node *>(arena);
   toFree.push_back(root);
 
-#if SHOW_MEMORY
-  for (auto *iter = root; iter != nullptr; iter = nextPhysical(iter)) {
-    removeNode(iter);
-    removeKey(iter);
-  }
-#endif
-
   while (toFree.size() > 0) {
     auto *n = toFree.back();
     toFree.pop_back();
-    // Add all children to toFree
+    tls->accum.entries_erased += n->entryPresent;
-    for (auto c = getChildGeq(n, 0); c != nullptr;
+    ++tls->accum.nodes_released;
-         c = getChildGeq(n, c->parentsIndex + 1)) {
+
-      assert(c != nullptr);
+    removeNode(n);
-      toFree.push_back(c);
+    removeKey(n);
+
+    switch (n->getType()) {
+    case Type_Node0: {
+      auto *n0 = static_cast<Node0 *>(n);
+      tls->release(n0);
+    } break;
+    case Type_Node3: {
+      auto *n3 = static_cast<Node3 *>(n);
+      toFree.append(std::span<Node *>(n3->children, n3->numChildren));
+      tls->release(n3);
+    } break;
+    case Type_Node16: {
+      auto *n16 = static_cast<Node16 *>(n);
+      toFree.append(std::span<Node *>(n16->children, n16->numChildren));
+      tls->release(n16);
+    } break;
+    case Type_Node48: {
+      auto *n48 = static_cast<Node48 *>(n);
+      toFree.append(std::span<Node *>(n48->children, n48->numChildren));
+      tls->release(n48);
+    } break;
+    case Type_Node256: {
+      auto *n256 = static_cast<Node256 *>(n);
+      auto *out = toFree.unsafePrepareAppend(n256->numChildren).data();
+      n256->bitSet.forEachSet([&](int i) { *out++ = n256->children[i]; });
+      assert(out == toFree.end());
+      tls->release(n256);
+    } break;
+    default:                   // GCOVR_EXCL_LINE
+      __builtin_unreachable(); // GCOVR_EXCL_LINE
     }
-    safe_free(n, n->size());
   }
 }
 
 void addPointWrite(Node *&root, std::span<const uint8_t> key,
-                   InternalVersionT writeVersion, WriteContext *tls,
+                   InternalVersionT writeVersion, WriteContext *tls) {
-                   ConflictSet::Impl *impl) {
   ++tls->accum.point_writes;
-  auto *n = *insert(&root, key, writeVersion, tls, impl);
+  auto *n = *insert(&root, key, writeVersion, tls);
   if (!n->entryPresent) {
     ++tls->accum.entries_inserted;
     auto *p = nextLogical(n);
@@ -2858,8 +2983,9 @@ void addPointWrite(Node *&root, std::span<const uint8_t> key,
   }
 }
 
-// Precondition: `node->entryPresent`
+// Precondition: `node->entryPresent`, and node is not the root
-void fixupMaxVersion(Node *node, ConflictSet::Impl *impl, WriteContext *tls) {
+void fixupMaxVersion(Node *node, WriteContext *tls) {
+  assert(node->parent);
   InternalVersionT max;
   assert(node->entryPresent);
   max = std::max(node->entry.pointVersion, tls->zero);
@@ -2893,7 +3019,7 @@ void fixupMaxVersion(Node *node, ConflictSet::Impl *impl, WriteContext *tls) {
   default:                   // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   }
-  setMaxVersion(node, impl, max);
+  setMaxVersion(node, max);
 }
 
 void addWriteRange(Node *&root, std::span<const uint8_t> begin,
@@ -2904,44 +3030,30 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
                                 std::min(begin.size(), end.size()));
   if (lcp == int(begin.size()) && end.size() == begin.size() + 1 &&
       end.back() == 0) {
-    return addPointWrite(root, begin, writeVersion, tls, impl);
+    return addPointWrite(root, begin, writeVersion, tls);
   }
   ++tls->accum.range_writes;
   const bool beginIsPrefix = lcp == int(begin.size());
 
-  Node **useAsRoot =
+  Node **useAsRoot = insert(&root, begin.subspan(0, lcp), writeVersion, tls);
-      insert(&root, begin.subspan(0, lcp), writeVersion, tls, impl);
 
-  int consumed = lcp;
+  begin = begin.subspan(lcp, begin.size() - lcp);
-
+  end = end.subspan(lcp, end.size() - lcp);
-  begin = begin.subspan(consumed, begin.size() - consumed);
-  end = end.subspan(consumed, end.size() - consumed);
-
-  auto *beginNode = *insert(useAsRoot, begin, writeVersion, tls, impl);
-
-  const bool insertedBegin = !beginNode->entryPresent;
 
+  auto *beginNode = *insert(useAsRoot, begin, writeVersion, tls);
   addKey(beginNode);
-  beginNode->entryPresent = true;
+  if (!beginNode->entryPresent) {
-
-  if (insertedBegin) {
     ++tls->accum.entries_inserted;
     auto *p = nextLogical(beginNode);
     beginNode->entry.rangeVersion =
         p == nullptr ? tls->zero : std::max(p->entry.rangeVersion, tls->zero);
-    beginNode->entry.pointVersion = writeVersion;
+    beginNode->entryPresent = true;
   }
-  assert(writeVersion >= beginNode->entry.pointVersion);
   beginNode->entry.pointVersion = writeVersion;
 
-  auto *endNode = *insert(useAsRoot, end, writeVersion, tls, impl);
+  auto *endNode = *insert(useAsRoot, end, writeVersion, tls);
-
-  const bool insertedEnd = !endNode->entryPresent;
-
   addKey(endNode);
-  endNode->entryPresent = true;
+  if (!endNode->entryPresent) {
-
-  if (insertedEnd) {
     ++tls->accum.entries_inserted;
     auto *p = nextLogical(endNode);
     endNode->entry.pointVersion =
@@ -2951,15 +3063,25 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
       beginNode = *useAsRoot;
       assert(beginNode->entryPresent);
     }
+    endNode->entryPresent = true;
   }
   endNode->entry.rangeVersion = writeVersion;
 
-  for (beginNode = nextLogical(beginNode); beginNode != endNode;
+  // Erase nodes in range
-       beginNode = erase(beginNode, tls, impl, /*logical*/ true, endNode)) {
+  assert(!beginNode->endOfRange);
+  assert(!endNode->endOfRange);
+  endNode->endOfRange = true;
+  auto *iter = beginNode;
+  for (iter = nextLogical(iter); !iter->endOfRange;
+       iter = erase(iter, tls, impl, /*logical*/ true)) {
+    assert(!iter->endOfRange);
   }
+  assert(iter->endOfRange);
+  iter->endOfRange = false;
 
-  // Inserting end trashed endNode's maxVersion. Fix that
+  // Inserting end trashed the last node's maxVersion. Fix that. Safe to call
-  fixupMaxVersion(endNode, impl, tls);
+  // since the end key always has non-zero size.
+  fixupMaxVersion(iter, tls);
 }
 
 Node *firstGeqPhysical(Node *n, const std::span<const uint8_t> key) {
@@ -3070,7 +3192,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     if (oldestExtantVersion < writeVersion - kMaxCorrectVersionWindow)
         [[unlikely]] {
       if (writeVersion > newestVersionFullPrecision + kNominalVersionWindow) {
-        destroyTree(root);
+        eraseTree(root, &tls);
         init(writeVersion - kNominalVersionWindow);
       }
 
@@ -3096,7 +3218,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         addWriteRange(root, begin, end, InternalVersionT(writeVersion), &tls,
                       this);
       } else {
-        addPointWrite(root, begin, InternalVersionT(writeVersion), &tls, this);
+        addPointWrite(root, begin, InternalVersionT(writeVersion), &tls);
       }
     }
 
@@ -3127,7 +3249,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     // Don't erase the root
     if (n == root) {
       rezero(n, oldestVersion);
-      rootMaxVersion = std::max(rootMaxVersion, oldestVersion);
       n = nextPhysical(n);
     }
     int64_t set_oldest_iterations_accum = 0;
@@ -3145,8 +3266,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);
-        Node *dummy = nullptr;
+        n = erase(n, &tls, this, /*logical*/ false);
-        n = erase(n, &tls, this, /*logical*/ false, dummy);
       } else {
         maybeDecreaseCapacity(n, &tls, this);
         n = nextPhysical(n);
@@ -3218,7 +3338,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     root = tls.allocate<Node0>(0);
     root->numChildren = 0;
     root->parent = nullptr;
-    rootMaxVersion = this->oldestVersion;
     root->entryPresent = false;
     root->partialKeyLen = 0;
 
@@ -3238,7 +3357,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     initMetrics();
   }
   ~Impl() {
-    destroyTree(root);
+    eraseTree(root, &tls);
     safe_free(metrics, metricsCount * sizeof(metrics[0]));
   }
 
@@ -3249,7 +3368,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   int64_t keyUpdates;
 
   Node *root;
-  InternalVersionT rootMaxVersion;
   InternalVersionT oldestVersion;
   int64_t oldestVersionFullPrecision;
   int64_t oldestExtantVersion;
@@ -3348,118 +3466,6 @@ Metric::Metric(ConflictSet::Impl *impl, const char *name, const char *help,
   ++impl->metricsCount;
 }
 
-InternalVersionT maxVersion(Node *n, ConflictSet::Impl *impl) {
-  int index = n->parentsIndex;
-  n = n->parent;
-  if (n == nullptr) {
-    return impl->rootMaxVersion;
-  }
-  switch (n->getType()) {
-  case Type_Node0:           // GCOVR_EXCL_LINE
-    __builtin_unreachable(); // GCOVR_EXCL_LINE
-  case Type_Node3: {
-    auto *n3 = static_cast<Node3 *>(n);
-    int i = getNodeIndex(n3, index);
-    return n3->childMaxVersion[i];
-  }
-  case Type_Node16: {
-    auto *n16 = static_cast<Node16 *>(n);
-    int i = getNodeIndex(n16, index);
-    return n16->childMaxVersion[i];
-  }
-  case Type_Node48: {
-    auto *n48 = static_cast<Node48 *>(n);
-    assert(n48->bitSet.test(index));
-    return n48->childMaxVersion[n48->index[index]];
-  }
-  case Type_Node256: {
-    auto *n256 = static_cast<Node256 *>(n);
-    assert(n256->bitSet.test(index));
-    return n256->childMaxVersion[index];
-  }
-  default:                   // GCOVR_EXCL_LINE
-    __builtin_unreachable(); // GCOVR_EXCL_LINE
-  }
-}
-
-// Precondition `n` is not the root
-InternalVersionT exchangeMaxVersion(Node *n, InternalVersionT newMax) {
-  int index = n->parentsIndex;
-  n = n->parent;
-  assert(n != nullptr);
-  switch (n->getType()) {
-  case Type_Node0:           // GCOVR_EXCL_LINE
-    __builtin_unreachable(); // GCOVR_EXCL_LINE
-  case Type_Node3: {
-    auto *n3 = static_cast<Node3 *>(n);
-    int i = getNodeIndex(n3, index);
-    return std::exchange(n3->childMaxVersion[i], newMax);
-  }
-  case Type_Node16: {
-    auto *n16 = static_cast<Node16 *>(n);
-    int i = getNodeIndex(n16, index);
-    return std::exchange(n16->childMaxVersion[i], newMax);
-  }
-  case Type_Node48: {
-    auto *n48 = static_cast<Node48 *>(n);
-    assert(n48->bitSet.test(index));
-    return std::exchange(n48->childMaxVersion[n48->index[index]], newMax);
-  }
-  case Type_Node256: {
-    auto *n256 = static_cast<Node256 *>(n);
-    assert(n256->bitSet.test(index));
-    return std::exchange(n256->childMaxVersion[index], newMax);
-  }
-  default:                   // GCOVR_EXCL_LINE
-    __builtin_unreachable(); // GCOVR_EXCL_LINE
-  }
-}
-
-void setMaxVersion(Node *n, ConflictSet::Impl *impl, InternalVersionT newMax) {
-  int index = n->parentsIndex;
-  n = n->parent;
-  if (n == nullptr) {
-    impl->rootMaxVersion = newMax;
-    return;
-  }
-  switch (n->getType()) {
-  case Type_Node0:           // GCOVR_EXCL_LINE
-    __builtin_unreachable(); // GCOVR_EXCL_LINE
-  case Type_Node3: {
-    auto *n3 = static_cast<Node3 *>(n);
-    int i = getNodeIndex(n3, index);
-    n3->childMaxVersion[i] = newMax;
-    return;
-  }
-  case Type_Node16: {
-    auto *n16 = static_cast<Node16 *>(n);
-    int i = getNodeIndex(n16, index);
-    n16->childMaxVersion[i] = newMax;
-    return;
-  }
-  case Type_Node48: {
-    auto *n48 = static_cast<Node48 *>(n);
-    assert(n48->bitSet.test(index));
-    int i = n48->index[index];
-    n48->childMaxVersion[i] = newMax;
-    n48->maxOfMax[i >> Node48::kMaxOfMaxShift] = std::max<InternalVersionT>(
-        n48->maxOfMax[i >> Node48::kMaxOfMaxShift], newMax);
-    return;
-  }
-  case Type_Node256: {
-    auto *n256 = static_cast<Node256 *>(n);
-    assert(n256->bitSet.test(index));
-    n256->childMaxVersion[index] = newMax;
-    n256->maxOfMax[index >> Node256::kMaxOfMaxShift] =
-        std::max<InternalVersionT>(
-            n256->maxOfMax[index >> Node256::kMaxOfMaxShift], newMax);
-    return;
-  }
-  default:                   // GCOVR_EXCL_LINE
-    __builtin_unreachable(); // GCOVR_EXCL_LINE
-  }
-}
-
 Node *&getInTree(Node *n, ConflictSet::Impl *impl) {
   return n->parent == nullptr ? impl->root
                               : getChildExists(n->parent, n->parentsIndex);
@@ -3741,13 +3747,13 @@ std::string getSearchPath(Node *n) {
         fprintf(file,
                 " k_%p [label=\"m=%" PRId64 " p=%" PRId64 " r=%" PRId64
                 "\n%s\", pos=\"%d,%d!\"];\n",
-                (void *)n, maxVersion(n, impl).toInt64(),
+                (void *)n, maxVersion(n).toInt64(),
                 n->entry.pointVersion.toInt64(),
                 n->entry.rangeVersion.toInt64(),
                 getPartialKeyPrintable(n).c_str(), x, y);
       } else {
         fprintf(file, " k_%p [label=\"m=%" PRId64 "\n%s\", pos=\"%d,%d!\"];\n",
-                (void *)n, maxVersion(n, impl).toInt64(),
+                (void *)n, maxVersion(n).toInt64(),
                 getPartialKeyPrintable(n).c_str(), x, y);
       }
       x += kSeparation;
@@ -3858,11 +3864,11 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
       expected = std::max(expected, borrowed->entry.rangeVersion);
     }
   }
-  if (maxVersion(node, impl) > oldestVersion &&
+  if (node->parent && maxVersion(node) > oldestVersion &&
-      maxVersion(node, impl) != expected) {
+      maxVersion(node) != expected) {
     fprintf(stderr, "%s has max version %" PRId64 " . Expected %" PRId64 "\n",
-            getSearchPathPrintable(node).c_str(),
+            getSearchPathPrintable(node).c_str(), maxVersion(node).toInt64(),
-            maxVersion(node, impl).toInt64(), expected.toInt64());
+            expected.toInt64());
     success = false;
   }
   return expected;
@@ -3926,7 +3932,7 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
   bool success = true;
 
   if (node->partialKeyLen > 0) {
-    fprintf(stderr, "Root cannot have a partial key");
+    fprintf(stderr, "Root cannot have a partial key\n");
     success = false;
   }
   checkParentPointers(node, success);

Internal.h

@@ -273,6 +273,16 @@ template <class T> struct Vector {
     size_ += slice.size();
   }
 
+  // Caller must write to the returned slice
+  std::span<T> unsafePrepareAppend(int appendSize) {
+    if (size_ + appendSize > capacity) {
+      grow(std::max<int>(size_ + appendSize, capacity * 2));
+    }
+    auto result = std::span<T>(t + size_, appendSize);
+    size_ += appendSize;
+    return result;
+  }
+
   void push_back(const T &t) { append(std::span<const T>(&t, 1)); }
 
   T *begin() { return t; }

README.md

@@ -24,16 +24,15 @@ Hardware for all benchmarks is an AMD Ryzen 9 7900 with (2x32GB) 5600MT/s CL28-3
 
 |               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
 |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
-|               10.80 |       92,600,541.52 |    0.6% |          180.38 |           54.49 |  3.310 |          41.51 |    0.4% |      0.01 | `point reads`
+|               11.04 |       90,614,308.12 |    0.8% |          180.38 |           55.13 |  3.272 |          41.51 |    0.4% |      0.01 | `point reads`
-|               15.00 |       66,687,691.68 |    0.4% |          278.44 |           76.44 |  3.642 |          55.56 |    0.3% |      0.01 | `prefix reads`
+|               14.96 |       66,843,629.12 |    0.4% |          274.41 |           74.73 |  3.672 |          55.05 |    0.3% |      0.01 | `prefix reads`
-|               36.81 |       27,163,394.61 |    0.4% |          795.06 |          187.91 |  4.231 |         142.67 |    0.2% |      0.01 | `range reads`
+|               37.06 |       26,982,847.61 |    0.2% |          791.04 |          185.28 |  4.269 |         142.67 |    0.2% |      0.01 | `range reads`
-|               18.14 |       55,137,674.01 |    1.2% |          338.19 |           92.86 |  3.642 |          42.81 |    0.4% |      0.01 | `point writes`
+|               17.89 |       55,887,365.73 |    0.6% |          335.54 |           89.79 |  3.737 |          43.84 |    0.4% |      0.01 | `point writes`
-|               33.19 |       30,127,119.71 |    0.1% |          681.03 |          170.05 |  4.005 |          98.68 |    0.2% |      0.01 | `prefix writes`
+|               31.85 |       31,394,336.65 |    0.3% |          615.32 |          159.63 |  3.855 |          87.69 |    0.2% |      0.01 | `prefix writes`
-|               37.37 |       26,759,432.70 |    1.9% |          779.70 |          195.45 |  3.989 |         114.21 |    0.0% |      0.01 | `range writes`
+|               36.17 |       27,647,221.45 |    0.6% |          705.11 |          182.80 |  3.857 |         100.62 |    0.1% |      0.01 | `range writes`
-|               74.36 |       13,448,582.47 |    1.9% |        1,425.68 |          389.08 |  3.664 |         258.88 |    0.1% |      0.01 | `monotonic increasing point writes`
+|               79.01 |       12,656,457.78 |    0.7% |        1,498.35 |          402.46 |  3.723 |         270.50 |    0.1% |      0.01 | `monotonic increasing point writes`
-|          316,928.00 |            3,155.29 |    1.5% |    3,992,986.00 |    1,699,813.00 |  2.349 |     806,226.50 |    0.0% |      0.01 | `worst case for radix tree`
+|          303,667.50 |            3,293.08 |    1.1% |    3,931,273.00 |    1,612,702.50 |  2.438 |     806,223.33 |    0.0% |      0.01 | `worst case for radix tree`
-|               75.26 |       13,286,517.16 |    0.5% |        1,590.01 |          386.67 |  4.112 |         258.00 |    0.0% |      0.01 | `create and destroy`
+|               83.70 |       11,947,443.83 |    0.7% |        1,738.03 |          429.06 |  4.051 |         270.01 |    0.0% |      0.01 | `create and destroy`
-
 
 # "Real data" test
 
@@ -48,7 +47,7 @@ Check: 4.47891 seconds, 364.05 MB/s, Add: 4.55599 seconds, 123.058 MB/s, Gc rati
 ## radix tree
 
 ```
-Check: 0.910234 seconds, 1791.35 MB/s, Add: 1.25908 seconds, 445.287 MB/s, Gc ratio: 44.0415%
+Check: 0.958985 seconds, 1700.28 MB/s, Add: 1.35083 seconds, 415.044 MB/s, Gc ratio: 44.4768%, Peak idle memory: 2.33588e+06
 ```
 
 ## hash table

ServerBench.cpp

@@ -164,6 +164,63 @@ double toSeconds(timeval t) {
   return double(t.tv_sec) + double(t.tv_usec) * 1e-6;
 }
 
+#include <linux/perf_event.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#ifdef __linux__
+struct PerfCounter {
+  explicit PerfCounter(int event) {
+    struct perf_event_attr pe;
+
+    memset(&pe, 0, sizeof(pe));
+    pe.type = PERF_TYPE_HARDWARE;
+    pe.size = sizeof(pe);
+    pe.config = event;
+    pe.inherit = 1;
+    pe.exclude_kernel = 1;
+    pe.exclude_hv = 1;
+
+    fd = perf_event_open(&pe, 0, -1, -1, 0);
+    if (fd == -1) {
+      fprintf(stderr, "Error opening leader %llx\n", pe.config);
+      exit(EXIT_FAILURE);
+    }
+  }
+
+  int64_t total() {
+    int64_t count;
+    if (read(fd, &count, sizeof(count)) != sizeof(count)) {
+      perror("read instructions from perf");
+      abort();
+    }
+    return count;
+  }
+
+  ~PerfCounter() { close(fd); }
+
+private:
+  int fd;
+  static long perf_event_open(struct perf_event_attr *hw_event, pid_t pid,
+                              int cpu, int group_fd, unsigned long flags) {
+    int ret;
+
+    ret = syscall(SYS_perf_event_open, hw_event, pid, cpu, group_fd, flags);
+    return ret;
+  }
+};
+#else
+struct PerfCounter {
+  explicit PerPerfCounter(int) {}
+  int64_t total() { return 0; }
+};
+#endif
+
 int main(int argc, char **argv) {
   if (argc != 3) {
     goto fail;
@@ -176,6 +233,8 @@ int main(int argc, char **argv) {
     int metricsCount;
     cs.getMetricsV1(&metrics, &metricsCount);
 
+    PerfCounter instructions{PERF_COUNT_HW_INSTRUCTIONS};
+    PerfCounter cycles{PERF_COUNT_HW_CPU_CYCLES};
     auto w = std::thread{workload, &cs};
 
     for (;;) {
@@ -203,6 +262,16 @@ int main(int argc, char **argv) {
               "transactions_total ";
       body += std::to_string(transactions.load(std::memory_order_relaxed));
       body += "\n";
+      body += "# HELP instructions_total Total number of instructions\n"
+              "# TYPE instructions_total counter\n"
+              "instructions_total ";
+      body += std::to_string(instructions.total());
+      body += "\n";
+      body += "# HELP cycles_total Total number of cycles\n"
+              "# TYPE cycles_total counter\n"
+              "cycles_total ";
+      body += std::to_string(cycles.total());
+      body += "\n";
 
       for (int i = 0; i < metricsCount; ++i) {
         body += "# HELP ";