Just use a normal && in a place where being branch-free isn't a big deal

CMakeLists.txt

@@ -353,6 +353,11 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
   add_executable(real_data_bench RealDataBench.cpp)
   target_link_libraries(real_data_bench PRIVATE ${PROJECT_NAME})
   set_target_properties(real_data_bench PROPERTIES SKIP_BUILD_RPATH ON)
+
+  # fuzzer-based perf
+  add_executable(driver_perf TestDriver.cpp)
+  target_compile_definitions(driver_perf PRIVATE PERF_TEST=1)
+  target_link_libraries(driver_perf PRIVATE ${PROJECT_NAME})
 endif()
 
 # packaging

ConflictSet.cpp

@@ -705,7 +705,61 @@ size_t Node::size() const {
   }
 }
 
-struct NodeAllocators {
+// A type that's plumbed along the check call tree. Lifetime ends after each
+// check call.
+struct ReadContext {
+  double point_read_accum = 0;
+  double prefix_read_accum = 0;
+  double range_read_accum = 0;
+  double point_read_short_circuit_accum = 0;
+  double prefix_read_short_circuit_accum = 0;
+  double range_read_short_circuit_accum = 0;
+  double point_read_iterations_accum = 0;
+  double prefix_read_iterations_accum = 0;
+  double range_read_iterations_accum = 0;
+  double range_read_node_scan_accum = 0;
+  ConflictSet::Impl *impl;
+};
+
+// A type that's plumbed along the non-const call tree. Same lifetime as
+// ConflictSet::Impl
+struct WriteContext {
+  double entries_erased_accum = 0;
+  double insert_iterations_accum = 0;
+  double entries_inserted_accum = 0;
+  double nodes_allocated_accum = 0;
+  double nodes_released_accum = 0;
+  template <class T> T *allocate(int c) {
+    ++nodes_allocated_accum;
+    if constexpr (std::is_same_v<T, Node0>) {
+      return node0.allocate(c);
+    } else if constexpr (std::is_same_v<T, Node3>) {
+      return node3.allocate(c);
+    } else if constexpr (std::is_same_v<T, Node16>) {
+      return node16.allocate(c);
+    } else if constexpr (std::is_same_v<T, Node48>) {
+      return node48.allocate(c);
+    } else if constexpr (std::is_same_v<T, Node256>) {
+      return node256.allocate(c);
+    }
+  }
+  template <class T> void release(T *c) {
+    static_assert(!std::is_same_v<T, Node>);
+    ++nodes_released_accum;
+    if constexpr (std::is_same_v<T, Node0>) {
+      return node0.release(c);
+    } else if constexpr (std::is_same_v<T, Node3>) {
+      return node3.release(c);
+    } else if constexpr (std::is_same_v<T, Node16>) {
+      return node16.release(c);
+    } else if constexpr (std::is_same_v<T, Node48>) {
+      return node48.release(c);
+    } else if constexpr (std::is_same_v<T, Node256>) {
+      return node256.release(c);
+    }
+  }
+
+private:
   BoundedFreeListAllocator<Node0> node0;
   BoundedFreeListAllocator<Node3> node3;
   BoundedFreeListAllocator<Node16> node16;
@@ -949,8 +1003,7 @@ int getChildGeq(Node *self, int child) {
 
 // Caller is responsible for assigning a non-null pointer to the returned
 // reference if null
-Node *&getOrCreateChild(Node *&self, uint8_t index,
+Node *&getOrCreateChild(Node *&self, uint8_t index, WriteContext *tls) {
-                        NodeAllocators *allocators) {
 
   // Fast path for if it exists already
   switch (self->getType()) {
@@ -991,9 +1044,9 @@ Node *&getOrCreateChild(Node *&self, uint8_t index,
   case Type_Node0: {
     auto *self0 = static_cast<Node0 *>(self);
 
-    auto *newSelf = allocators->node3.allocate(self->partialKeyLen);
+    auto *newSelf = tls->allocate<Node3>(self->partialKeyLen);
     newSelf->copyChildrenAndKeyFrom(*self0);
-    allocators->node0.release(self0);
+    tls->release(self0);
     self = newSelf;
 
     goto insert3;
@@ -1001,9 +1054,9 @@ Node *&getOrCreateChild(Node *&self, uint8_t index,
   case Type_Node3: {
     if (self->numChildren == Node3::kMaxNodes) {
       auto *self3 = static_cast<Node3 *>(self);
-      auto *newSelf = allocators->node16.allocate(self->partialKeyLen);
+      auto *newSelf = tls->allocate<Node16>(self->partialKeyLen);
       newSelf->copyChildrenAndKeyFrom(*self3);
-      allocators->node3.release(self3);
+      tls->release(self3);
       self = newSelf;
       goto insert16;
     }
@@ -1033,9 +1086,9 @@ Node *&getOrCreateChild(Node *&self, uint8_t index,
   case Type_Node16: {
     if (self->numChildren == Node16::kMaxNodes) {
       auto *self16 = static_cast<Node16 *>(self);
-      auto *newSelf = allocators->node48.allocate(self->partialKeyLen);
+      auto *newSelf = tls->allocate<Node48>(self->partialKeyLen);
       newSelf->copyChildrenAndKeyFrom(*self16);
-      allocators->node16.release(self16);
+      tls->release(self16);
       self = newSelf;
       goto insert48;
     }
@@ -1111,9 +1164,9 @@ Node *&getOrCreateChild(Node *&self, uint8_t index,
 
     if (self->numChildren == 48) {
       auto *self48 = static_cast<Node48 *>(self);
-      auto *newSelf = allocators->node256.allocate(self->partialKeyLen);
+      auto *newSelf = tls->allocate<Node256>(self->partialKeyLen);
       newSelf->copyChildrenAndKeyFrom(*self48);
-      allocators->node48.release(self48);
+      tls->release(self48);
       self = newSelf;
       goto insert256;
     }
@@ -1167,18 +1220,17 @@ Node *nextLogical(Node *node) {
 
 // Invalidates `self`, replacing it with a node of at least capacity.
 // Does not return nodes to freelists when kUseFreeList is false.
-void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
+void freeAndMakeCapacityAtLeast(Node *&self, int capacity, WriteContext *tls,
-                                NodeAllocators *allocators,
                                 ConflictSet::Impl *impl,
                                 const bool kUseFreeList) {
   switch (self->getType()) {
   case Type_Node0: {
     auto *self0 = (Node0 *)self;
-    auto *newSelf = allocators->node0.allocate(capacity);
+    auto *newSelf = tls->allocate<Node0>(capacity);
     newSelf->copyChildrenAndKeyFrom(*self0);
     getInTree(self, impl) = newSelf;
     if (kUseFreeList) {
-      allocators->node0.release(self0);
+      tls->release(self0);
     } else {
       removeNode(self0);
       safe_free(self0, self0->size());
@@ -1187,11 +1239,11 @@ void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
   } break;
   case Type_Node3: {
     auto *self3 = (Node3 *)self;
-    auto *newSelf = allocators->node3.allocate(capacity);
+    auto *newSelf = tls->allocate<Node3>(capacity);
     newSelf->copyChildrenAndKeyFrom(*self3);
     getInTree(self, impl) = newSelf;
     if (kUseFreeList) {
-      allocators->node3.release(self3);
+      tls->release(self3);
     } else {
       removeNode(self3);
       safe_free(self3, self3->size());
@@ -1200,11 +1252,11 @@ void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
   } break;
   case Type_Node16: {
     auto *self16 = (Node16 *)self;
-    auto *newSelf = allocators->node16.allocate(capacity);
+    auto *newSelf = tls->allocate<Node16>(capacity);
     newSelf->copyChildrenAndKeyFrom(*self16);
     getInTree(self, impl) = newSelf;
     if (kUseFreeList) {
-      allocators->node16.release(self16);
+      tls->release(self16);
     } else {
       removeNode(self16);
       safe_free(self16, self16->size());
@@ -1213,11 +1265,11 @@ void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
   } break;
   case Type_Node48: {
     auto *self48 = (Node48 *)self;
-    auto *newSelf = allocators->node48.allocate(capacity);
+    auto *newSelf = tls->allocate<Node48>(capacity);
     newSelf->copyChildrenAndKeyFrom(*self48);
     getInTree(self, impl) = newSelf;
     if (kUseFreeList) {
-      allocators->node48.release(self48);
+      tls->release(self48);
     } else {
       removeNode(self48);
       safe_free(self48, self48->size());
@@ -1226,11 +1278,11 @@ void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
   } break;
   case Type_Node256: {
     auto *self256 = (Node256 *)self;
-    auto *newSelf = allocators->node256.allocate(capacity);
+    auto *newSelf = tls->allocate<Node256>(capacity);
     newSelf->copyChildrenAndKeyFrom(*self256);
     getInTree(self, impl) = newSelf;
     if (kUseFreeList) {
-      allocators->node256.release(self256);
+      tls->release(self256);
     } else {
       removeNode(self256);
       safe_free(self256, self256->size());
@@ -1245,7 +1297,7 @@ void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
 // Fix larger-than-desired capacities. Does not return nodes to freelists,
 // since that wouldn't actually reclaim the memory used for partial key
 // capacity.
-void maybeDecreaseCapacity(Node *&self, NodeAllocators *allocators,
+void maybeDecreaseCapacity(Node *&self, WriteContext *tls,
                            ConflictSet::Impl *impl) {
 
   const int maxCapacity =
@@ -1258,7 +1310,7 @@ void maybeDecreaseCapacity(Node *&self, NodeAllocators *allocators,
   if (self->getCapacity() <= maxCapacity) {
     return;
   }
-  freeAndMakeCapacityAtLeast(self, maxCapacity, allocators, impl, false);
+  freeAndMakeCapacityAtLeast(self, maxCapacity, tls, impl, false);
 }
 
 void rezero(Node *n, InternalVersionT z) {
@@ -1308,8 +1360,8 @@ void rezero(Node *n, InternalVersionT z) {
   }
 }
 
-void maybeDownsize(Node *self, NodeAllocators *allocators,
+void maybeDownsize(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
-                   ConflictSet::Impl *impl, Node *&dontInvalidate) {
+                   Node *&dontInvalidate) {
 
 #if DEBUG_VERBOSE && !defined(NDEBUG)
   fprintf(stderr, "maybeDownsize: %s\n", getSearchPathPrintable(self).c_str());
@@ -1321,17 +1373,17 @@ void maybeDownsize(Node *self, NodeAllocators *allocators,
   case Type_Node3: {
     auto *self3 = (Node3 *)self;
     if (self->numChildren == 0) {
-      auto *newSelf = allocators->node0.allocate(self->partialKeyLen);
+      auto *newSelf = tls->allocate<Node0>(self->partialKeyLen);
       newSelf->copyChildrenAndKeyFrom(*self3);
       getInTree(self, impl) = newSelf;
-      allocators->node3.release(self3);
+      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, allocators, impl, true);
+        freeAndMakeCapacityAtLeast(child, minCapacity, tls, impl, true);
         if (update) {
           dontInvalidate = child;
         }
@@ -1364,34 +1416,34 @@ void maybeDownsize(Node *self, NodeAllocators *allocators,
       }
 
       getInTree(self, impl) = child;
-      allocators->node3.release(self3);
+      tls->release(self3);
     }
   } break;
   case Type_Node16:
     if (self->numChildren + int(self->entryPresent) < kMinChildrenNode16) {
       auto *self16 = (Node16 *)self;
-      auto *newSelf = allocators->node3.allocate(self->partialKeyLen);
+      auto *newSelf = tls->allocate<Node3>(self->partialKeyLen);
       newSelf->copyChildrenAndKeyFrom(*self16);
       getInTree(self, impl) = newSelf;
-      allocators->node16.release(self16);
+      tls->release(self16);
     }
     break;
   case Type_Node48:
     if (self->numChildren + int(self->entryPresent) < kMinChildrenNode48) {
       auto *self48 = (Node48 *)self;
-      auto *newSelf = allocators->node16.allocate(self->partialKeyLen);
+      auto *newSelf = tls->allocate<Node16>(self->partialKeyLen);
       newSelf->copyChildrenAndKeyFrom(*self48);
       getInTree(self, impl) = newSelf;
-      allocators->node48.release(self48);
+      tls->release(self48);
     }
     break;
   case Type_Node256:
     if (self->numChildren + int(self->entryPresent) < kMinChildrenNode256) {
       auto *self256 = (Node256 *)self;
-      auto *newSelf = allocators->node48.allocate(self->partialKeyLen);
+      auto *newSelf = tls->allocate<Node48>(self->partialKeyLen);
       newSelf->copyChildrenAndKeyFrom(*self256);
       getInTree(self, impl) = newSelf;
-      allocators->node256.release(self256);
+      tls->release(self256);
     }
     break;
   default:                   // GCOVR_EXCL_LINE
@@ -1402,9 +1454,10 @@ void maybeDownsize(Node *self, NodeAllocators *allocators,
 // 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
-// will update it to its new pointee as well.
+// will update it to its new pointee as well. Precondition: `self->entryPresent`
-Node *erase(Node *self, NodeAllocators *allocators, ConflictSet::Impl *impl,
+Node *erase(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
             bool logical, Node *&dontInvalidate) {
+  ++tls->entries_erased_accum;
   assert(self->parent != nullptr);
 
 #if DEBUG_VERBOSE && !defined(NDEBUG)
@@ -1417,11 +1470,12 @@ Node *erase(Node *self, NodeAllocators *allocators, ConflictSet::Impl *impl,
   auto *result = logical ? nextLogical(self) : nextPhysical(self);
 
   removeKey(self);
+  assert(self->entryPresent);
   self->entryPresent = false;
 
   if (self->numChildren != 0) {
     const bool update = result == dontInvalidate;
-    maybeDownsize(self, allocators, impl, result);
+    maybeDownsize(self, tls, impl, result);
     if (update) {
       dontInvalidate = result;
     }
@@ -1429,7 +1483,7 @@ Node *erase(Node *self, NodeAllocators *allocators, ConflictSet::Impl *impl,
   }
 
   assert(self->getType() == Type_Node0);
-  allocators->node0.release((Node0 *)self);
+  tls->release((Node0 *)self);
 
   switch (parent->getType()) {
   case Type_Node0:           // GCOVR_EXCL_LINE
@@ -1517,7 +1571,7 @@ Node *erase(Node *self, NodeAllocators *allocators, ConflictSet::Impl *impl,
   }
 
   const bool update = result == dontInvalidate;
-  maybeDownsize(parent, allocators, impl, result);
+  maybeDownsize(parent, tls, impl, result);
   if (update) {
     dontInvalidate = result;
   }
@@ -1712,30 +1766,20 @@ struct SearchStepWise {
   }
 };
 
-thread_local double point_read_accum = 0;
-thread_local double prefix_read_accum = 0;
-thread_local double range_read_accum = 0;
-thread_local double point_read_short_circuit_accum = 0;
-thread_local double prefix_read_short_circuit_accum = 0;
-thread_local double range_read_short_circuit_accum = 0;
-thread_local double point_read_iterations_accum = 0;
-thread_local double prefix_read_iterations_accum = 0;
-thread_local double range_read_iterations_accum = 0;
-thread_local double range_read_node_scan_accum = 0;
-
 // Logically this is the same as performing firstGeq and then checking against
 // point or range version according to cmp, but this version short circuits as
 // soon as it can prove that there's no conflict.
 bool checkPointRead(Node *n, const std::span<const uint8_t> key,
-                    InternalVersionT readVersion, ConflictSet::Impl *impl) {
+                    InternalVersionT readVersion, ReadContext *tls) {
-  ++point_read_accum;
+  ++tls->point_read_accum;
 #if DEBUG_VERBOSE && !defined(NDEBUG)
   fprintf(stderr, "Check point read: %s\n", printable(key).c_str());
 #endif
   auto remaining = key;
-  for (;; ++point_read_iterations_accum) {
+  auto *impl = tls->impl;
+  for (;; ++tls->point_read_iterations_accum) {
     if (maxVersion(n, impl) <= readVersion) {
-      ++point_read_short_circuit_accum;
+      ++tls->point_read_short_circuit_accum;
       return true;
     }
     if (remaining.size() == 0) {
@@ -1806,20 +1850,21 @@ downLeftSpine:
 // max version or range version if this prefix doesn't exist, but this version
 // short circuits as soon as it can prove that there's no conflict.
 bool checkPrefixRead(Node *n, const std::span<const uint8_t> key,
-                     InternalVersionT readVersion, ConflictSet::Impl *impl) {
+                     InternalVersionT readVersion, ReadContext *tls) {
-  ++prefix_read_accum;
+  ++tls->prefix_read_accum;
 #if DEBUG_VERBOSE && !defined(NDEBUG)
   fprintf(stderr, "Check prefix read: %s\n", printable(key).c_str());
 #endif
   auto remaining = key;
-  for (;; ++prefix_read_iterations_accum) {
+  auto *impl = tls->impl;
+  for (;; ++tls->prefix_read_iterations_accum) {
     auto m = maxVersion(n, impl);
     if (remaining.size() == 0) {
       return m <= readVersion;
     }
 
     if (m <= readVersion) {
-      ++prefix_read_short_circuit_accum;
+      ++tls->prefix_read_short_circuit_accum;
       return true;
     }
 
@@ -2049,8 +2094,8 @@ scan16(const InternalVersionT *vs, int begin, int end,
 // account for the range version of firstGt(searchpath(n) + [end - 1])
 template <bool kAVX512>
 bool checkMaxBetweenExclusive(Node *n, int begin, int end,
-                              InternalVersionT readVersion) {
+                              InternalVersionT readVersion, ReadContext *tls) {
-  ++range_read_node_scan_accum;
+  ++tls->range_read_node_scan_accum;
   assume(-1 <= begin);
   assume(begin <= 256);
   assume(-1 <= end);
@@ -2319,13 +2364,14 @@ Vector<uint8_t> getSearchPath(Arena &arena, Node *n) {
 template <bool kAVX512>
 bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
                           int end, InternalVersionT readVersion,
-                          ConflictSet::Impl *impl) {
+                          ReadContext *tls) {
 #if DEBUG_VERBOSE && !defined(NDEBUG)
   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<kAVX512>(n, begin, end, readVersion);
+    return checkMaxBetweenExclusive<kAVX512>(n, begin, end, readVersion, tls);
   }
 
   auto *child = getChild(n, remaining[0]);
@@ -2391,9 +2437,9 @@ namespace {
 // that are >= key is <= readVersion
 template <bool kAVX512> struct CheckRangeLeftSide {
   CheckRangeLeftSide(Node *n, std::span<const uint8_t> key, int prefixLen,
-                     InternalVersionT readVersion, ConflictSet::Impl *impl)
+                     InternalVersionT readVersion, ReadContext *tls)
       : n(n), remaining(key), prefixLen(prefixLen), readVersion(readVersion),
-        impl(impl) {
+        impl(tls->impl), tls(tls) {
 #if DEBUG_VERBOSE && !defined(NDEBUG)
     fprintf(stderr, "Check range left side from %s for keys starting with %s\n",
             printable(key).c_str(),
@@ -2406,6 +2452,7 @@ template <bool kAVX512> struct CheckRangeLeftSide {
   int prefixLen;
   InternalVersionT readVersion;
   ConflictSet::Impl *impl;
+  ReadContext *tls;
   int searchPathLen = 0;
   bool ok;
 
@@ -2421,8 +2468,8 @@ template <bool kAVX512> struct CheckRangeLeftSide {
     }
 
     if (searchPathLen >= prefixLen) {
-      if (!checkMaxBetweenExclusive<kAVX512>(n, remaining[0], 256,
+      if (!checkMaxBetweenExclusive<kAVX512>(n, remaining[0], 256, readVersion,
-                                             readVersion)) {
+                                             tls)) {
         ok = false;
         return true;
       }
@@ -2511,9 +2558,9 @@ template <bool kAVX512> struct CheckRangeLeftSide {
 // that are < key is <= readVersion
 template <bool kAVX512> struct CheckRangeRightSide {
   CheckRangeRightSide(Node *n, std::span<const uint8_t> key, int prefixLen,
-                      InternalVersionT readVersion, ConflictSet::Impl *impl)
+                      InternalVersionT readVersion, ReadContext *tls)
       : n(n), key(key), remaining(key), prefixLen(prefixLen),
-        readVersion(readVersion), impl(impl) {
+        readVersion(readVersion), impl(tls->impl), tls(tls) {
 #if DEBUG_VERBOSE && !defined(NDEBUG)
     fprintf(stderr, "Check range right side to %s for keys starting with %s\n",
             printable(key).c_str(),
@@ -2527,6 +2574,7 @@ template <bool kAVX512> struct CheckRangeRightSide {
   int prefixLen;
   InternalVersionT readVersion;
   ConflictSet::Impl *impl;
+  ReadContext *tls;
   int searchPathLen = 0;
   bool ok;
 
@@ -2551,8 +2599,8 @@ template <bool kAVX512> struct CheckRangeRightSide {
         return true;
       }
 
-      if (!checkMaxBetweenExclusive<kAVX512>(n, -1, remaining[0],
+      if (!checkMaxBetweenExclusive<kAVX512>(n, -1, remaining[0], readVersion,
-                                             readVersion)) {
+                                             tls)) {
         ok = false;
         return true;
       }
@@ -2647,28 +2695,29 @@ template <bool kAVX512> struct CheckRangeRightSide {
 template <bool kAVX512>
 bool checkRangeReadImpl(Node *n, std::span<const uint8_t> begin,
                         std::span<const uint8_t> end,
-                        InternalVersionT readVersion, ConflictSet::Impl *impl) {
+                        InternalVersionT readVersion, ReadContext *tls) {
   int lcp = longestCommonPrefix(begin.data(), end.data(),
                                 std::min(begin.size(), end.size()));
   if (lcp == int(begin.size()) && end.size() == begin.size() + 1 &&
       end.back() == 0) {
-    return checkPointRead(n, begin, readVersion, impl);
+    return checkPointRead(n, begin, readVersion, tls);
   }
   if (lcp == int(begin.size() - 1) && end.size() == begin.size() &&
       int(begin.back()) + 1 == int(end.back())) {
-    return checkPrefixRead(n, begin, readVersion, impl);
+    return checkPrefixRead(n, begin, readVersion, tls);
   }
 
-  ++range_read_accum;
+  ++tls->range_read_accum;
 
   SearchStepWise search{n, begin.subspan(0, lcp)};
   Arena arena;
-  for (;; ++range_read_iterations_accum) {
+  auto *impl = tls->impl;
+  for (;; ++tls->range_read_iterations_accum) {
     assert(getSearchPath(arena, search.n) <=>
                begin.subspan(0, lcp - search.remaining.size()) ==
            0);
     if (maxVersion(search.n, impl) <= readVersion) {
-      ++range_read_short_circuit_accum;
+      ++tls->range_read_short_circuit_accum;
       return true;
     }
     if (search.step()) {
@@ -2689,46 +2738,46 @@ bool checkRangeReadImpl(Node *n, std::span<const uint8_t> begin,
 
   if (lcp == int(begin.size())) {
     CheckRangeRightSide<kAVX512> checkRangeRightSide{n, end, lcp, readVersion,
-                                                     impl};
+                                                     tls};
     while (!checkRangeRightSide.step())
       ;
     return checkRangeRightSide.ok;
   }
 
   if (!checkRangeStartsWith<kAVX512>(n, begin.subspan(0, lcp), begin[lcp],
-                                     end[lcp], readVersion, impl)) {
+                                     end[lcp], readVersion, tls)) {
     return false;
   }
 
   CheckRangeLeftSide<kAVX512> checkRangeLeftSide{n, begin, lcp + 1, readVersion,
-                                                 impl};
+                                                 tls};
   CheckRangeRightSide<kAVX512> checkRangeRightSide{n, end, lcp + 1, readVersion,
-                                                   impl};
+                                                   tls};
 
   for (;;) {
     bool leftDone = checkRangeLeftSide.step();
     bool rightDone = checkRangeRightSide.step();
     if (!leftDone && !rightDone) {
-      range_read_iterations_accum += 2;
+      tls->range_read_iterations_accum += 2;
       continue;
     }
     if (leftDone && rightDone) {
       break;
     } else if (leftDone) {
       while (!checkRangeRightSide.step()) {
-        ++range_read_iterations_accum;
+        ++tls->range_read_iterations_accum;
       }
       break;
     } else {
       assert(rightDone);
       while (!checkRangeLeftSide.step()) {
-        ++range_read_iterations_accum;
+        ++tls->range_read_iterations_accum;
       }
     }
     break;
   }
 
-  return checkRangeLeftSide.ok & checkRangeRightSide.ok;
+  return checkRangeLeftSide.ok && checkRangeRightSide.ok;
 }
 
 #ifdef __x86_64__
@@ -2747,7 +2796,7 @@ checkMaxBetweenExclusive<true>(Node *n, int begin, int end,
 template __attribute__((target("avx512f"))) bool
 checkRangeStartsWith<true>(Node *n, std::span<const uint8_t> key, int begin,
                            int end, InternalVersionT readVersion,
-                           ConflictSet::Impl *impl);
+                           ReadContext *);
 template __attribute__((target("avx512f"))) bool
 CheckRangeLeftSide<true>::step();
 template __attribute__((target("avx512f"))) bool
@@ -2755,27 +2804,27 @@ CheckRangeRightSide<true>::step();
 template __attribute__((target("avx512f"))) bool
 checkRangeReadImpl<true>(Node *n, std::span<const uint8_t> begin,
                          std::span<const uint8_t> end,
-                         InternalVersionT readVersion, ConflictSet::Impl *impl);
+                         InternalVersionT readVersion, ReadContext *);
 #endif
 
 #if defined(__SANITIZE_THREAD__) || !defined(__x86_64__)
 bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
                     std::span<const uint8_t> end, InternalVersionT readVersion,
-                    ConflictSet::Impl *impl) {
+                    ReadContext *tls) {
-  return checkRangeReadImpl<false>(n, begin, end, readVersion, impl);
+  return checkRangeReadImpl<false>(n, begin, end, readVersion, tls);
 }
 #else
 __attribute__((target("default"))) bool
 checkRangeRead(Node *n, std::span<const uint8_t> begin,
                std::span<const uint8_t> end, InternalVersionT readVersion,
-               ConflictSet::Impl *impl) {
+               ReadContext *tls) {
-  return checkRangeReadImpl<false>(n, begin, end, readVersion, impl);
+  return checkRangeReadImpl<false>(n, begin, end, readVersion, tls);
 }
 __attribute__((target("avx512f"))) bool
 checkRangeRead(Node *n, std::span<const uint8_t> begin,
                std::span<const uint8_t> end, InternalVersionT readVersion,
-               ConflictSet::Impl *impl) {
+               ReadContext *tls) {
-  return checkRangeReadImpl<true>(n, begin, end, readVersion, impl);
+  return checkRangeReadImpl<true>(n, begin, end, readVersion, tls);
 }
 #endif
 
@@ -2784,11 +2833,11 @@ checkRangeRead(Node *n, std::span<const uint8_t> begin,
 // path of the result's parent will have `maxVersion` at least `writeVersion` as
 // a postcondition. Nodes along the search path to `key` may be invalidated.
 template <bool kBegin>
-[[nodiscard]] Node *
+[[nodiscard]] Node *insert(Node **self, std::span<const uint8_t> key,
-insert(Node **self, std::span<const uint8_t> key, InternalVersionT writeVersion,
+                           InternalVersionT writeVersion, WriteContext *tls,
-       NodeAllocators *allocators, ConflictSet::Impl *impl) {
+                           ConflictSet::Impl *impl) {
 
-  for (;;) {
+  for (;; ++tls->insert_iterations_accum) {
 
     if ((*self)->partialKeyLen > 0) {
       // Handle an existing partial key
@@ -2800,7 +2849,7 @@ insert(Node **self, std::span<const uint8_t> key, InternalVersionT writeVersion,
         InternalVersionT oldMaxVersion = maxVersion(old, impl);
 
         // *self will have one child
-        *self = allocators->node3.allocate(partialKeyIndex);
+        *self = tls->allocate<Node3>(partialKeyIndex);
 
         memcpy((char *)*self + kNodeCopyBegin, (char *)old + kNodeCopyBegin,
                kNodeCopySize);
@@ -2813,8 +2862,7 @@ insert(Node **self, std::span<const uint8_t> key, InternalVersionT writeVersion,
         memcpy((*self)->partialKey(), old->partialKey(),
                (*self)->partialKeyLen);
 
-        getOrCreateChild(*self, old->partialKey()[partialKeyIndex],
+        getOrCreateChild(*self, old->partialKey()[partialKeyIndex], tls) = old;
-                         allocators) = old;
         old->parent = *self;
         old->parentsIndex = old->partialKey()[partialKeyIndex];
         setMaxVersion(old, impl, oldMaxVersion);
@@ -2854,9 +2902,9 @@ insert(Node **self, std::span<const uint8_t> key, InternalVersionT writeVersion,
       setMaxVersion(*self, impl, writeVersion);
     }
 
-    auto &child = getOrCreateChild(*self, key.front(), allocators);
+    auto &child = getOrCreateChild(*self, key.front(), tls);
     if (!child) {
-      child = allocators->node0.allocate(key.size() - 1);
+      child = tls->allocate<Node0>(key.size() - 1);
       child->numChildren = 0;
       child->entryPresent = false;
       child->partialKeyLen = 0;
@@ -2898,10 +2946,11 @@ void destroyTree(Node *root) {
 }
 
 void addPointWrite(Node *&root, std::span<const uint8_t> key,
-                   InternalVersionT writeVersion, NodeAllocators *allocators,
+                   InternalVersionT writeVersion, WriteContext *tls,
                    ConflictSet::Impl *impl) {
-  auto *n = insert<true>(&root, key, writeVersion, allocators, impl);
+  auto *n = insert<true>(&root, key, writeVersion, tls, impl);
   if (!n->entryPresent) {
+    ++tls->entries_inserted_accum;
     auto *p = nextLogical(n);
 
     addKey(n);
@@ -2920,13 +2969,13 @@ void addPointWrite(Node *&root, std::span<const uint8_t> key,
 
 void addWriteRange(Node *&root, std::span<const uint8_t> begin,
                    std::span<const uint8_t> end, InternalVersionT writeVersion,
-                   NodeAllocators *allocators, ConflictSet::Impl *impl) {
+                   WriteContext *tls, ConflictSet::Impl *impl) {
 
   int lcp = longestCommonPrefix(begin.data(), end.data(),
                                 std::min(begin.size(), end.size()));
   if (lcp == int(begin.size()) && end.size() == begin.size() + 1 &&
       end.back() == 0) {
-    return addPointWrite(root, begin, writeVersion, allocators, impl);
+    return addPointWrite(root, begin, writeVersion, tls, impl);
   }
   const bool beginIsPrefix = lcp == int(begin.size());
   auto remaining = begin.subspan(0, lcp);
@@ -2963,8 +3012,7 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
   begin = begin.subspan(consumed, begin.size() - consumed);
   end = end.subspan(consumed, end.size() - consumed);
 
-  auto *beginNode =
+  auto *beginNode = insert<true>(useAsRoot, begin, writeVersion, tls, impl);
-      insert<true>(useAsRoot, begin, writeVersion, allocators, impl);
 
   const bool insertedBegin = !beginNode->entryPresent;
 
@@ -2972,6 +3020,7 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
   beginNode->entryPresent = true;
 
   if (insertedBegin) {
+    ++tls->entries_inserted_accum;
     auto *p = nextLogical(beginNode);
     beginNode->entry.rangeVersion =
         p == nullptr ? InternalVersionT::zero
@@ -2984,7 +3033,7 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
   assert(writeVersion >= beginNode->entry.pointVersion);
   beginNode->entry.pointVersion = writeVersion;
 
-  auto *endNode = insert<false>(useAsRoot, end, writeVersion, allocators, impl);
+  auto *endNode = insert<false>(useAsRoot, end, writeVersion, tls, impl);
 
   const bool insertedEnd = !endNode->entryPresent;
 
@@ -2992,6 +3041,7 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
   endNode->entryPresent = true;
 
   if (insertedEnd) {
+    ++tls->entries_inserted_accum;
     auto *p = nextLogical(endNode);
     endNode->entry.pointVersion =
         p == nullptr ? InternalVersionT::zero
@@ -3005,13 +3055,12 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
   if (beginIsPrefix && insertedEnd) {
     // beginNode may have been invalidated when inserting end. TODO can we do
     // better?
-    beginNode = insert<true>(useAsRoot, begin, writeVersion, allocators, impl);
+    beginNode = insert<true>(useAsRoot, begin, writeVersion, tls, impl);
     assert(beginNode->entryPresent);
   }
 
   for (beginNode = nextLogical(beginNode); beginNode != endNode;
-       beginNode =
+       beginNode = erase(beginNode, tls, impl, /*logical*/ true, endNode)) {
-           erase(beginNode, allocators, impl, /*logical*/ true, endNode)) {
   }
 }
 
@@ -3077,6 +3126,8 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     timespec ts_begin;
     clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts_begin);
 #endif
+    ReadContext tls;
+    tls.impl = this;
     int commits_accum = 0;
     int conflicts_accum = 0;
     int too_olds_accum = 0;
@@ -3092,32 +3143,32 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
           reads[i].readVersion < oldestVersionFullPrecision ? TooOld
           : (end.size() > 0
                  ? checkRangeRead(root, begin, end,
-                                  InternalVersionT(reads[i].readVersion), this)
+                                  InternalVersionT(reads[i].readVersion), &tls)
                  : checkPointRead(root, begin,
-                                  InternalVersionT(reads[i].readVersion), this))
+                                  InternalVersionT(reads[i].readVersion), &tls))
               ? Commit
               : Conflict;
       commits_accum += result[i] == Commit;
       conflicts_accum += result[i] == Conflict;
       too_olds_accum += result[i] == TooOld;
     }
-    point_read_total.add(std::exchange(point_read_accum, 0));
+    point_read_total.add(std::exchange(tls.point_read_accum, 0));
-    prefix_read_total.add(std::exchange(prefix_read_accum, 0));
+    prefix_read_total.add(std::exchange(tls.prefix_read_accum, 0));
-    range_read_total.add(std::exchange(range_read_accum, 0));
+    range_read_total.add(std::exchange(tls.range_read_accum, 0));
     range_read_node_scan_total.add(
-        std::exchange(range_read_node_scan_accum, 0));
+        std::exchange(tls.range_read_node_scan_accum, 0));
     point_read_short_circuit_total.add(
-        std::exchange(point_read_short_circuit_accum, 0));
+        std::exchange(tls.point_read_short_circuit_accum, 0));
     prefix_read_short_circuit_total.add(
-        std::exchange(prefix_read_short_circuit_accum, 0));
+        std::exchange(tls.prefix_read_short_circuit_accum, 0));
     range_read_short_circuit_total.add(
-        std::exchange(range_read_short_circuit_accum, 0));
+        std::exchange(tls.range_read_short_circuit_accum, 0));
     point_read_iterations_total.add(
-        std::exchange(point_read_iterations_accum, 0));
+        std::exchange(tls.point_read_iterations_accum, 0));
     prefix_read_iterations_total.add(
-        std::exchange(prefix_read_iterations_accum, 0));
+        std::exchange(tls.prefix_read_iterations_accum, 0));
     range_read_iterations_total.add(
-        std::exchange(range_read_iterations_accum, 0));
+        std::exchange(tls.range_read_iterations_accum, 0));
     commits_total.add(commits_accum);
     conflicts_total.add(conflicts_accum);
     too_olds_total.add(too_olds_accum);
@@ -3140,6 +3191,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     }
 
     newestVersionFullPrecision = writeVersion;
+    newest_version.set(newestVersionFullPrecision);
     setOldestVersion(
         std::max(oldestVersionFullPrecision,
                  newestVersionFullPrecision - kNominalVersionWindow));
@@ -3148,22 +3200,29 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
       gcScanStep(1000);
     }
 
+    double 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;
       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) {
         keyUpdates += 3;
-        addWriteRange(root, begin, end, InternalVersionT(writeVersion),
+        addWriteRange(root, begin, end, InternalVersionT(writeVersion), &tls,
-                      &allocators, this);
+                      this);
       } else {
         keyUpdates += 2;
-        addPointWrite(root, begin, InternalVersionT(writeVersion), &allocators,
+        addPointWrite(root, begin, InternalVersionT(writeVersion), &tls, this);
-                      this);
       }
     }
 
     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));
+    insert_iterations_total.add(std::exchange(tls.insert_iterations_accum, 0));
+    write_bytes_total.add(write_byte_accum);
   }
 
   // Spends up to `fuel` gc'ing, and returns its unused fuel. Reclaims memory
@@ -3178,7 +3237,8 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
       rootMaxVersion = std::max(rootMaxVersion, oldestVersion);
       n = nextPhysical(n);
     }
-    for (; fuel > 0 && n != nullptr;) {
+    double set_oldest_iterations_accum = 0;
+    for (; fuel > 0 && n != nullptr; ++set_oldest_iterations_accum) {
       rezero(n, oldestVersion);
       // The "make sure gc keeps up with writes" calculations assume that we're
       // scanning key by key, not node by node. Make sure we only spend fuel
@@ -3193,12 +3253,13 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         // node is greater than the point version of the left node
         assert(n->entry.rangeVersion <= oldestVersion);
         Node *dummy = nullptr;
-        n = erase(n, &allocators, this, /*logical*/ false, dummy);
+        n = erase(n, &tls, this, /*logical*/ false, dummy);
       } else {
-        maybeDecreaseCapacity(n, &allocators, this);
+        maybeDecreaseCapacity(n, &tls, this);
         n = nextPhysical(n);
       }
     }
+    gc_iterations_total.add(set_oldest_iterations_accum);
     if (n == nullptr) {
       removalKey = {};
       oldestExtantVersion = oldestVersionAtGcBegin;
@@ -3235,6 +3296,11 @@ 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));
+    oldest_version.set(oldestVersionFullPrecision);
   }
 
   int64_t getBytes() const { return totalBytes; }
@@ -3243,16 +3309,18 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     this->oldestVersion = InternalVersionT(oldestVersion);
     oldestVersionFullPrecision = oldestExtantVersion = oldestVersionAtGcBegin =
         newestVersionFullPrecision = oldestVersion;
+    oldest_version.set(oldestVersionFullPrecision);
+    newest_version.set(newestVersionFullPrecision);
 
-    allocators.~NodeAllocators();
+    tls.~WriteContext();
-    new (&allocators) NodeAllocators();
+    new (&tls) WriteContext();
 
     removalKeyArena = Arena{};
     removalKey = {};
     keyUpdates = 10;
 
     // Insert ""
-    root = allocators.node0.allocate(0);
+    root = tls.allocate<Node0>(0);
     root->numChildren = 0;
     root->parent = nullptr;
     rootMaxVersion = this->oldestVersion;
@@ -3279,7 +3347,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     safe_free(metrics, metricsCount * sizeof(metrics[0]));
   }
 
-  NodeAllocators allocators;
+  WriteContext tls;
 
   Arena removalKeyArena;
   std::span<const uint8_t> removalKey;
@@ -3314,7 +3382,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 #define COUNTER(name, help)                                                    \
   Counter name { this, #name, help }
   // ==================== METRICS DEFINITIONS ====================
-  GAUGE(memory_bytes, "Total number of bytes in use");
   COUNTER(point_read_total, "Total number of point reads checked");
   COUNTER(point_read_short_circuit_total,
           "Total number of point reads that did not require a full search to "
@@ -3343,6 +3410,24 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   COUNTER(too_olds_total,
           "Total number of checks where the result is \"too old\"");
   COUNTER(check_bytes_total, "Total number of key bytes checked");
+  GAUGE(memory_bytes, "Total number of bytes in use");
+  COUNTER(nodes_allocated_total,
+          "The total number of physical tree nodes allocated");
+  COUNTER(nodes_released_total,
+          "The total number of physical tree nodes released");
+  COUNTER(insert_iterations_total,
+          "The total number of iterations of the main loop for insertion");
+  COUNTER(entries_inserted_total,
+          "The total number of entries inserted in the tree");
+  COUNTER(entries_erased_total,
+          "The total number of entries erased from the tree");
+  COUNTER(
+      gc_iterations_total,
+      "The total number of iterations of the main loop for garbage collection");
+  COUNTER(write_bytes_total, "Total number of key bytes in calls to addWrites");
+  GAUGE(oldest_version,
+        "The lowest version that doesn't result in \"TooOld\" for checks");
+  GAUGE(newest_version, "The version of the most recent call to addWrites");
 #if MEASURE_CHECK_CPU_TIME
   COUNTER(check_cpu_seconds_total,
           "Total cpu seconds spent in a call to check");

Internal.h

@@ -578,7 +578,8 @@ inline const char *resultToStr(ConflictSet::Result r) {
 
 namespace {
 
-template <class ConflictSetImpl> struct TestDriver {
+template <class ConflictSetImpl, bool kEnableAssertions = true>
+struct TestDriver {
   Arbitrary arbitrary;
   explicit TestDriver(const uint8_t *data, size_t size)
       : arbitrary({data, size}) {}
@@ -724,10 +725,14 @@ template <class ConflictSetImpl> struct TestDriver {
       cs.addWrites(writes, numPointWrites + numRangeWrites, v);
       CALLGRIND_STOP_INSTRUMENTATION;
 
-      refImpl.addWrites(writes, numPointWrites + numRangeWrites, v);
+      if constexpr (kEnableAssertions) {
+        refImpl.addWrites(writes, numPointWrites + numRangeWrites, v);
+      }
 
       cs.setOldestVersion(oldestVersion);
-      refImpl.setOldestVersion(oldestVersion);
+      if constexpr (kEnableAssertions) {
+        refImpl.setOldestVersion(oldestVersion);
+      }
 
 #ifdef THREAD_TEST
       thread2.join();
@@ -821,16 +826,18 @@ template <class ConflictSetImpl> struct TestDriver {
       cs.check(reads, results1, numPointReads + numRangeReads);
       CALLGRIND_STOP_INSTRUMENTATION;
 
-      // Call remaining const methods
+      if constexpr (kEnableAssertions) {
-      cs.getBytes();
+        // Call remaining const methods
-      ConflictSet::MetricsV1 *m;
+        cs.getBytes();
-      int count;
+        ConflictSet::MetricsV1 *m;
-      cs.getMetricsV1(&m, &count);
+        int count;
-      for (int i = 0; i < count; ++i) {
+        cs.getMetricsV1(&m, &count);
-        m[i].getValue();
+        for (int i = 0; i < count; ++i) {
-      }
+          m[i].getValue();
+        }
 
-      refImpl.check(reads, results2, numPointReads + numRangeReads);
+        refImpl.check(reads, results2, numPointReads + numRangeReads);
+      }
 
       auto compareResults = [reads](ConflictSet::Result *results1,
                                     ConflictSet::Result *results2, int count) {
@@ -857,9 +864,12 @@ template <class ConflictSetImpl> struct TestDriver {
         return true;
       };
 
-      if (!compareResults(results1, results2, numPointReads + numRangeReads)) {
+      if constexpr (kEnableAssertions) {
-        ok = false;
+        if (!compareResults(results1, results2,
-        return true;
+                            numPointReads + numRangeReads)) {
+          ok = false;
+          return true;
+        }
       }
 
 #ifdef THREAD_TEST

TestDriver.cpp

@@ -3,13 +3,18 @@
 #include <fstream>
 #include <sstream>
 
+#ifndef PERF_TEST
+#define PERF_TEST 0
+#endif
+
 int main(int argc, char **argv) {
   for (int i = 1; i < argc; ++i) {
     std::ifstream t(argv[i], std::ios::binary);
     std::stringstream buffer;
     buffer << t.rdbuf();
     auto str = buffer.str();
-    TestDriver<ConflictSet> driver{(const uint8_t *)str.data(), str.size()};
+    TestDriver<ConflictSet, !PERF_TEST> driver{(const uint8_t *)str.data(),
+                                               str.size()};
     while (!driver.next())
       ;
     if (!driver.ok) {

include/ConflictSet.h

@@ -91,7 +91,7 @@ struct __attribute__((__visibility__("default"))) ConflictSet {
   /** Experimental! */
   struct MetricsV1 {
     /** A null-terminated string with static lifetime. Identifies this metric.
-     * Matches the regex [a-zA-Z_:][a-zA-Z0-9_:]*
+     * Matches the regex [a-zA-Z][a-zA-Z0-9_]*
      */
     const char *name;
     /** A null-terminated string with static lifetime. Describes this metric.