Attempt to fix likely arm bug

It seems all callers ultimately want this

CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.18)
 project(
   conflict-set
-  VERSION 0.0.9
+  VERSION 0.0.10
   DESCRIPTION
     "A data structure for optimistic concurrency control on ranges of bitwise-lexicographically-ordered keys."
   HOMEPAGE_URL "https://git.weaselab.dev/weaselab/conflict-set"
@@ -19,12 +19,6 @@ include(CheckCXXSourceCompiles)
 
 set(DEFAULT_BUILD_TYPE "Release")
 
-if(EMSCRIPTEN OR CMAKE_SYSTEM_NAME STREQUAL WASI)
-  set(WASM ON)
-else()
-  set(WASM OFF)
-endif()
-
 if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
   message(
     STATUS
@@ -81,15 +75,6 @@ if(EMSCRIPTEN)
 endif()
 
 if(NOT USE_SIMD_FALLBACK)
-  cmake_push_check_state()
-  list(APPEND CMAKE_REQUIRED_FLAGS -msimd128)
-  check_include_file_cxx("wasm_simd128.h" HAS_WASM_SIMD)
-  if(HAS_WASM_SIMD)
-    add_compile_options(-msimd128)
-    add_compile_definitions(HAS_WASM_SIMD)
-  endif()
-  cmake_pop_check_state()
-
   cmake_push_check_state()
   list(APPEND CMAKE_REQUIRED_FLAGS -mavx)
   check_include_file_cxx("immintrin.h" HAS_AVX)
@@ -162,47 +147,40 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
 
   file(GLOB CORPUS_TESTS ${CMAKE_CURRENT_SOURCE_DIR}/corpus/*)
 
-  # extra testing that relies on shared libraries, which aren't available with
-  # wasm
-  if(NOT WASM)
-    # Shared library version of FoundationDB's skip list implementation
-    add_library(skip_list SHARED SkipList.cpp)
-    target_compile_options(skip_list PRIVATE -fno-exceptions
-                                             -fvisibility=hidden)
-    target_include_directories(skip_list
-                               PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/include)
-    set_target_properties(
-      skip_list PROPERTIES LIBRARY_OUTPUT_DIRECTORY
-                           "${CMAKE_CURRENT_BINARY_DIR}/skip_list")
-    set_target_properties(skip_list PROPERTIES OUTPUT_NAME ${PROJECT_NAME})
-    set_target_properties(
-      skip_list PROPERTIES VERSION ${PROJECT_VERSION} SOVERSION
-                                                      ${PROJECT_VERSION_MAJOR})
+  # Shared library version of FoundationDB's skip list implementation
+  add_library(skip_list SHARED SkipList.cpp)
+  target_compile_options(skip_list PRIVATE -fno-exceptions -fvisibility=hidden)
+  target_include_directories(skip_list
+                             PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/include)
+  set_target_properties(
+    skip_list PROPERTIES LIBRARY_OUTPUT_DIRECTORY
+                         "${CMAKE_CURRENT_BINARY_DIR}/skip_list")
+  set_target_properties(skip_list PROPERTIES OUTPUT_NAME ${PROJECT_NAME})
+  set_target_properties(skip_list PROPERTIES VERSION ${PROJECT_VERSION}
+                                             SOVERSION ${PROJECT_VERSION_MAJOR})
 
-    # Shared library version of a std::unordered_map-based conflict set (point
-    # queries only)
-    add_library(hash_table SHARED HashTable.cpp)
-    target_compile_options(hash_table PRIVATE -fno-exceptions
-                                              -fvisibility=hidden)
-    target_include_directories(hash_table
-                               PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/include)
-    set_target_properties(
-      hash_table PROPERTIES LIBRARY_OUTPUT_DIRECTORY
-                            "${CMAKE_CURRENT_BINARY_DIR}/hash_table")
-    set_target_properties(hash_table PROPERTIES OUTPUT_NAME ${PROJECT_NAME})
-    set_target_properties(
-      hash_table PROPERTIES VERSION ${PROJECT_VERSION} SOVERSION
-                                                       ${PROJECT_VERSION_MAJOR})
+  # Shared library version of a std::unordered_map-based conflict set (point
+  # queries only)
+  add_library(hash_table SHARED HashTable.cpp)
+  target_compile_options(hash_table PRIVATE -fno-exceptions -fvisibility=hidden)
+  target_include_directories(hash_table
+                             PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/include)
+  set_target_properties(
+    hash_table PROPERTIES LIBRARY_OUTPUT_DIRECTORY
+                          "${CMAKE_CURRENT_BINARY_DIR}/hash_table")
+  set_target_properties(hash_table PROPERTIES OUTPUT_NAME ${PROJECT_NAME})
+  set_target_properties(
+    hash_table PROPERTIES VERSION ${PROJECT_VERSION} SOVERSION
+                                                     ${PROJECT_VERSION_MAJOR})
 
-    add_executable(driver_skip_list TestDriver.cpp)
-    target_compile_options(driver_skip_list PRIVATE ${TEST_FLAGS})
-    target_link_libraries(driver_skip_list PRIVATE skip_list)
+  add_executable(driver_skip_list TestDriver.cpp)
+  target_compile_options(driver_skip_list PRIVATE ${TEST_FLAGS})
+  target_link_libraries(driver_skip_list PRIVATE skip_list)
 
-    foreach(TEST ${CORPUS_TESTS})
-      get_filename_component(hash ${TEST} NAME)
-      add_test(NAME skip_list_${hash} COMMAND driver_skip_list ${TEST})
-    endforeach()
-  endif()
+  foreach(TEST ${CORPUS_TESTS})
+    get_filename_component(hash ${TEST} NAME)
+    add_test(NAME skip_list_${hash} COMMAND driver_skip_list ${TEST})
+  endforeach()
 
   # ad hoc testing
   add_executable(conflict_set_main ConflictSet.cpp)
@@ -320,7 +298,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
   add_test(NAME conflict_set_cxx_api_test COMMAND conflict_set_cxx_api_test)
 
   # symbol visibility tests
-  if(NOT WASM AND NOT CMAKE_BUILD_TYPE STREQUAL Debug)
+  if(NOT CMAKE_BUILD_TYPE STREQUAL Debug)
     if(APPLE)
       set(symbol_exports ${CMAKE_CURRENT_SOURCE_DIR}/apple-symbol-exports.txt)
       set(symbol_imports ${CMAKE_CURRENT_SOURCE_DIR}/apple-symbol-imports.txt)

ConflictSet.cpp

@@ -18,6 +18,7 @@ limitations under the License.
 #include "Internal.h"
 
 #include <algorithm>
+#include <atomic>
 #include <bit>
 #include <cassert>
 #include <compare>
@@ -552,7 +553,7 @@ struct Metric {
   const char *name;
   const char *help;
   ConflictSet::MetricsV1::Type type;
-  std::atomic<double> value;
+  std::atomic<int64_t> value;
 
 protected:
   Metric(ConflictSet::Impl *impl, const char *name, const char *help,
@@ -563,7 +564,7 @@ struct Gauge : private Metric {
   Gauge(ConflictSet::Impl *impl, const char *name, const char *help)
       : Metric(impl, name, help, ConflictSet::MetricsV1::Gauge) {}
 
-  void set(double value) {
+  void set(int64_t value) {
     this->value.store(value, std::memory_order_relaxed);
   }
 };
@@ -573,17 +574,10 @@ struct Counter : private Metric {
       : Metric(impl, name, help, ConflictSet::MetricsV1::Counter) {}
   // Expensive. Accumulate locally and then call add instead of repeatedly
   // calling add.
-  void add(double value) {
+  void add(int64_t value) {
     assert(value >= 0);
-    static_assert(std::atomic<double>::is_always_lock_free);
-    double old = this->value.load(std::memory_order_relaxed);
-    for (;;) {
-      double newVal = old + value;
-      if (this->value.compare_exchange_weak(old, newVal,
-                                            std::memory_order_relaxed)) {
-        break;
-      }
-    }
+    static_assert(std::atomic<int64_t>::is_always_lock_free);
+    this->value.fetch_add(value, std::memory_order_relaxed);
   }
 };
 
@@ -708,29 +702,36 @@ size_t Node::size() const {
 // 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;
+  int64_t point_read_accum = 0;
+  int64_t prefix_read_accum = 0;
+  int64_t range_read_accum = 0;
+  int64_t point_read_short_circuit_accum = 0;
+  int64_t prefix_read_short_circuit_accum = 0;
+  int64_t range_read_short_circuit_accum = 0;
+  int64_t point_read_iterations_accum = 0;
+  int64_t prefix_read_iterations_accum = 0;
+  int64_t range_read_iterations_accum = 0;
+  int64_t 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;
+
+  struct Accum {
+    int64_t entries_erased = 0;
+    int64_t insert_iterations = 0;
+    int64_t entries_inserted = 0;
+    int64_t nodes_allocated = 0;
+    int64_t nodes_released = 0;
+    int64_t point_writes = 0;
+    int64_t range_writes = 0;
+    int64_t write_bytes = 0;
+  } accum;
+
   template <class T> T *allocate(int c) {
-    ++nodes_allocated_accum;
+    ++accum.nodes_allocated;
     if constexpr (std::is_same_v<T, Node0>) {
       return node0.allocate(c);
     } else if constexpr (std::is_same_v<T, Node3>) {
@@ -745,7 +746,7 @@ struct WriteContext {
   }
   template <class T> void release(T *c) {
     static_assert(!std::is_same_v<T, Node>);
-    ++nodes_released_accum;
+    ++accum.nodes_released;
     if constexpr (std::is_same_v<T, Node0>) {
       return node0.release(c);
     } else if constexpr (std::is_same_v<T, Node3>) {
@@ -843,28 +844,41 @@ template <class NodeT> int getNodeIndex(NodeT *self, uint8_t index) {
 #endif
 }
 
+// Precondition - an entry for index must exist in the node
+Node *&getChildExists(Node3 *self, uint8_t index) {
+  return self->children[getNodeIndex(self, index)];
+}
+// Precondition - an entry for index must exist in the node
+Node *&getChildExists(Node16 *self, uint8_t index) {
+  return self->children[getNodeIndex(self, index)];
+}
+// Precondition - an entry for index must exist in the node
+Node *&getChildExists(Node48 *self, uint8_t index) {
+  assert(self->bitSet.test(index));
+  return self->children[self->index[index]];
+}
+// Precondition - an entry for index must exist in the node
+Node *&getChildExists(Node256 *self, uint8_t index) {
+  assert(self->bitSet.test(index));
+  return self->children[index];
+}
+
 // Precondition - an entry for index must exist in the node
 Node *&getChildExists(Node *self, uint8_t index) {
   switch (self->getType()) {
   case Type_Node0:           // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   case Type_Node3: {
-    auto *self3 = static_cast<Node3 *>(self);
-    return self3->children[getNodeIndex(self3, index)];
+    return getChildExists(static_cast<Node3 *>(self), index);
   }
   case Type_Node16: {
-    auto *self16 = static_cast<Node16 *>(self);
-    return self16->children[getNodeIndex(self16, index)];
+    return getChildExists(static_cast<Node16 *>(self), index);
   }
   case Type_Node48: {
-    auto *self48 = static_cast<Node48 *>(self);
-    assert(self48->bitSet.test(index));
-    return self48->children[self48->index[index]];
+    return getChildExists(static_cast<Node48 *>(self), index);
   }
   case Type_Node256: {
-    auto *self256 = static_cast<Node256 *>(self);
-    assert(self256->bitSet.test(index));
-    return self256->children[index];
+    return getChildExists(static_cast<Node256 *>(self), index);
   }
   default:                   // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
@@ -877,35 +891,39 @@ void setMaxVersion(Node *n, ConflictSet::Impl *, InternalVersionT maxVersion);
 
 Node *&getInTree(Node *n, ConflictSet::Impl *);
 
+Node *getChild(Node0 *, uint8_t) { return nullptr; }
+Node *getChild(Node3 *self, uint8_t index) {
+  int i = getNodeIndex(self, index);
+  return i < 0 ? nullptr : self->children[i];
+}
+Node *getChild(Node16 *self, uint8_t index) {
+  int i = getNodeIndex(self, index);
+  return i < 0 ? nullptr : self->children[i];
+}
+Node *getChild(Node48 *self, uint8_t index) {
+  int i = self->index[index];
+  return i < 0 ? nullptr : self->children[i];
+}
+Node *getChild(Node256 *self, uint8_t index) { return self->children[index]; }
+
 Node *getChild(Node *self, uint8_t index) {
   switch (self->getType()) {
   case Type_Node0:
-    return nullptr;
-  case Type_Node3: {
-    auto *self3 = static_cast<Node3 *>(self);
-    int i = getNodeIndex(self3, index);
-    return i < 0 ? nullptr : self3->children[i];
-  }
-  case Type_Node16: {
-    auto *self16 = static_cast<Node16 *>(self);
-    int i = getNodeIndex(self16, index);
-    return i < 0 ? nullptr : self16->children[i];
-  }
-  case Type_Node48: {
-    auto *self48 = static_cast<Node48 *>(self);
-    int i = self48->index[index];
-    return i < 0 ? nullptr : self48->children[i];
-  }
-  case Type_Node256: {
-    auto *self256 = static_cast<Node256 *>(self);
-    return self256->children[index];
-  }
+    return getChild(static_cast<Node0 *>(self), index);
+  case Type_Node3:
+    return getChild(static_cast<Node3 *>(self), index);
+  case Type_Node16:
+    return getChild(static_cast<Node16 *>(self), index);
+  case Type_Node48:
+    return getChild(static_cast<Node48 *>(self), index);
+  case Type_Node256:
+    return getChild(static_cast<Node256 *>(self), index);
   default:                   // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   }
 }
 
-template <class NodeT> int getChildGeqSimd(NodeT *self, int child) {
+template <class NodeT> Node *getChildGeqSimd(NodeT *self, int child) {
   static_assert(std::is_same_v<NodeT, Node3> || std::is_same_v<NodeT, Node16>);
 
   // cachegrind says the plain loop is fewer instructions and more mis-predicted
@@ -916,10 +934,13 @@ template <class NodeT> int getChildGeqSimd(NodeT *self, int child) {
     Node3 *n = (Node3 *)self;
     for (int i = 0; i < n->numChildren; ++i) {
       if (n->index[i] >= child) {
-        return n->index[i];
+        return n->children[i];
       }
     }
-    return -1;
+    return nullptr;
+  }
+  if (child > 255) {
+    return nullptr;
   }
 
 #ifdef HAS_AVX
@@ -929,16 +950,7 @@ template <class NodeT> int getChildGeqSimd(NodeT *self, int child) {
   __m128i results = _mm_cmpeq_epi8(key_vec, _mm_min_epu8(key_vec, indices));
   int mask = (1 << self->numChildren) - 1;
   uint32_t bitfield = _mm_movemask_epi8(results) & mask;
-  int result = bitfield == 0 ? -1 : self->index[std::countr_zero(bitfield)];
-  assert(result == [&]() -> int {
-    for (int i = 0; i < self->numChildren; ++i) {
-      if (self->index[i] >= child) {
-        return self->index[i];
-      }
-    }
-    return -1;
-  }());
-  return result;
+  return bitfield == 0 ? nullptr : self->children[std::countr_zero(bitfield)];
 #elif defined(HAS_ARM_NEON)
   uint8x16_t indices;
   memcpy(&indices, self->index, sizeof(self->index));
@@ -955,47 +967,92 @@ template <class NodeT> int getChildGeqSimd(NodeT *self, int child) {
           vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(results), 4)),
           0) &
       mask;
-  int simd = bitfield == 0 ? -1 : self->index[std::countr_zero(bitfield) / 4];
-  assert(simd == [&]() -> int {
-    for (int i = 0; i < self->numChildren; ++i) {
-      if (self->index[i] >= child) {
-        return self->index[i];
-      }
-    }
-    return -1;
-  }());
-  return simd;
+  return bitfield == 0 ? nullptr
+                       : self->children[std::countr_zero(bitfield) / 4];
 #else
   for (int i = 0; i < self->numChildren; ++i) {
     if (i > 0) {
       assert(self->index[i - 1] < self->index[i]);
     }
     if (self->index[i] >= child) {
-      return self->index[i];
+      return self->children[i];
     }
   }
-  return -1;
+  return nullptr;
 #endif
 }
 
-int getChildGeq(Node *self, int child) {
-  if (child > 255) {
-    return -1;
+Node *getChildGeq(Node0 *, int) { return nullptr; }
+Node *getChildGeq(Node3 *self, int child) {
+  return getChildGeqSimd(self, child);
+}
+Node *getChildGeq(Node16 *self, int child) {
+  return getChildGeqSimd(self, child);
+}
+Node *getChildGeq(Node48 *self, int child) {
+  int c = self->bitSet.firstSetGeq(child);
+  if (c < 0) {
+    return nullptr;
   }
+  return self->children[self->index[c]];
+}
+Node *getChildGeq(Node256 *self, int child) {
+  int c = self->bitSet.firstSetGeq(child);
+  if (c < 0) {
+    return nullptr;
+  }
+  return self->children[c];
+}
+
+Node *getChildGeq(Node *self, int child) {
   switch (self->getType()) {
   case Type_Node0:
-    return -1;
+    return getChildGeq(static_cast<Node0 *>(self), child);
   case Type_Node3:
-    return getChildGeqSimd(static_cast<Node3 *>(self), child);
+    return getChildGeq(static_cast<Node3 *>(self), child);
   case Type_Node16:
-    return getChildGeqSimd(static_cast<Node16 *>(self), child);
+    return getChildGeq(static_cast<Node16 *>(self), child);
   case Type_Node48:
-    [[fallthrough]];
-  case Type_Node256: {
-    static_assert(offsetof(Node48, bitSet) == offsetof(Node256, bitSet));
-    auto *self48 = static_cast<Node48 *>(self);
-    return self48->bitSet.firstSetGeq(child);
+    return getChildGeq(static_cast<Node48 *>(self), child);
+  case Type_Node256:
+    return getChildGeq(static_cast<Node256 *>(self), child);
+  default:                   // GCOVR_EXCL_LINE
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
   }
+}
+
+// Precondition: self has a child
+Node *getFirstChildExists(Node3 *self) {
+  assert(self->numChildren > 0);
+  return self->children[0];
+}
+// Precondition: self has a child
+Node *getFirstChildExists(Node16 *self) {
+  assert(self->numChildren > 0);
+  return self->children[0];
+}
+// Precondition: self has a child
+Node *getFirstChildExists(Node48 *self) {
+  return self->children[self->index[self->bitSet.firstSetGeq(0)]];
+}
+// Precondition: self has a child
+Node *getFirstChildExists(Node256 *self) {
+  return self->children[self->bitSet.firstSetGeq(0)];
+}
+
+// Precondition: self has a child
+Node *getFirstChildExists(Node *self) {
+  switch (self->getType()) {
+  case Type_Node0:           // GCOVR_EXCL_LINE
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  case Type_Node3:
+    return getFirstChildExists(static_cast<Node3 *>(self));
+  case Type_Node16:
+    return getFirstChildExists(static_cast<Node16 *>(self));
+  case Type_Node48:
+    return getFirstChildExists(static_cast<Node48 *>(self));
+  case Type_Node256:
+    return getFirstChildExists(static_cast<Node256 *>(self));
   default:                   // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   }
@@ -1201,8 +1258,8 @@ Node *nextPhysical(Node *node) {
   int index = -1;
   for (;;) {
     auto nextChild = getChildGeq(node, index + 1);
-    if (nextChild >= 0) {
-      return getChildExists(node, nextChild);
+    if (nextChild != nullptr) {
+      return nextChild;
     }
     index = node->parentsIndex;
     node = node->parent;
@@ -1458,7 +1515,7 @@ void maybeDownsize(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
 // will update it to its new pointee as well. Precondition: `self->entryPresent`
 Node *erase(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
             bool logical, Node *&dontInvalidate) {
-  ++tls->entries_erased_accum;
+  ++tls->accum.entries_erased;
   assert(self->parent != nullptr);
 
 #if DEBUG_VERBOSE && !defined(NDEBUG)
@@ -1580,21 +1637,16 @@ Node *erase(Node *self, WriteContext *tls, ConflictSet::Impl *impl,
   return result;
 }
 
-struct Iterator {
-  Node *n;
-  int cmp;
-};
-
 Node *nextSibling(Node *node) {
   for (;;) {
     if (node->parent == nullptr) {
       return nullptr;
     }
     auto next = getChildGeq(node->parent, node->parentsIndex + 1);
-    if (next < 0) {
+    if (next == nullptr) {
       node = node->parent;
     } else {
-      return getChildExists(node->parent, next);
+      return next;
     }
   }
 }
@@ -1787,17 +1839,15 @@ bool checkPointRead(Node *n, const std::span<const uint8_t> key,
       if (n->entryPresent) {
         return n->entry.pointVersion <= readVersion;
       }
-      int c = getChildGeq(n, 0);
-      assert(c >= 0);
-      n = getChildExists(n, c);
+      n = getFirstChildExists(n);
       goto downLeftSpine;
     }
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
-        n = getChildExists(n, c);
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
+        n = c;
         goto downLeftSpine;
       } else {
         n = nextSibling(n);
@@ -1837,14 +1887,9 @@ bool checkPointRead(Node *n, const std::span<const uint8_t> key,
     }
   }
 downLeftSpine:
-  for (;;) {
-    if (n->entryPresent) {
-      return n->entry.rangeVersion <= readVersion;
-    }
-    int c = getChildGeq(n, 0);
-    assert(c >= 0);
-    n = getChildExists(n, c);
+  for (; !n->entryPresent; n = getFirstChildExists(n)) {
   }
+  return n->entry.rangeVersion <= readVersion;
 }
 
 // Logically this is the same as performing firstGeq and then checking against
@@ -1871,9 +1916,9 @@ bool checkPrefixRead(Node *n, const std::span<const uint8_t> key,
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
-        n = getChildExists(n, c);
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
+        n = c;
         goto downLeftSpine;
       } else {
         n = nextSibling(n);
@@ -1917,14 +1962,9 @@ bool checkPrefixRead(Node *n, const std::span<const uint8_t> key,
     }
   }
 downLeftSpine:
-  for (;;) {
-    if (n->entryPresent) {
-      return n->entry.rangeVersion <= readVersion;
-    }
-    int c = getChildGeq(n, 0);
-    assert(c >= 0);
-    n = getChildExists(n, c);
+  for (; !n->entryPresent; n = getFirstChildExists(n)) {
   }
+  return n->entry.rangeVersion <= readVersion;
 }
 
 #ifdef HAS_AVX
@@ -2377,9 +2417,9 @@ bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
 
   auto *child = getChild(n, remaining[0]);
   if (child == nullptr) {
-    int c = getChildGeq(n, remaining[0]);
-    if (c >= 0) {
-      n = getChildExists(n, c);
+    auto c = getChildGeq(n, remaining[0]);
+    if (c != nullptr) {
+      n = c;
       goto downLeftSpine;
     } else {
       n = nextSibling(n);
@@ -2423,14 +2463,9 @@ bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
   __builtin_unreachable(); // GCOVR_EXCL_LINE
 
 downLeftSpine:
-  for (;;) {
-    if (n->entryPresent) {
-      return n->entry.rangeVersion <= readVersion;
-    }
-    int c = getChildGeq(n, 0);
-    assert(c >= 0);
-    n = getChildExists(n, c);
+  for (; !n->entryPresent; n = getFirstChildExists(n)) {
   }
+  return n->entry.rangeVersion <= readVersion;
 }
 
 namespace {
@@ -2478,13 +2513,13 @@ template <bool kAVX512> struct CheckRangeLeftSide {
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
         if (searchPathLen < prefixLen) {
-          n = getChildExists(n, c);
+          n = c;
           return downLeftSpine();
         }
-        n = getChildExists(n, c);
+        n = c;
         ok = maxVersion(n, impl) <= readVersion;
         return true;
       } else {
@@ -2543,15 +2578,10 @@ template <bool kAVX512> struct CheckRangeLeftSide {
   }
 
   bool downLeftSpine() {
-    for (;;) {
-      if (n->entryPresent) {
-        ok = n->entry.rangeVersion <= readVersion;
-        return true;
-      }
-      int c = getChildGeq(n, 0);
-      assert(c >= 0);
-      n = getChildExists(n, c);
+    for (; !n->entryPresent; n = getFirstChildExists(n)) {
     }
+    ok = n->entry.rangeVersion <= readVersion;
+    return true;
   }
 };
 
@@ -2615,9 +2645,9 @@ template <bool kAVX512> struct CheckRangeRightSide {
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
-        n = getChildExists(n, c);
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
+        n = c;
         return downLeftSpine();
       } else {
         return backtrack();
@@ -2667,12 +2697,12 @@ template <bool kAVX512> struct CheckRangeRightSide {
         return true;
       }
       auto next = getChildGeq(n->parent, n->parentsIndex + 1);
-      if (next < 0) {
+      if (next == nullptr) {
         searchPathLen -= 1 + n->partialKeyLen;
         n = n->parent;
       } else {
         searchPathLen -= n->partialKeyLen;
-        n = getChildExists(n->parent, next);
+        n = next;
         searchPathLen += n->partialKeyLen;
         return downLeftSpine();
       }
@@ -2680,15 +2710,10 @@ template <bool kAVX512> struct CheckRangeRightSide {
   }
 
   bool downLeftSpine() {
-    for (;;) {
-      if (n->entryPresent) {
-        ok = n->entry.rangeVersion <= readVersion;
-        return true;
-      }
-      int c = getChildGeq(n, 0);
-      assert(c >= 0);
-      n = getChildExists(n, c);
+    for (; !n->entryPresent; n = getFirstChildExists(n)) {
     }
+    ok = n->entry.rangeVersion <= readVersion;
+    return true;
   }
 };
 } // namespace
@@ -2838,7 +2863,7 @@ template <bool kBegin>
                            InternalVersionT writeVersion, WriteContext *tls,
                            ConflictSet::Impl *impl) {
 
-  for (;; ++tls->insert_iterations_accum) {
+  for (;; ++tls->accum.insert_iterations) {
 
     if ((*self)->partialKeyLen > 0) {
       // Handle an existing partial key
@@ -2936,9 +2961,8 @@ void destroyTree(Node *root) {
     auto *n = toFree.back();
     toFree.pop_back();
     // Add all children to toFree
-    for (int child = getChildGeq(n, 0); child >= 0;
-         child = getChildGeq(n, child + 1)) {
-      auto *c = getChildExists(n, child);
+    for (auto c = getChildGeq(n, 0); c != nullptr;
+         c = getChildGeq(n, c->parentsIndex + 1)) {
       assert(c != nullptr);
       toFree.push_back(c);
     }
@@ -2949,9 +2973,10 @@ void destroyTree(Node *root) {
 void addPointWrite(Node *&root, std::span<const uint8_t> key,
                    InternalVersionT writeVersion, WriteContext *tls,
                    ConflictSet::Impl *impl) {
+  ++tls->accum.point_writes;
   auto *n = insert<true>(&root, key, writeVersion, tls, impl);
   if (!n->entryPresent) {
-    ++tls->entries_inserted_accum;
+    ++tls->accum.entries_inserted;
     auto *p = nextLogical(n);
 
     addKey(n);
@@ -2978,6 +3003,7 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
       end.back() == 0) {
     return addPointWrite(root, begin, writeVersion, tls, impl);
   }
+  ++tls->accum.range_writes;
   const bool beginIsPrefix = lcp == int(begin.size());
   auto remaining = begin.subspan(0, lcp);
 
@@ -3021,7 +3047,7 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
   beginNode->entryPresent = true;
 
   if (insertedBegin) {
-    ++tls->entries_inserted_accum;
+    ++tls->accum.entries_inserted;
     auto *p = nextLogical(beginNode);
     beginNode->entry.rangeVersion =
         p == nullptr ? InternalVersionT::zero
@@ -3042,7 +3068,7 @@ void addWriteRange(Node *&root, std::span<const uint8_t> begin,
   endNode->entryPresent = true;
 
   if (insertedEnd) {
-    ++tls->entries_inserted_accum;
+    ++tls->accum.entries_inserted;
     auto *p = nextLogical(endNode);
     endNode->entry.pointVersion =
         p == nullptr ? InternalVersionT::zero
@@ -3074,9 +3100,9 @@ Node *firstGeqPhysical(Node *n, const std::span<const uint8_t> key) {
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
-        n = getChildExists(n, c);
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
+        n = c;
         return n;
       } else {
         n = nextSibling(n);
@@ -3118,21 +3144,15 @@ Node *firstGeqPhysical(Node *n, const std::span<const uint8_t> key) {
   }
 }
 
-#define MEASURE_CHECK_CPU_TIME 0
-
 struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 
   void check(const ReadRange *reads, Result *result, int count) {
-#if MEASURE_CHECK_CPU_TIME
-    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;
-    double check_byte_accum = 0;
+    int64_t check_byte_accum = 0;
     for (int i = 0; i < count; ++i) {
       const auto &r = reads[i];
       check_byte_accum += r.begin.len + r.end.len;
@@ -3167,13 +3187,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     conflicts_total.add(conflicts_accum);
     too_olds_total.add(too_olds_accum);
     check_bytes_total.add(check_byte_accum);
-#if MEASURE_CHECK_CPU_TIME
-    timespec ts_end;
-    clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts_end);
-    check_cpu_seconds_total.add(
-        std::max<double>(0, (ts_end.tv_nsec * 1e-9 + ts_end.tv_sec) -
-                                (ts_begin.tv_nsec * 1e-9 + ts_begin.tv_sec)));
-#endif
   }
 
   void addWrites(const WriteRange *writes, int count, int64_t writeVersion) {
@@ -3184,50 +3197,51 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 
     assert(writeVersion >= newestVersionFullPrecision);
 
-    if (writeVersion > newestVersionFullPrecision + kNominalVersionWindow) {
-      destroyTree(root);
-      init(writeVersion - kNominalVersionWindow);
+    if (oldestExtantVersion < writeVersion - kMaxCorrectVersionWindow)
+        [[unlikely]] {
+      if (writeVersion > newestVersionFullPrecision + kNominalVersionWindow) {
+        destroyTree(root);
+        init(writeVersion - kNominalVersionWindow);
+      }
+
+      newestVersionFullPrecision = writeVersion;
+      newest_version.set(newestVersionFullPrecision);
+      setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
+      while (oldestExtantVersion <
+             newestVersionFullPrecision - kMaxCorrectVersionWindow) {
+        gcScanStep(1000);
+      }
+    } else {
+      newestVersionFullPrecision = writeVersion;
+      newest_version.set(newestVersionFullPrecision);
+      setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
     }
 
-    newestVersionFullPrecision = writeVersion;
-    newest_version.set(newestVersionFullPrecision);
-    setOldestVersion(
-        std::max(oldestVersionFullPrecision,
-                 newestVersionFullPrecision - kNominalVersionWindow));
-    while (oldestExtantVersion <
-           newestVersionFullPrecision - kMaxCorrectVersionWindow) {
-      gcScanStep(1000);
-    }
-
-    double write_byte_accum = 0;
-    int point_writes_accum = 0;
-    int range_writes_accum = 0;
     for (int i = 0; i < count; ++i) {
       const auto &w = writes[i];
-      write_byte_accum += w.begin.len + w.end.len;
+      tls.accum.write_bytes += w.begin.len + w.end.len;
       auto begin = std::span<const uint8_t>(w.begin.p, w.begin.len);
       auto end = std::span<const uint8_t>(w.end.p, w.end.len);
       if (w.end.len > 0) {
         keyUpdates += 3;
-        ++range_writes_accum;
         addWriteRange(root, begin, end, InternalVersionT(writeVersion), &tls,
                       this);
       } else {
         keyUpdates += 2;
-        ++point_writes_accum;
         addPointWrite(root, begin, InternalVersionT(writeVersion), &tls, this);
       }
     }
 
     memory_bytes.set(totalBytes);
-    point_writes_total.add(point_writes_accum);
-    range_writes_total.add(range_writes_accum);
-    nodes_allocated_total.add(std::exchange(tls.nodes_allocated_accum, 0));
-    nodes_released_total.add(std::exchange(tls.nodes_released_accum, 0));
-    entries_inserted_total.add(std::exchange(tls.entries_inserted_accum, 0));
-    entries_erased_total.add(std::exchange(tls.entries_erased_accum, 0));
-    insert_iterations_total.add(std::exchange(tls.insert_iterations_accum, 0));
-    write_bytes_total.add(write_byte_accum);
+    point_writes_total.add(tls.accum.point_writes);
+    range_writes_total.add(tls.accum.range_writes);
+    nodes_allocated_total.add(tls.accum.nodes_allocated);
+    nodes_released_total.add(tls.accum.nodes_released);
+    entries_inserted_total.add(tls.accum.entries_inserted);
+    entries_erased_total.add(tls.accum.entries_erased);
+    insert_iterations_total.add(tls.accum.insert_iterations);
+    write_bytes_total.add(tls.accum.write_bytes);
+    memset(&tls.accum, 0, sizeof(tls.accum));
   }
 
   // Spends up to `fuel` gc'ing, and returns its unused fuel. Reclaims memory
@@ -3242,7 +3256,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
       rootMaxVersion = std::max(rootMaxVersion, oldestVersion);
       n = nextPhysical(n);
     }
-    double set_oldest_iterations_accum = 0;
+    int64_t 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
@@ -3302,10 +3316,10 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     keyUpdates = gcScanStep(keyUpdates);
 
     memory_bytes.set(totalBytes);
-    nodes_allocated_total.add(std::exchange(tls.nodes_allocated_accum, 0));
-    nodes_released_total.add(std::exchange(tls.nodes_released_accum, 0));
-    entries_inserted_total.add(std::exchange(tls.entries_inserted_accum, 0));
-    entries_erased_total.add(std::exchange(tls.entries_erased_accum, 0));
+    nodes_allocated_total.add(std::exchange(tls.accum.nodes_allocated, 0));
+    nodes_released_total.add(std::exchange(tls.accum.nodes_released, 0));
+    entries_inserted_total.add(std::exchange(tls.accum.entries_inserted, 0));
+    entries_erased_total.add(std::exchange(tls.accum.entries_erased, 0));
     oldest_version.set(oldestVersionFullPrecision);
   }
 
@@ -3443,10 +3457,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   GAUGE(
       oldest_extant_version,
       "A lower bound on the lowest version associated with an existing entry");
-#if MEASURE_CHECK_CPU_TIME
-  COUNTER(check_cpu_seconds_total,
-          "Total cpu seconds spent in a call to check");
-#endif
   // ==================== END METRICS DEFINITIONS ====================
 #undef GAUGE
 #undef COUNTER
@@ -3606,29 +3616,27 @@ double internal_getMetricValue(const ConflictSet::MetricsV1 *metric) {
 
 // GCOVR_EXCL_START
 
-Iterator firstGeqLogical(Node *n, const std::span<const uint8_t> key) {
+Node *firstGeqLogical(Node *n, const std::span<const uint8_t> key) {
   auto remaining = key;
   for (;;) {
     if (remaining.size() == 0) {
       if (n->entryPresent) {
-        return {n, 0};
+        return n;
       }
-      int c = getChildGeq(n, 0);
-      assert(c >= 0);
-      n = getChildExists(n, c);
+      n = getFirstChildExists(n);
       goto downLeftSpine;
     }
 
     auto *child = getChild(n, remaining[0]);
     if (child == nullptr) {
-      int c = getChildGeq(n, remaining[0]);
-      if (c >= 0) {
-        n = getChildExists(n, c);
+      auto c = getChildGeq(n, remaining[0]);
+      if (c != nullptr) {
+        n = c;
         goto downLeftSpine;
       } else {
         n = nextSibling(n);
         if (n == nullptr) {
-          return {nullptr, 1};
+          return nullptr;
         }
         goto downLeftSpine;
       }
@@ -3660,14 +3668,9 @@ Iterator firstGeqLogical(Node *n, const std::span<const uint8_t> key) {
     }
   }
 downLeftSpine:
-  for (;;) {
-    if (n->entryPresent) {
-      return {n, 1};
-    }
-    int c = getChildGeq(n, 0);
-    assert(c >= 0);
-    n = getChildExists(n, c);
+  for (; !n->entryPresent; n = getFirstChildExists(n)) {
   }
+  return n;
 }
 
 void ConflictSet::check(const ReadRange *reads, Result *results,
@@ -3841,9 +3844,8 @@ std::string getSearchPath(Node *n) {
                 getPartialKeyPrintable(n).c_str(), x, y);
       }
       x += kSeparation;
-      for (int child = getChildGeq(n, 0); child >= 0;
-           child = getChildGeq(n, child + 1)) {
-        auto *c = getChildExists(n, child);
+      for (auto c = getChildGeq(n, 0); c != nullptr;
+           c = getChildGeq(n, c->parentsIndex + 1)) {
         fprintf(file, " k_%p -> k_%p;\n", (void *)n, (void *)c);
         print(c, y - kSeparation);
       }
@@ -3862,19 +3864,19 @@ std::string getSearchPath(Node *n) {
 }
 
 void checkParentPointers(Node *node, bool &success) {
-  for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
-    auto *child = getChildExists(node, i);
+  for (auto child = getChildGeq(node, 0); child != nullptr;
+       child = getChildGeq(node, child->parentsIndex + 1)) {
     if (child->parent != node) {
       fprintf(stderr, "%s child %d has parent pointer %p. Expected %p\n",
-              getSearchPathPrintable(node).c_str(), i, (void *)child->parent,
-              (void *)node);
+              getSearchPathPrintable(node).c_str(), child->parentsIndex,
+              (void *)child->parent, (void *)node);
       success = false;
     }
     checkParentPointers(child, success);
   }
 }
 
-Iterator firstGeq(Node *n, std::string_view key) {
+Node *firstGeq(Node *n, std::string_view key) {
   return firstGeqLogical(
       n, std::span<const uint8_t>((const uint8_t *)key.data(), key.size()));
 }
@@ -3928,12 +3930,12 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
                 bool &success, ConflictSet::Impl *impl) {
   checkVersionsGeqOldestExtant(node,
                                InternalVersionT(impl->oldestExtantVersion));
-  auto expected = InternalVersionT::zero;
+  auto expected = oldestVersion;
   if (node->entryPresent) {
     expected = std::max(expected, node->entry.pointVersion);
   }
-  for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
-    auto *child = getChildExists(node, i);
+  for (auto child = getChildGeq(node, 0); child != nullptr;
+       child = getChildGeq(node, child->parentsIndex + 1)) {
     expected = std::max(
         expected, checkMaxVersion(root, child, oldestVersion, success, impl));
     if (child->entryPresent) {
@@ -3945,8 +3947,8 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
   auto inc = strinc(key, ok);
   if (ok) {
     auto borrowed = firstGeq(root, inc);
-    if (borrowed.n != nullptr) {
-      expected = std::max(expected, borrowed.n->entry.rangeVersion);
+    if (borrowed != nullptr) {
+      expected = std::max(expected, borrowed->entry.rangeVersion);
     }
   }
   if (maxVersion(node, impl) > oldestVersion &&
@@ -3961,14 +3963,14 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
 
 [[maybe_unused]] int64_t checkEntriesExist(Node *node, bool &success) {
   int64_t total = node->entryPresent;
-  for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
-    auto *child = getChildExists(node, i);
+  for (auto child = getChildGeq(node, 0); child != nullptr;
+       child = getChildGeq(node, child->parentsIndex + 1)) {
     int64_t e = checkEntriesExist(child, success);
     total += e;
     if (e == 0) {
       Arena arena;
       fprintf(stderr, "%s has child %02x with no reachable entries\n",
-              getSearchPathPrintable(node).c_str(), i);
+              getSearchPathPrintable(node).c_str(), child->parentsIndex);
       success = false;
     }
   }
@@ -4005,8 +4007,8 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
     success = false;
   }
   // TODO check that the max capacity property eventually holds
-  for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
-    auto *child = getChildExists(node, i);
+  for (auto child = getChildGeq(node, 0); child != nullptr;
+       child = getChildGeq(node, child->parentsIndex + 1)) {
     checkMemoryBoundInvariants(child, success);
   }
 }
@@ -4151,26 +4153,42 @@ int main(void) { printTree(); }
 
 #ifdef ENABLE_FUZZ
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
-  TestDriver<ConflictSet::Impl> driver{data, size};
+  Arbitrary arbitrary({data, size});
+  TestDriver<ConflictSet::Impl> driver1{arbitrary};
+  TestDriver<ConflictSet::Impl> driver2{arbitrary};
 
+  bool done1 = false;
+  bool done2 = false;
   for (;;) {
-    bool done = driver.next();
-    if (!driver.ok) {
-      debugPrintDot(stdout, driver.cs.root, &driver.cs);
-      fflush(stdout);
-      abort();
+    if (!done1) {
+      done1 = driver1.next();
+      if (!driver1.ok) {
+        debugPrintDot(stdout, driver1.cs.root, &driver1.cs);
+        fflush(stdout);
+        abort();
+      }
+      if (!checkCorrectness(driver1.cs.root, driver1.cs.oldestVersion,
+                            &driver1.cs)) {
+        debugPrintDot(stdout, driver1.cs.root, &driver1.cs);
+        fflush(stdout);
+        abort();
+      }
     }
-#if DEBUG_VERBOSE && !defined(NDEBUG)
-    fprintf(stderr, "Check correctness\n");
-#endif
-    bool success =
-        checkCorrectness(driver.cs.root, driver.cs.oldestVersion, &driver.cs);
-    if (!success) {
-      debugPrintDot(stdout, driver.cs.root, &driver.cs);
-      fflush(stdout);
-      abort();
+    if (!done2) {
+      done2 = driver2.next();
+      if (!driver2.ok) {
+        debugPrintDot(stdout, driver2.cs.root, &driver2.cs);
+        fflush(stdout);
+        abort();
+      }
+      if (!checkCorrectness(driver2.cs.root, driver2.cs.oldestVersion,
+                            &driver2.cs)) {
+        debugPrintDot(stdout, driver2.cs.root, &driver2.cs);
+        fflush(stdout);
+        abort();
+      }
     }
-    if (done) {
+    if (done1 && done2) {
       break;
     }
   }

Internal.h

@@ -580,11 +580,14 @@ namespace {
 
 template <class ConflictSetImpl, bool kEnableAssertions = true>
 struct TestDriver {
-  Arbitrary arbitrary;
-  explicit TestDriver(const uint8_t *data, size_t size)
-      : arbitrary({data, size}) {}
+  Arbitrary *arbitrary;
+  explicit TestDriver(Arbitrary &a) : arbitrary(&a) {
+#if DEBUG_VERBOSE && !defined(NDEBUG)
+    fprintf(stderr, "%p Initial version: {%" PRId64 "}\n", this, writeVersion);
+#endif
+  }
 
-  int64_t oldestVersion = arbitrary.next();
+  int64_t oldestVersion = arbitrary->next();
   int64_t writeVersion = oldestVersion;
   ConflictSetImpl cs{oldestVersion};
   ReferenceImpl refImpl{oldestVersion};
@@ -593,33 +596,34 @@ struct TestDriver {
 
   bool ok = true;
 
-  const int prefixLen = arbitrary.bounded(512);
-  const int prefixByte = arbitrary.randT<uint8_t>();
+  const int prefixLen = arbitrary->bounded(512);
+  const int prefixByte = arbitrary->randT<uint8_t>();
 
   // Call until it returns true, for "done". Check internal invariants etc
   // between calls to next.
   bool next() {
     assert(cs.getBytes() >= 0);
-    if (!arbitrary.hasEntropy()) {
+    if (!arbitrary->hasEntropy()) {
       return true;
     }
     Arena arena;
     {
-      int numPointWrites = arbitrary.bounded(100);
-      int numRangeWrites = arbitrary.bounded(100);
-      int64_t v = (writeVersion += arbitrary.bounded(10) ? arbitrary.bounded(10)
-                                                         : arbitrary.next());
+      int numPointWrites = arbitrary->bounded(100);
+      int numRangeWrites = arbitrary->bounded(100);
+      int64_t v =
+          (writeVersion +=
+           arbitrary->bounded(10) ? arbitrary->bounded(10) : arbitrary->next());
       auto *writes =
           new (arena) ConflictSet::WriteRange[numPointWrites + numRangeWrites];
       auto keys = set<std::string_view>(arena);
       while (int(keys.size()) < numPointWrites + numRangeWrites * 2) {
-        if (!arbitrary.hasEntropy()) {
+        if (!arbitrary->hasEntropy()) {
           return true;
         }
-        int keyLen = prefixLen + arbitrary.bounded(kMaxKeySuffixLen);
+        int keyLen = prefixLen + arbitrary->bounded(kMaxKeySuffixLen);
         auto *begin = new (arena) uint8_t[keyLen];
         memset(begin, prefixByte, prefixLen);
-        arbitrary.randomBytes(begin + prefixLen, keyLen - prefixLen);
+        arbitrary->randomBytes(begin + prefixLen, keyLen - prefixLen);
         keys.insert(std::string_view((const char *)begin, keyLen));
       }
 
@@ -629,7 +633,7 @@ struct TestDriver {
                rangesRemaining = numRangeWrites;
            pointsRemaining > 0 || rangesRemaining > 0; ++i) {
         bool pointRead = pointsRemaining > 0 && rangesRemaining > 0
-                             ? bool(arbitrary.bounded(2))
+                             ? bool(arbitrary->bounded(2))
                              : pointsRemaining > 0;
         if (pointRead) {
           assert(pointsRemaining > 0);
@@ -648,33 +652,20 @@ struct TestDriver {
           ++iter;
           --rangesRemaining;
         }
-#if DEBUG_VERBOSE && !defined(NDEBUG)
-        if (writes[i].end.len == 0) {
-          fprintf(stderr, "Write: {%s}\n", printable(writes[i].begin).c_str());
-        } else {
-          fprintf(stderr, "Write: [%s, %s)\n",
-                  printable(writes[i].begin).c_str(),
-                  printable(writes[i].end).c_str());
-        }
-#endif
       }
       assert(iter == keys.end());
       assert(i == numPointWrites + numRangeWrites);
 
-#if DEBUG_VERBOSE && !defined(NDEBUG)
-      fprintf(stderr, "Write @ %" PRId64 "\n", v);
-#endif
-
       // Test non-canonical writes
       if (numPointWrites > 0) {
-        int overlaps = arbitrary.bounded(numPointWrites);
+        int overlaps = arbitrary->bounded(numPointWrites);
         for (int i = 0; i < numPointWrites + numRangeWrites && overlaps > 0;
              ++i) {
           if (writes[i].end.len == 0) {
-            int keyLen = prefixLen + arbitrary.bounded(kMaxKeySuffixLen);
+            int keyLen = prefixLen + arbitrary->bounded(kMaxKeySuffixLen);
             auto *begin = new (arena) uint8_t[keyLen];
             memset(begin, prefixByte, prefixLen);
-            arbitrary.randomBytes(begin + prefixLen, keyLen - prefixLen);
+            arbitrary->randomBytes(begin + prefixLen, keyLen - prefixLen);
             writes[i].end.len = keyLen;
             writes[i].end.p = begin;
             auto c =
@@ -692,10 +683,10 @@ struct TestDriver {
           }
         }
       }
-      if (arbitrary.bounded(2)) {
+      if (arbitrary->bounded(2)) {
         // Shuffle writes
         for (int i = numPointWrites + numRangeWrites - 1; i > 0; --i) {
-          int j = arbitrary.bounded(i + 1);
+          int j = arbitrary->bounded(i + 1);
           if (i != j) {
             using std::swap;
             swap(writes[i], writes[j]);
@@ -704,7 +695,7 @@ struct TestDriver {
       }
 
       oldestVersion +=
-          arbitrary.bounded(10) ? arbitrary.bounded(10) : arbitrary.next();
+          arbitrary->bounded(10) ? arbitrary->bounded(10) : arbitrary->next();
       oldestVersion = std::min(oldestVersion, writeVersion);
 
 #ifdef THREAD_TEST
@@ -721,6 +712,20 @@ struct TestDriver {
       ready.wait();
 #endif
 
+#if DEBUG_VERBOSE && !defined(NDEBUG)
+      for (int i = 0; i < numPointWrites + numRangeWrites; ++i) {
+        if (writes[i].end.len == 0) {
+          fprintf(stderr, "%p Write: {%s}\n", this,
+                  printable(writes[i].begin).c_str());
+        } else {
+          fprintf(stderr, "%p Write: [%s, %s)\n", this,
+                  printable(writes[i].begin).c_str(),
+                  printable(writes[i].end).c_str());
+        }
+      }
+      fprintf(stderr, "%p Write @ %" PRId64 "\n", this, v);
+#endif
+
       CALLGRIND_START_INSTRUMENTATION;
       cs.addWrites(writes, numPointWrites + numRangeWrites, v);
       CALLGRIND_STOP_INSTRUMENTATION;
@@ -729,6 +734,10 @@ struct TestDriver {
         refImpl.addWrites(writes, numPointWrites + numRangeWrites, v);
       }
 
+#if DEBUG_VERBOSE && !defined(NDEBUG)
+      fprintf(stderr, "%p Set oldest version: %" PRId64 "\n", this,
+              oldestVersion);
+#endif
       cs.setOldestVersion(oldestVersion);
       if constexpr (kEnableAssertions) {
         refImpl.setOldestVersion(oldestVersion);
@@ -739,24 +748,24 @@ struct TestDriver {
 #endif
     }
     {
-      int numPointReads = arbitrary.bounded(100);
-      int numRangeReads = arbitrary.bounded(100);
+      int numPointReads = arbitrary->bounded(100);
+      int numRangeReads = arbitrary->bounded(100);
 
-      int64_t v = std::max<int64_t>(writeVersion - (arbitrary.bounded(10)
-                                                        ? arbitrary.bounded(10)
-                                                        : arbitrary.next()),
+      int64_t v = std::max<int64_t>(writeVersion - (arbitrary->bounded(10)
+                                                        ? arbitrary->bounded(10)
+                                                        : arbitrary->next()),
                                     0);
       auto *reads =
           new (arena) ConflictSet::ReadRange[numPointReads + numRangeReads];
       auto keys = set<std::string_view>(arena);
       while (int(keys.size()) < numPointReads + numRangeReads * 2) {
-        if (!arbitrary.hasEntropy()) {
+        if (!arbitrary->hasEntropy()) {
           return true;
         }
-        int keyLen = prefixLen + arbitrary.bounded(kMaxKeySuffixLen);
+        int keyLen = prefixLen + arbitrary->bounded(kMaxKeySuffixLen);
         auto *begin = new (arena) uint8_t[keyLen];
         memset(begin, prefixByte, prefixLen);
-        arbitrary.randomBytes(begin + prefixLen, keyLen - prefixLen);
+        arbitrary->randomBytes(begin + prefixLen, keyLen - prefixLen);
         keys.insert(std::string_view((const char *)begin, keyLen));
       }
 
@@ -765,7 +774,7 @@ struct TestDriver {
       for (int pointsRemaining = numPointReads, rangesRemaining = numRangeReads;
            pointsRemaining > 0 || rangesRemaining > 0; ++i) {
         bool pointRead = pointsRemaining > 0 && rangesRemaining > 0
-                             ? bool(arbitrary.bounded(2))
+                             ? bool(arbitrary->bounded(2))
                              : pointsRemaining > 0;
         if (pointRead) {
           assert(pointsRemaining > 0);
@@ -787,10 +796,10 @@ struct TestDriver {
         reads[i].readVersion = v;
 #if DEBUG_VERBOSE && !defined(NDEBUG)
         if (reads[i].end.len == 0) {
-          fprintf(stderr, "Read: {%s} @ %" PRId64 "\n",
+          fprintf(stderr, "%p Read: {%s} @ %" PRId64 "\n", this,
                   printable(reads[i].begin).c_str(), reads[i].readVersion);
         } else {
-          fprintf(stderr, "Read: [%s, %s) @ %" PRId64 "\n",
+          fprintf(stderr, "%p Read: [%s, %s) @ %" PRId64 "\n", this,
                   printable(reads[i].begin).c_str(),
                   printable(reads[i].end).c_str(), reads[i].readVersion);
         }
@@ -839,24 +848,27 @@ struct TestDriver {
         refImpl.check(reads, results2, numPointReads + numRangeReads);
       }
 
-      auto compareResults = [reads](ConflictSet::Result *results1,
-                                    ConflictSet::Result *results2, int count) {
+      auto compareResults = [reads, this](ConflictSet::Result *results1,
+                                          ConflictSet::Result *results2,
+                                          int count) {
         for (int i = 0; i < count; ++i) {
           if (results1[i] != results2[i]) {
             if (reads[i].end.len == 0) {
-              fprintf(stderr,
-                      "Expected %s, got %s for read of {%s} at version %" PRId64
-                      "\n",
-                      resultToStr(results2[i]), resultToStr(results1[i]),
-                      printable(reads[i].begin).c_str(), reads[i].readVersion);
-            } else {
               fprintf(
                   stderr,
-                  "Expected %s, got %s for read of [%s, %s) at version %" PRId64
+                  "%p Expected %s, got %s for read of {%s} at version %" PRId64
                   "\n",
-                  resultToStr(results2[i]), resultToStr(results1[i]),
-                  printable(reads[i].begin).c_str(),
-                  printable(reads[i].end).c_str(), reads[i].readVersion);
+                  (void *)this, resultToStr(results2[i]),
+                  resultToStr(results1[i]), printable(reads[i].begin).c_str(),
+                  reads[i].readVersion);
+            } else {
+              fprintf(stderr,
+                      "%p Expected %s, got %s for read of [%s, %s) at version "
+                      "%" PRId64 "\n",
+                      (void *)this, resultToStr(results2[i]),
+                      resultToStr(results1[i]),
+                      printable(reads[i].begin).c_str(),
+                      printable(reads[i].end).c_str(), reads[i].readVersion);
             }
             return false;
           }

SkipList.cpp

@@ -778,10 +778,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     for (int s = stripes - 1; s >= 0; s--) {
       for (int i = 0; i * 2 < ss; ++i) {
         const auto &w = combinedWriteConflictRanges[s * stripeSize / 2 + i];
-#if DEBUG_VERBOSE
-        printf("Write begin: %s\n", printable(w.begin).c_str());
-        fflush(stdout);
-#endif
         values[i * 2] = w.first;
         values[i * 2 + 1] = w.second;
         keyUpdates += 3;

TestDriver.cpp

@@ -13,25 +13,58 @@ int main(int argc, char **argv) {
     std::stringstream buffer;
     buffer << t.rdbuf();
     auto str = buffer.str();
-    TestDriver<ConflictSet, !PERF_TEST> driver{(const uint8_t *)str.data(),
-                                               str.size()};
-    while (!driver.next())
-      ;
-    if (!driver.ok) {
-      abort();
+    Arbitrary arbitrary({(const uint8_t *)str.data(), str.size()});
+    TestDriver<ConflictSet, !PERF_TEST> driver1{arbitrary};
+    TestDriver<ConflictSet, !PERF_TEST> driver2{arbitrary};
+    bool done1 = false;
+    bool done2 = false;
+    for (;;) {
+      if (!done1) {
+        done1 = driver1.next();
+        if (!driver1.ok) {
+          abort();
+        }
+      }
+      if (!done2) {
+        done2 = driver2.next();
+        if (!driver2.ok) {
+          abort();
+        }
+      }
+      if (done1 && done2) {
+        break;
+      }
     }
 
-    ConflictSet::MetricsV1 *metrics;
-    int metricsCount;
-    driver.cs.getMetricsV1(&metrics, &metricsCount);
-    printf("#################### METRICS FOR %s ####################\n",
-           argv[i]);
-    for (int i = 0; i < metricsCount; ++i) {
-      printf("# HELP %s %s\n", metrics[i].name, metrics[i].help);
-      printf("# TYPE %s %s\n", metrics[i].name,
-             metrics[i].type == metrics[i].Counter ? "counter" : "gauge");
-      printf("%s %g\n", metrics[i].name, metrics[i].getValue());
+    {
+      ConflictSet::MetricsV1 *metrics;
+      int metricsCount;
+      driver1.cs.getMetricsV1(&metrics, &metricsCount);
+      printf("#################### METRICS for ConflictSet 1 for %s "
+             "####################\n",
+             argv[i]);
+      for (int i = 0; i < metricsCount; ++i) {
+        printf("# HELP %s %s\n", metrics[i].name, metrics[i].help);
+        printf("# TYPE %s %s\n", metrics[i].name,
+               metrics[i].type == metrics[i].Counter ? "counter" : "gauge");
+        printf("%s %g\n", metrics[i].name, metrics[i].getValue());
+      }
+      puts("");
+    }
+    {
+      ConflictSet::MetricsV1 *metrics;
+      int metricsCount;
+      driver2.cs.getMetricsV1(&metrics, &metricsCount);
+      printf("#################### METRICS for ConflictSet 2 for %s "
+             "####################\n",
+             argv[i]);
+      for (int i = 0; i < metricsCount; ++i) {
+        printf("# HELP %s %s\n", metrics[i].name, metrics[i].help);
+        printf("# TYPE %s %s\n", metrics[i].name,
+               metrics[i].type == metrics[i].Counter ? "counter" : "gauge");
+        printf("%s %g\n", metrics[i].name, metrics[i].getValue());
+      }
+      puts("");
     }
-    puts("");
   }
 }

aarch64-symbol-imports.txt

@@ -1,4 +1,4 @@
-__aarch64_cas8_relax
+__aarch64_ldadd8_relax
 __getauxval@GLIBC_2.17
 __stack_chk_fail@GLIBC_2.17
 __stack_chk_guard@GLIBC_2.17

corpus/0f5c5f28c31e44ee0ec085fec09adf0981065771

@@ -0,0 +1,5 @@
+
+<EFBFBD><EFBFBD>
+
+
+2

corpus/12bdd00fd4038756cbcf8ecdad1b0cd862603cd8

@@ -0,0 +1 @@
+<EFBFBD>