Merge pull request 'Improve perf/codegen' (#94 ) from perf into main

Reviewed-on: weaselab/conflict-set#94
Merge pull request 'Replace test_fixup_256 with a regression test for the Node256 fixupMaxVersion false-Commit bug' (#90 ) from weaselbot/conflict-set:weaselbot/issue-89 into main
2026-10-27 20:45:52 +00:00 · 2026-10-27 19:33:17 +00:00 · 2026-10-27 19:32:43 +00:00 · 2026-10-27 19:30:11 +00:00 · 2026-10-27 19:29:26 +00:00 · 2026-10-27 15:09:58 -04:00
17 changed files with 782 additions and 471 deletions
@@ -38,21 +38,43 @@ jobs:
      matrix:
        include:
          - name: 64-bit-versions
+            runner: ubuntu-latest-amd64
+            arch: amd64
+            llvm_version: "21"
+            msan_url: ""
            cmake_args: -DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1
          - name: debug
+            runner: ubuntu-latest-amd64
+            arch: amd64
+            llvm_version: "21"
+            msan_url: https://minio.weaselab.dev/public/x86_64/msan-toolchain-21.1.8.tar.zst
            cmake_args: -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan
          - name: simd-fallback
+            runner: ubuntu-latest-amd64
+            arch: amd64
+            llvm_version: "21"
+            msan_url: ""
            cmake_args: -DUSE_SIMD_FALLBACK=ON
          - name: gcc
+            runner: ubuntu-latest-amd64
+            arch: amd64
+            llvm_version: "21"
+            msan_url: ""
            cmake_args: -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++
-    runs-on: ubuntu-latest-amd64
+          - name: debug-arm64
+            runner: ubuntu-latest-arm64
+            arch: arm64
+            llvm_version: "22"
+            msan_url: https://minio.weaselab.dev/public/aarch64/msan-toolchain-22.1.8.tar.zst
+            cmake_args: -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan
+    runs-on: ${{ matrix.runner }}
    steps:
      - uses: actions/checkout@v4

      - uses: actions/cache@v4
        with:
          path: /var/cache/apt/archives
-          key: apt-amd64-${{ hashFiles('.gitea/workflows/ci.yml') }}
+          key: apt-${{ matrix.arch }}-${{ hashFiles('.gitea/workflows/ci.yml') }}

      - name: Install common dependencies
        run: |
@@ -68,18 +90,18 @@ jobs:
        run: |
          . /etc/os-release
          wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | sudo tee /etc/apt/trusted.gpg.d/apt.llvm.org.asc
-          echo "deb http://apt.llvm.org/${VERSION_CODENAME}/ llvm-toolchain-${VERSION_CODENAME}-21 main" | sudo tee /etc/apt/sources.list.d/llvm.list
+          echo "deb http://apt.llvm.org/${VERSION_CODENAME}/ llvm-toolchain-${VERSION_CODENAME}-${{ matrix.llvm_version }} main" | sudo tee /etc/apt/sources.list.d/llvm.list
          sudo apt-get update -qq
          sudo apt-get install -y \
-            clang-21 llvm-21 lld-21 mold
+            clang-${{ matrix.llvm_version }} llvm-${{ matrix.llvm_version }} lld-${{ matrix.llvm_version }} mold
          for tool in clang clang++ llvm-ar llvm-nm llvm-ranlib llvm-objcopy llvm-cov llvm-symbolizer lld ld.lld; do
-            sudo update-alternatives --install /usr/bin/${tool} ${tool} /usr/bin/${tool}-21 100
+            sudo update-alternatives --install /usr/bin/${tool} ${tool} /usr/bin/${tool}-${{ matrix.llvm_version }} 100
          done

      - name: Download MSan toolchain
-        if: matrix.name == 'debug'
+        if: matrix.msan_url != ''
        run: |
-          curl -Ls "https://minio.weaselab.dev/public/x86_64/msan-toolchain-21.1.8.tar.zst" -o /tmp/msan-toolchain.tar.zst
+          curl -Ls "${{ matrix.msan_url }}" -o /tmp/msan-toolchain.tar.zst
          sudo mkdir -p /opt/msan
          sudo tar --zstd -xf /tmp/msan-toolchain.tar.zst -C /opt/msan

@@ -185,12 +207,7 @@ jobs:
      - name: Test
        run: |
          cd build
-          # On arm64, valgrind needs the MAKE_MEM_DEFINED client requests for
-          # https://git.weaselab.dev/weaselab/conflict-set/issues/39, but this
-          # build has -DNVALGRIND, which compiles them out. Skip valgrind
-          # tests here; they run annotated in the test job.
-          ctest --no-compress-output --test-output-size-passed 100000 --test-output-size-failed 100000 ${{ matrix.arch == 'arm64' && '-E valgrind' || '' }} -T Test -j "$(nproc)" --timeout 90 > /dev/null
-
+          ctest --no-compress-output --test-output-size-passed 100000 --test-output-size-failed 100000 -T Test -j "$(nproc)" --timeout 90 > /dev/null
      - name: Package
        run: |
          cd build
@@ -5,7 +5,7 @@ project(
  DESCRIPTION
    "A data structure for optimistic concurrency control on ranges of bitwise-lexicographically-ordered keys."
  HOMEPAGE_URL "https://git.weaselab.dev/weaselab/conflict-set"
-  LANGUAGES C CXX ASM)
+  LANGUAGES C CXX)
 set(CMAKE_CXX_STANDARD 20)

 file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/version.txt ${PROJECT_VERSION})
@@ -130,19 +130,7 @@ endif()

 set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "")

-# Architecture-specific SIMD assembly. These functions operate on
-# potentially-indeterminate memory, which is UB in C++ but well-defined in
-# assembly.
-set(SIMD_ASM_FILES)
-if(CMAKE_SYSTEM_PROCESSOR STREQUAL x86_64 AND NOT USE_SIMD_FALLBACK)
-  set(SIMD_ASM_FILES ${CMAKE_CURRENT_SOURCE_DIR}/simd_x86_64.S)
-elseif((CMAKE_SYSTEM_PROCESSOR STREQUAL aarch64 OR CMAKE_SYSTEM_PROCESSOR
-                                                   STREQUAL arm64)
-       AND NOT USE_SIMD_FALLBACK)
-  set(SIMD_ASM_FILES ${CMAKE_CURRENT_SOURCE_DIR}/simd_aarch64.S)
-endif()
-
-add_library(${PROJECT_NAME}-object OBJECT ConflictSet.cpp ${SIMD_ASM_FILES})
+add_library(${PROJECT_NAME}-object OBJECT ConflictSet.cpp)
 target_compile_options(${PROJECT_NAME}-object PRIVATE -fno-exceptions
                                                      -fvisibility=hidden)
 target_include_directories(${PROJECT_NAME}-object
@@ -194,6 +182,8 @@ endif()

 include(CTest)

+option(TEST_SKIP_LIST "Run the slow skip list cross-check tests" OFF)
+
 # disable tests if this is being used through e.g. FetchContent
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)

@@ -236,8 +226,9 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
  target_compile_options(driver_skip_list PRIVATE ${TEST_FLAGS})
  target_link_libraries(driver_skip_list PRIVATE skip_list)

-  # enable to test skip list
-  if(0)
+  # Optional cross-check of SkipList.cpp against ReferenceImpl. Opt-in via
+  # -DTEST_SKIP_LIST=ON, since these are slow.
+  if(TEST_SKIP_LIST)
    foreach(TEST ${CORPUS_TESTS})
      get_filename_component(hash ${TEST} NAME)
      add_test(NAME skip_list_${hash} COMMAND driver_skip_list ${TEST})
@@ -245,7 +236,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
  endif()

  # ad hoc testing
-  add_executable(conflict_set_main ConflictSet.cpp ${SIMD_ASM_FILES})
+  add_executable(conflict_set_main ConflictSet.cpp)
  target_include_directories(conflict_set_main
                             PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include)
  target_compile_definitions(conflict_set_main PRIVATE ENABLE_MAIN)
@@ -261,7 +252,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
    cmake_pop_check_state()

    if(HAS_LIB_FUZZER)
-      add_executable(conflict_set_fuzz_test ConflictSet.cpp ${SIMD_ASM_FILES})
+      add_executable(conflict_set_fuzz_test ConflictSet.cpp)
      target_include_directories(conflict_set_fuzz_test
                                 PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include)
      target_compile_definitions(conflict_set_fuzz_test PRIVATE ENABLE_FUZZ)
@@ -273,8 +264,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
  endif()

  # whitebox tests asan+ubsan
-  add_executable(fuzz_driver ConflictSet.cpp FuzzTestDriver.cpp
-                             ${SIMD_ASM_FILES})
+  add_executable(fuzz_driver ConflictSet.cpp FuzzTestDriver.cpp)
  target_compile_options(fuzz_driver PRIVATE ${TEST_FLAGS})
  if(NOT CMAKE_CROSSCOMPILING)
    target_compile_options(fuzz_driver PRIVATE -fsanitize=address,undefined)
@@ -290,8 +280,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)

  # whitebox tests msan
  if(MSAN_TOOLCHAIN_PATH)
-    add_executable(fuzz_driver_msan ConflictSet.cpp FuzzTestDriver.cpp
-                                    ${SIMD_ASM_FILES})
+    add_executable(fuzz_driver_msan ConflictSet.cpp FuzzTestDriver.cpp)
    target_compile_options(fuzz_driver_msan PRIVATE ${TEST_FLAGS})
    if(NOT CMAKE_CROSSCOMPILING)
      target_compile_options(
@@ -319,8 +308,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)

  # tsan tests
  if(NOT CMAKE_CROSSCOMPILING AND NOT DISABLE_TSAN)
-    add_executable(tsan_driver ConflictSet.cpp FuzzTestDriver.cpp
-                               ${SIMD_ASM_FILES})
+    add_executable(tsan_driver ConflictSet.cpp FuzzTestDriver.cpp)
    target_compile_options(tsan_driver PRIVATE ${TEST_FLAGS} -fsanitize=thread)
    target_link_options(tsan_driver PRIVATE -fsanitize=thread)
    target_compile_definitions(tsan_driver PRIVATE ENABLE_FUZZ THREAD_TEST)
@@ -405,6 +393,24 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
                        PROPERTIES CXX_STANDARD_REQUIRED ON)
  add_test(NAME conflict_set_cxx_api_test COMMAND conflict_set_cxx_api_test)

+  # SHOW_MEMORY instrumentation, which the rest of the test suite doesn't
+  # compile with
+  add_executable(conflict_set_show_memory_test ConflictSet.cpp
+                                               ShowMemoryTest.cpp)
+  target_compile_options(conflict_set_show_memory_test PRIVATE ${TEST_FLAGS})
+  target_compile_definitions(conflict_set_show_memory_test
+                             PRIVATE SHOW_MEMORY=1)
+  target_include_directories(conflict_set_show_memory_test
+                             PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include)
+  if(NOT CMAKE_CROSSCOMPILING)
+    target_compile_options(conflict_set_show_memory_test
+                           PRIVATE -fsanitize=address,undefined)
+    target_link_options(conflict_set_show_memory_test PRIVATE
+                        -fsanitize=address,undefined)
+  endif()
+  add_test(NAME conflict_set_show_memory_test
+           COMMAND conflict_set_show_memory_test)
+
  # symbol visibility tests
  if(NOT CMAKE_BUILD_TYPE STREQUAL Debug)
    if(APPLE)
@@ -28,7 +28,6 @@ limitations under the License.
 #include "Internal.h"
 #include "LongestCommonPrefix.h"
 #include "Metrics.h"
-#include "simd.h"

 #include <algorithm>
 #include <bit>
@@ -81,17 +80,19 @@ using namespace weaselab;
 #define assume assert
 #endif

-#if SHOW_MEMORY
+// The SHOW_MEMORY instrumentation. Declared unconditionally so that each of
+// these has a single definition in every configuration (see the definitions at
+// the end of this file), rather than a no-op stub standing in for the real
+// thing.
 void addNode(struct Node *);
 void removeNode(struct Node *);
 void addKey(struct Node *);
 void removeKey(struct Node *);
-#else
-constexpr void addNode(struct Node *) {}
-constexpr void removeNode(struct Node *) {}
-constexpr void addKey(struct Node *) {}
-constexpr void removeKey(struct Node *) {}
-#endif
+// Like `removeKey`, but with a caller-provided search path length. Used when
+// the node's ancestors may already have been released, which makes walking
+// `parent` pointers invalid.
+void removeKey(struct Node *, int64_t searchPathLength);
+int64_t getSearchPathLength(struct Node *);

 // ==================== BEGIN IMPLEMENTATION ====================

@@ -117,13 +118,40 @@ struct InternalVersionT {
  constexpr InternalVersionT() = default;
  constexpr explicit InternalVersionT(int64_t value) : value(value) {}
  constexpr int64_t toInt64() const { return value; } // GCOVR_EXCL_LINE
-  constexpr auto operator<=>(const InternalVersionT &rhs) const {
+  constexpr bool operator<(const InternalVersionT &rhs) const {
 #if USE_64_BIT
-    return value <=> rhs.value;
+    return value < rhs.value;
 #else
    // Maintains ordering after overflow, as long as the full-precision versions
    // are within `kMaxCorrectVersionWindow` of eachother.
-    return int32_t(value - rhs.value) <=> 0;
+    return int32_t(value - rhs.value) < 0;
+#endif
+  }
+  constexpr bool operator<=(const InternalVersionT &rhs) const {
+#if USE_64_BIT
+    return value <= rhs.value;
+#else
+    // Maintains ordering after overflow, as long as the full-precision versions
+    // are within `kMaxCorrectVersionWindow` of eachother.
+    return int32_t(value - rhs.value) <= 0;
+#endif
+  }
+  constexpr bool operator>=(const InternalVersionT &rhs) const {
+#if USE_64_BIT
+    return value >= rhs.value;
+#else
+    // Maintains ordering after overflow, as long as the full-precision versions
+    // are within `kMaxCorrectVersionWindow` of eachother.
+    return int32_t(value - rhs.value) >= 0;
+#endif
+  }
+  constexpr bool operator>(const InternalVersionT &rhs) const {
+#if USE_64_BIT
+    return value > rhs.value;
+#else
+    // Maintains ordering after overflow, as long as the full-precision versions
+    // are within `kMaxCorrectVersionWindow` of eachother.
+    return int32_t(value - rhs.value) > 0;
 #endif
  }
  constexpr bool operator==(const InternalVersionT &) const = default;
@@ -308,7 +336,9 @@ struct Node {
  uint8_t *partialKey();
  Type getType() const { return type; }
  int32_t getCapacity() const {
-    assert(!releaseDeferred);
+    // `partialKeyCapacity` is not repurposed by `deferRelease`, so this is also
+    // valid for nodes with `releaseDeferred` set. It's needed to account for
+    // the capacity of a deferred node when it's finally released.
    return partialKeyCapacity;
  }

@@ -489,6 +519,7 @@ inline void Node3::copyChildrenAndKeyFrom(const Node16 &other) {

 inline void Node16::copyChildrenAndKeyFrom(const Node3 &other) {
  copyCommon(*this, other);
+  memset(index, 0, sizeof(index));
  memcpy(index, other.index, Node3::kMaxNodes);
  memcpy(children, other.children,
         Node3::kMaxNodes * sizeof(children[0])); // NOLINT
@@ -500,6 +531,12 @@ inline void Node16::copyChildrenAndKeyFrom(const Node3 &other) {
    assert(children[i]->parent == &other);
    children[i]->parent = this;
  }
+  // The index bytes beyond numChildren are initialized so that the SIMD loads
+  // of the full 16 bytes are well-defined (reading indeterminate bytes is UB
+  // in C++). valgrind then treats them as undefined so that it can still flag
+  // any read that relies on them.
+  VALGRIND_MAKE_MEM_UNDEFINED(index + numChildren,
+                              sizeof(index) - numChildren * sizeof(index[0]));
 }

 inline void Node16::copyChildrenAndKeyFrom(const Node16 &other) {
@@ -516,6 +553,7 @@ inline void Node16::copyChildrenAndKeyFrom(const Node16 &other) {

 inline void Node16::copyChildrenAndKeyFrom(const Node48 &other) {
  copyCommon(*this, other);
+  memset(index, 0, sizeof(index));
  int i = 0;
  other.bitSet.forEachSet([&](int c) {
    // Suppress a false positive -Waggressive-loop-optimizations warning
@@ -529,6 +567,12 @@ inline void Node16::copyChildrenAndKeyFrom(const Node48 &other) {
    ++i;
  });
  memcpy(partialKey(), &other + 1, partialKeyLen);
+  // The index bytes beyond numChildren are initialized so that the SIMD loads
+  // of the full 16 bytes are well-defined (reading indeterminate bytes is UB
+  // in C++). valgrind then treats them as undefined so that it can still flag
+  // any read that relies on them.
+  VALGRIND_MAKE_MEM_UNDEFINED(index + numChildren,
+                              sizeof(index) - numChildren * sizeof(index[0]));
 }

 inline void Node48::copyChildrenAndKeyFrom(const Node16 &other) {
@@ -536,6 +580,7 @@ inline void Node48::copyChildrenAndKeyFrom(const Node16 &other) {
  assert(numChildren == Node16::kMaxNodes);
  memset(index, -1, sizeof(index));
  memset(children, 0, sizeof(children));
+  memset(reverseIndex, 0, sizeof(reverseIndex));
  const auto z = InternalVersionT::zero;
  for (auto &v : childMaxVersion) {
    v = z;
@@ -555,6 +600,9 @@ inline void Node48::copyChildrenAndKeyFrom(const Node16 &other) {
        std::max(maxOfMax[i >> Node48::kMaxOfMaxShift], childMaxVersion[i]);
    ++i;
  }
+  // The reverseIndex bytes beyond numChildren are initialized so that the
+  // SIMD loads of a full 16-byte page in scan16 are well-defined (reading
+  // indeterminate bytes is UB in C++).
 }

 inline void Node48::copyChildrenAndKeyFrom(const Node48 &other) {
@@ -581,6 +629,7 @@ inline void Node48::copyChildrenAndKeyFrom(const Node256 &other) {
  copyCommon(*this, other);
  memset(index, -1, sizeof(index));
  memset(children, 0, sizeof(children));
+  memset(reverseIndex, 0, sizeof(reverseIndex));
  const auto z = InternalVersionT::zero;
  for (auto &v : childMaxVersion) {
    v = z;
@@ -602,6 +651,9 @@ inline void Node48::copyChildrenAndKeyFrom(const Node256 &other) {
    ++i;
  });
  memcpy(partialKey(), &other + 1, partialKeyLen);
+  // The reverseIndex bytes beyond numChildren are initialized so that the
+  // SIMD loads of a full 16-byte page in scan16 are well-defined (reading
+  // indeterminate bytes is UB in C++).
 }

 inline void Node256::copyChildrenAndKeyFrom(const Node48 &other) {
@@ -911,21 +963,51 @@ int getNodeIndexExists(Node3 *self, uint8_t index) {

 int getNodeIndex(Node16 *self, uint8_t index) {

-#if defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
-  uint32_t bitfield =
-      find_eq_16(self->index, index) & ((1 << self->numChildren) - 1);
+#ifdef HAS_AVX
+  // Based on https://www.the-paper-trail.org/post/art-paper-notes/
+
+  // key_vec is 16 repeated copies of the searched-for byte, one for every
+  // possible position in child_keys that needs to be searched.
+  __m128i key_vec = _mm_set1_epi8(index);
+
+  // Compare all child_keys to 'index' in parallel. Don't worry if some of the
+  // keys aren't valid, we'll mask the results to only consider the valid ones
+  // below.
+  __m128i indices;
+  memcpy(&indices, self->index, Node16::kMaxNodes);
+  __m128i results = _mm_cmpeq_epi8(key_vec, indices);
+
+  // Build a mask to select only the first node->num_children values from the
+  // comparison (because the other values are meaningless)
+  uint32_t mask = (1 << self->numChildren) - 1;
+
+  // Change the results of the comparison into a bitfield, masking off any
+  // invalid comparisons.
+  uint32_t bitfield = _mm_movemask_epi8(results) & mask;
+
+  // No match if there are no '1's in the bitfield.
  if (bitfield == 0)
    return -1;
+
+  // Find the index of the first '1' in the bitfield by counting the leading
+  // zeros.
  return std::countr_zero(bitfield);
 #elif defined(HAS_ARM_NEON)
-  // The index load is done in assembly (find_eq_16) so that reading the
-  // potentially-indeterminate unused index bytes is well-defined.
-  uint64_t bitfield = find_eq_16(self->index, index);
+  // Based on
+  // https://community.arm.com/arm-community-blogs/b/infrastructure-solutions-blog/posts/porting-x86-vector-bitmask-optimizations-to-arm-neon
+
+  uint8x16_t indices;
+  memcpy(&indices, self->index, Node16::kMaxNodes);
+  // 0xff for each match
+  uint16x8_t results =
+      vreinterpretq_u16_u8(vceqq_u8(vdupq_n_u8(index), indices));
  assume(self->numChildren <= Node16::kMaxNodes);
  uint64_t mask = self->numChildren == 16
                      ? uint64_t(-1)
                      : (uint64_t(1) << (self->numChildren * 4)) - 1;
-  bitfield &= mask;
+  // 0xf for each match in valid range
+  uint64_t bitfield =
+      vget_lane_u64(vreinterpret_u64_u8(vshrn_n_u16(results, 4)), 0) & mask;
  if (bitfield == 0)
    return -1;
  return std::countr_zero(bitfield) / 4;
@@ -941,20 +1023,31 @@ int getNodeIndex(Node16 *self, uint8_t index) {

 int getNodeIndexExists(Node16 *self, uint8_t index) {

-#if defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
-  uint32_t bitfield =
-      find_eq_16(self->index, index) & ((1 << self->numChildren) - 1);
+#ifdef HAS_AVX
+  __m128i key_vec = _mm_set1_epi8(index);
+  __m128i indices;
+  memcpy(&indices, self->index, Node16::kMaxNodes);
+  __m128i results = _mm_cmpeq_epi8(key_vec, indices);
+  uint32_t mask = (1 << self->numChildren) - 1;
+  uint32_t bitfield = _mm_movemask_epi8(results) & mask;
  assume(bitfield != 0);
  return std::countr_zero(bitfield);
 #elif defined(HAS_ARM_NEON)
-  // The index load is done in assembly (find_eq_16) so that reading the
-  // potentially-indeterminate unused index bytes is well-defined.
-  uint64_t bitfield = find_eq_16(self->index, index);
+  // Based on
+  // https://community.arm.com/arm-community-blogs/b/infrastructure-solutions-blog/posts/porting-x86-vector-bitmask-optimizations-to-arm-neon
+
+  uint8x16_t indices;
+  memcpy(&indices, self->index, Node16::kMaxNodes);
+  // 0xff for each match
+  uint16x8_t results =
+      vreinterpretq_u16_u8(vceqq_u8(vdupq_n_u8(index), indices));
  assume(self->numChildren <= Node16::kMaxNodes);
  uint64_t mask = self->numChildren == 16
                      ? uint64_t(-1)
                      : (uint64_t(1) << (self->numChildren * 4)) - 1;
-  bitfield &= mask;
+  // 0xf for each match in valid range
+  uint64_t bitfield =
+      vget_lane_u64(vreinterpret_u64_u8(vshrn_n_u16(results, 4)), 0) & mask;
  assume(bitfield != 0);
  return std::countr_zero(bitfield) / 4;
 #else
@@ -1226,19 +1319,29 @@ TaggedNodePointer getChildGeq(Node16 *self, int child) {
    return nullptr;
  }

-#if defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
-  uint32_t bitfield =
-      find_ge_16(self->index, child) & ((1 << self->numChildren) - 1);
+#ifdef HAS_AVX
+  __m128i key_vec = _mm_set1_epi8(child);
+  __m128i indices;
+  memcpy(&indices, self->index, Node16::kMaxNodes);
+  __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;
  return bitfield == 0 ? nullptr : self->children[std::countr_zero(bitfield)];
 #elif defined(HAS_ARM_NEON)
-  // The index load is done in assembly (find_ge_16) so that reading the
-  // potentially-indeterminate unused index bytes is well-defined.
-  uint64_t bitfield = find_ge_16(self->index, child);
+  uint8x16_t indices;
+  memcpy(&indices, self->index, sizeof(self->index));
+  // 0xff for each leq
+  auto results = vcleq_u8(vdupq_n_u8(child), indices);
  assume(self->numChildren <= Node16::kMaxNodes);
  uint64_t mask = self->numChildren == 16
                      ? uint64_t(-1)
                      : (uint64_t(1) << (self->numChildren * 4)) - 1;
-  bitfield &= mask;
+  // 0xf for each 0xff (within mask)
+  uint64_t bitfield =
+      vget_lane_u64(
+          vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(results), 4)),
+          0) &
+      mask;
  return bitfield == 0 ? nullptr
                       : self->children[std::countr_zero(bitfield) / 4];
 #else
@@ -1942,7 +2045,6 @@ Node *erase(Node *self, WriteContext *writeContext, bool logical) {
      parent48->index[parentIndex] = toRemoveChildrenIndex;
      parent48->reverseIndex[toRemoveChildrenIndex] = parentIndex;
    }
-    parent48->childMaxVersion[lastChildrenIndex] = writeContext->zero;
    VALGRIND_MAKE_MEM_UNDEFINED(parent48->children + lastChildrenIndex,
                                sizeof(parent48->children[0]));

@@ -2037,23 +2139,33 @@ compare16_avx512(const InternalVersionT *vs, InternalVersionT rv) {
 }
 #endif

-// Returns true if v[i] <= readVersion for all i such that begin <= is[i] < end
-// Preconditions: begin <= end, end - begin < 256
-template <bool kAVX512>
-bool scan16(const InternalVersionT *vs, const uint8_t *is, int begin, int end,
-            InternalVersionT readVersion) {
-
-  assert(begin <= end);
-  assert(end - begin < 256);
-
 #ifdef HAS_ARM_NEON
-
-  // The index load is done in assembly (mask_in_range_16) so that reading
-  // potentially-indeterminate unused index bytes is well-defined. `vs` slots
-  // beyond the in-use range are always initialized (to zero) by the allocator,
-  // so the version compare below stays in C++.
-  uint64_t mask = mask_in_range_16(is, begin, end);
-
+// Returns a uint8x16_t whose byte i is 0xff if vs[i] > readVersion and 0
+// otherwise. When USE_64_BIT is set the comparison uses the full 64-bit value
+// (via vcgtq_s64); otherwise it falls back to the 32-bit subtract-and-compare
+// used by the original NEON fast path. The caller narrows this 16-byte vector
+// into the nibble-packed (4 bits per version) mask expected by scan16 and
+// checkMaxBetweenExclusiveImpl.
+inline uint8x16_t conflictMask16(const InternalVersionT *vs,
+                                 InternalVersionT readVersion) {
+#if USE_64_BIT
+  int64_t rv;
+  memcpy(&rv, &readVersion, sizeof(rv));
+  const auto rvVec = vdupq_n_s64(rv);
+  int32x2_t r32[8];
+  const auto *vsp = reinterpret_cast<const int64_t *>(vs);
+  for (int j = 0; j < 8; ++j) {
+    r32[j] = vmovn_s64(
+        vreinterpretq_s64_u64(vcgtq_s64(vld1q_s64(vsp + 2 * j), rvVec)));
+  }
+  uint32x4_t w4[4];
+  for (int k = 0; k < 4; ++k) {
+    w4[k] = vreinterpretq_u32_s32(vcombine_s32(r32[2 * k], r32[2 * k + 1]));
+  }
+  return vcombine_u8(
+      vmovn_u16(vcombine_u16(vmovn_u32(w4[0]), vmovn_u32(w4[1]))),
+      vmovn_u16(vcombine_u16(vmovn_u32(w4[2]), vmovn_u32(w4[3]))));
+#else
  uint32x4_t w4[4];
  memcpy(w4, vs, sizeof(w4));
  uint32_t rv;
@@ -2068,18 +2180,46 @@ bool scan16(const InternalVersionT *vs, const uint8_t *is, int begin, int end,
    conflicting[i] =
        vmovn_u32(vcgtq_s32(vreinterpretq_s32_u32(vsubq_u32(w4[i], rvVec)), z));
  }
-  auto combined =
-      vcombine_u8(vmovn_u16(vcombine_u16(conflicting[0], conflicting[1])),
-                  vmovn_u16(vcombine_u16(conflicting[2], conflicting[3])));
+  return vcombine_u8(vmovn_u16(vcombine_u16(conflicting[0], conflicting[1])),
+                     vmovn_u16(vcombine_u16(conflicting[2], conflicting[3])));
+#endif
+}
+#endif
+
+// Returns true if v[i] <= readVersion for all i such that begin <= is[i] < end
+// Preconditions: begin <= end, end - begin < 256
+template <bool kAVX512>
+bool scan16(const InternalVersionT *vs, const uint8_t *is, int begin, int end,
+            InternalVersionT readVersion) {
+
+  assert(begin <= end);
+  assert(end - begin < 256);
+
+#ifdef HAS_ARM_NEON
+
+  uint8x16_t indices;
+  memcpy(&indices, is, 16);
+  // 0xff for each in bounds
+  auto results =
+      vcltq_u8(vsubq_u8(indices, vdupq_n_u8(begin)), vdupq_n_u8(end - begin));
+  // 0xf for each 0xff
+  uint64_t mask = vget_lane_u64(
+      vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(results), 4)), 0);

  uint64_t compared = vget_lane_u64(
-      vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(combined), 4)), 0);
+      vreinterpret_u64_u8(vshrn_n_u16(
+          vreinterpretq_u16_u8(conflictMask16(vs, readVersion)), 4)),
+      0);

  return !(compared & mask);

-#elif defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
+#elif defined(HAS_AVX)

-  uint32_t mask = mask_in_range_16(is, begin, end);
+  __m128i indices;
+  memcpy(&indices, is, 16);
+  indices = _mm_sub_epi8(indices, _mm_set1_epi8(begin));
+  uint32_t mask = ~_mm_movemask_epi8(_mm_cmpeq_epi8(
+      indices, _mm_max_epu8(indices, _mm_set1_epi8(end - begin))));

  uint32_t compared = 0;
  if constexpr (kAVX512) {
@@ -2119,26 +2259,10 @@ bool scan16(const InternalVersionT *vs, int begin, int end,
  assert(begin <= end);

 #if defined(HAS_ARM_NEON)
-  uint32x4_t w4[4];
-  memcpy(w4, vs, sizeof(w4));
-  uint32_t rv;
-  memcpy(&rv, &readVersion, sizeof(rv));
-  const auto rvVec = vdupq_n_u32(rv);
-
-  int32x4_t z;
-  memset(&z, 0, sizeof(z));
-
-  uint16x4_t conflicting[4];
-  for (int i = 0; i < 4; ++i) {
-    conflicting[i] =
-        vmovn_u32(vcgtq_s32(vreinterpretq_s32_u32(vsubq_u32(w4[i], rvVec)), z));
-  }
-  auto combined =
-      vcombine_u8(vmovn_u16(vcombine_u16(conflicting[0], conflicting[1])),
-                  vmovn_u16(vcombine_u16(conflicting[2], conflicting[3])));
-
  uint64_t conflict = vget_lane_u64(
-      vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(combined), 4)), 0);
+      vreinterpret_u64_u8(vshrn_n_u16(
+          vreinterpretq_u16_u8(conflictMask16(vs, readVersion)), 4)),
+      0);

  conflict &= end == 16 ? -1 : (uint64_t(1) << (end << 2)) - 1;
  conflict >>= begin << 2;
@@ -2187,6 +2311,8 @@ bool checkMaxBetweenExclusiveImpl(Node3 *n, int begin, int end,
  assert(!(begin == -1 && end == 256));

  auto *self = static_cast<Node3 *>(n);
+  assume(1 <= self->numChildren);
+  assume(self->numChildren <= 3);

  ++begin;

@@ -2233,11 +2359,14 @@ bool checkMaxBetweenExclusiveImpl(Node16 *n, int begin, int end,

 #ifdef HAS_ARM_NEON

-  // The index load is done in assembly (mask_in_range_16) so that reading the
-  // potentially-indeterminate unused index bytes is well-defined. The unused
-  // childMaxVersion slots are always initialized (to zero) by the allocator,
-  // so the version compare below stays in C++.
-  uint64_t mask = mask_in_range_16(self->index, begin, end);
+  uint8x16_t indices;
+  memcpy(&indices, self->index, 16);
+  // 0xff for each in bounds
+  auto results =
+      vcltq_u8(vsubq_u8(indices, vdupq_n_u8(begin)), vdupq_n_u8(end - begin));
+  // 0xf for each 0xff
+  uint64_t mask = vget_lane_u64(
+      vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(results), 4)), 0);

  mask &= self->numChildren == 16
              ? uint64_t(-1)
@@ -2249,33 +2378,23 @@ bool checkMaxBetweenExclusiveImpl(Node16 *n, int begin, int end,
  const bool firstRangeOk =
      !child->entryPresent || child->entry.rangeVersion <= readVersion;

-  uint32x4_t w4[4];
-  memcpy(w4, self->childMaxVersion, sizeof(w4));
-  uint32_t rv;
-  memcpy(&rv, &readVersion, sizeof(rv));
-  const auto rvVec = vdupq_n_u32(rv);
-
-  int32x4_t z;
-  memset(&z, 0, sizeof(z));
-
-  uint16x4_t conflicting[4];
-  for (int i = 0; i < 4; ++i) {
-    conflicting[i] =
-        vmovn_u32(vcgtq_s32(vreinterpretq_s32_u32(vsubq_u32(w4[i], rvVec)), z));
-  }
-  auto combined =
-      vcombine_u8(vmovn_u16(vcombine_u16(conflicting[0], conflicting[1])),
-                  vmovn_u16(vcombine_u16(conflicting[2], conflicting[3])));
-
  uint64_t compared = vget_lane_u64(
-      vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(combined), 4)), 0);
+      vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(conflictMask16(
+                                          self->childMaxVersion, readVersion)),
+                                      4)),
+      0);

  return !(compared & mask) && firstRangeOk;

-#elif defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
+#elif defined(HAS_AVX)

-  uint32_t mask = mask_in_range_16(self->index, begin, end) &
-                  ((1 << self->numChildren) - 1);
+  __m128i indices;
+  memcpy(&indices, self->index, 16);
+  indices = _mm_sub_epi8(indices, _mm_set1_epi8(begin));
+  uint32_t mask =
+      0xffff & ~_mm_movemask_epi8(_mm_cmpeq_epi8(
+                   indices, _mm_max_epu8(indices, _mm_set1_epi8(end - begin))));
+  mask &= (1 << self->numChildren) - 1;
  if (!mask) {
    return true;
  }
@@ -2667,10 +2786,39 @@ checkMaxBetweenExclusiveImpl<true>(Node256 *n, int begin, int end,
  return self;
 }

+#if SHOW_MEMORY
+// Length of the key leading to `child`, given the length of the key leading to
+// its parent. Consistent with `getSearchPathLength`.
+int64_t childSearchPathLength(int64_t parentSearchPathLength, Node *child) {
+  return parentSearchPathLength + 1 + child->partialKeyLen;
+}
+#endif
+
 void eraseTree(Node *root, WriteContext *writeContext) {
  Arena arena;
  auto toFree = vector<Node *>(arena);
  toFree.push_back(root);
+  // Length of the key leading to each node in `toFree`, on a parallel stack.
+  // A node is released before its children are visited, so the `parent`
+  // pointers that `getSearchPathLength` walks are dangling by the time a queued
+  // node is popped. Carry the lengths along instead. Only the SHOW_MEMORY
+  // accounting needs them.
+#if SHOW_MEMORY
+  auto searchPathLengths = vector<int64_t>(arena);
+  searchPathLengths.push_back(getSearchPathLength(root));
+#endif
+
+  // Queue `child` for release, where `parentSearchPathLength` is the length of
+  // the key leading to its parent.
+  auto queueChild = [&](Node *child, int64_t parentSearchPathLength) {
+    toFree.push_back(child);
+#if SHOW_MEMORY
+    searchPathLengths.push_back(
+        childSearchPathLength(parentSearchPathLength, child));
+#else
+    (void)parentSearchPathLength;
+#endif
+  };

  while (toFree.size() > 0) {
    auto *n = toFree.back();
@@ -2678,7 +2826,13 @@ void eraseTree(Node *root, WriteContext *writeContext) {
    writeContext->accum.entries_erased += n->entryPresent;
    ++writeContext->accum.nodes_released;

-    removeKey(n);
+#if SHOW_MEMORY
+    const int64_t searchPathLength = searchPathLengths.back();
+    searchPathLengths.pop_back();
+#else
+    const int64_t searchPathLength = 0;
+#endif
+    removeKey(n, searchPathLength);

    switch (n->getType()) {
    case Type_Node0: {
@@ -2688,28 +2842,39 @@ void eraseTree(Node *root, WriteContext *writeContext) {
    case Type_Node3: {
      auto *n3 = static_cast<Node3 *>(n);
      for (int i = 0; i < n3->numChildren; ++i) {
-        toFree.push_back(n3->children[i]);
+        queueChild(n3->children[i], searchPathLength);
      }
      writeContext->release(n3);
    } break;
    case Type_Node16: {
      auto *n16 = static_cast<Node16 *>(n);
      for (int i = 0; i < n16->numChildren; ++i) {
-        toFree.push_back(n16->children[i]);
+        queueChild(n16->children[i], searchPathLength);
      }
      writeContext->release(n16);
    } break;
    case Type_Node48: {
      auto *n48 = static_cast<Node48 *>(n);
      for (int i = 0; i < n48->numChildren; ++i) {
-        toFree.push_back(n48->children[i]);
+        queueChild(n48->children[i], searchPathLength);
      }
      writeContext->release(n48);
    } break;
    case Type_Node256: {
      auto *n256 = static_cast<Node256 *>(n);
      auto *out = toFree.unsafePrepareAppend(n256->numChildren).data();
-      n256->bitSet.forEachSet([&](int i) { *out++ = n256->children[i]; });
+#if SHOW_MEMORY
+      auto *outSearchPathLengths =
+          searchPathLengths.unsafePrepareAppend(n256->numChildren).data();
+#endif
+      n256->bitSet.forEachSet([&](int i) {
+        Node *child = n256->children[i];
+        *out++ = child;
+#if SHOW_MEMORY
+        *outSearchPathLengths++ =
+            childSearchPathLength(searchPathLength, child);
+#endif
+      });
      assert(out == toFree.end());
      writeContext->release(n256);
    } break;
@@ -2806,7 +2971,11 @@ horizontalMax16(InternalVersionT *vs, InternalVersionT) {
  return max;
 }

-// Precondition: `node->entryPresent`, and node is not the root
+/** @pre `node` is not the root, `node->entryPresent`, and `writeContext !=
+ * nullptr`.
+ * @post `maxVersion(node)` is the max of `writeContext->zero` and the max
+ * version of all logical keys starting with the search path of `node`.
+ */
 void fixupMaxVersion(Node *node, WriteContext *writeContext) {
  assert(node->parent);
  InternalVersionT max;
@@ -2817,9 +2986,13 @@ void fixupMaxVersion(Node *node, WriteContext *writeContext) {
    break;
  case Type_Node3: {
    auto *self3 = static_cast<Node3 *>(node);
-    max = std::max(max,
-                   horizontalMaxUpTo16(self3->childMaxVersion,
-                                       writeContext->zero, self3->numChildren));
+    assume(1 <= self3->numChildren);
+    assume(self3->numChildren <= 3);
+    InternalVersionT m = self3->childMaxVersion[0];
+    for (int i = 1; i < self3->numChildren; ++i) {
+      m = std::max(m, self3->childMaxVersion[i]);
+    }
+    max = std::max(max, m);
  } break;
  case Type_Node16: {
    auto *self16 = static_cast<Node16 *>(node);
@@ -2835,8 +3008,7 @@ void fixupMaxVersion(Node *node, WriteContext *writeContext) {
  } break;
  case Type_Node256: {
    auto *self256 = static_cast<Node256 *>(node);
-    max = std::max(
-        max, horizontalMax16(self256->childMaxVersion, writeContext->zero));
+    max = std::max(max, horizontalMax16(self256->maxOfMax, writeContext->zero));
  } break;
  default:                   // GCOVR_EXCL_LINE
    __builtin_unreachable(); // GCOVR_EXCL_LINE
@@ -4783,7 +4955,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
        auto *results2 = new (arena) Result[count];
        check::Context context2;
        useSequential(reads, results2, count, context2);
-        assert(memcmp(result, results2, count) == 0);
+        assert(memcmp(result, results2, count * sizeof(Result)) == 0);
        assert(context.readContext == context2.readContext);
 #endif
      }
@@ -4884,15 +5056,17 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
        }
        if (e != nullptr) {
          while (e->releaseDeferred) {
-            e = b->forwardTo;
+            e = e->forwardTo;
          }
        }
        fprintf(stderr, "search path: %s, begin: %s\n",
                getSearchPathPrintable(b).c_str(),
                printable(context.results[i].remaining).c_str());
-        fprintf(stderr, "search path: %s, end: %s\n",
-                getSearchPathPrintable(e).c_str(),
-                printable(context.results[i].endRemaining).c_str());
+        if (e != nullptr) {
+          fprintf(stderr, "search path: %s, end: %s\n",
+                  getSearchPathPrintable(e).c_str(),
+                  printable(context.results[i].endRemaining).c_str());
+        }
      }
 #endif

@@ -4994,7 +5168,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {

    if (oldestExtantVersion < writeVersion - kMaxCorrectVersionWindow)
        [[unlikely]] {
-      if (writeVersion > newestVersionFullPrecision + kNominalVersionWindow) {
+      if (writeVersion - kNominalVersionWindow > newestVersionFullPrecision) {
        eraseTree(rootParent->children[0], &writeContext);
        init(writeVersion - kNominalVersionWindow);
      }
@@ -5372,8 +5546,9 @@ void internal_addWrites(ConflictSet::Impl *impl,
  impl->totalBytes += mallocBytesDelta;
  impl->memory_bytes.set(impl->totalBytes);
 #if SHOW_MEMORY
+  // Testing-only cross-check of the accounting.
  if (impl->totalBytes != mallocBytes) {
-    abort();
+    abort(); // GCOVR_EXCL_LINE
  }
 #endif
 }
@@ -5384,8 +5559,9 @@ void internal_setOldestVersion(ConflictSet::Impl *impl, int64_t oldestVersion) {
  impl->totalBytes += mallocBytesDelta;
  impl->memory_bytes.set(impl->totalBytes);
 #if SHOW_MEMORY
+  // Testing-only cross-check of the accounting.
  if (impl->totalBytes != mallocBytes) {
-    abort();
+    abort(); // GCOVR_EXCL_LINE
  }
 #endif
 }
@@ -5851,6 +6027,14 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,

 } // namespace

+// The SHOW_MEMORY instrumentation. Each of these functions is defined exactly
+// once, with the accounting compiled out when SHOW_MEMORY is disabled. Defining
+// them once (rather than defining no-op stubs when SHOW_MEMORY is disabled)
+// keeps every configuration of this file reporting each function on the same
+// line, which merging coverage from objects compiled with different options
+// requires: gcovr, merging the SHOW_MEMORY test with the rest of the build,
+// otherwise aborts with "Got function ... on multiple lines". The optimizer
+// removes the resulting empty calls.
 #if SHOW_MEMORY

 int64_t nodeBytes = 0;
@@ -5879,7 +6063,10 @@ int64_t getNodeSize(struct Node *n) {
  }
 }

+#endif
+
 int64_t getSearchPathLength(Node *n) {
+#if SHOW_MEMORY
  assert(n != nullptr);
  int64_t result = 0;
  for (;;) {
@@ -5891,9 +6078,14 @@ int64_t getSearchPathLength(Node *n) {
    n = n->parent;
  }
  return result;
+#else
+  (void)n;
+  return 0;
+#endif
 }

 void addNode(Node *n) {
+#if SHOW_MEMORY
  nodeBytes += getNodeSize(n);
  partialCapacityBytes += n->getCapacity();
  if (nodeBytes > peakNodeBytes) {
@@ -5902,14 +6094,22 @@ void addNode(Node *n) {
  if (partialCapacityBytes > peakPartialCapacityBytes) {
    peakPartialCapacityBytes = partialCapacityBytes;
  }
+#else
+  (void)n;
+#endif
 }

 void removeNode(Node *n) {
+#if SHOW_MEMORY
  nodeBytes -= getNodeSize(n);
  partialCapacityBytes -= n->getCapacity();
+#else
+  (void)n;
+#endif
 }

 void addKey(Node *n) {
+#if SHOW_MEMORY
  if (!n->entryPresent) {
    ++totalKeys;
    keyBytes += getSearchPathLength(n);
@@ -5920,15 +6120,36 @@ void addKey(Node *n) {
      peakKeyBytes = keyBytes;
    }
  }
+#else
+  (void)n;
+#endif
 }

 void removeKey(Node *n) {
+#if SHOW_MEMORY
  if (n->entryPresent) {
    --totalKeys;
    keyBytes -= getSearchPathLength(n);
  }
+#else
+  (void)n;
+#endif
 }

+void removeKey(Node *n, int64_t searchPathLength) {
+#if SHOW_MEMORY
+  if (n->entryPresent) {
+    --totalKeys;
+    keyBytes -= searchPathLength;
+  }
+#else
+  (void)n;
+  (void)searchPathLength;
+#endif
+}
+
+#if SHOW_MEMORY
+
 struct __attribute__((visibility("default"))) PeakPrinter {
  ~PeakPrinter() {
    printf("--- radix_tree ---\n");
@@ -59,6 +59,12 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
    while (keyUpdates > 0) {
      if (iter == map.end()) {
        iter = map.begin();
+        // The map is empty, so there are no more entries to garbage-collect.
+        // Break rather than spinning forever (keyUpdates is charged at 2x
+        // the entry count, so it may still be > 0 after emptying the map).
+        if (iter == map.end()) {
+          break;
+        }
      }
      for (; iter != map.end(); --keyUpdates) {
        if (iter->second <= oldestVersion) {
@@ -23,7 +23,9 @@ using namespace weaselab;
 #include <callgrind.h>

 #define DEBUG_VERBOSE 0
+#ifndef SHOW_MEMORY
 #define SHOW_MEMORY 0
+#endif

 // std::span is not trivially constructible. We want a span that leaves its
 // members uninitialized for performance reasons.
@@ -96,6 +98,11 @@ static_assert(std::is_trivial_v<TrivialSpan>);
 #if SHOW_MEMORY
 inline int64_t mallocBytes = 0;
 inline int64_t peakMallocBytes = 0;
+// Defined in ConflictSet.cpp
+extern int64_t nodeBytes;
+extern int64_t partialCapacityBytes;
+extern int64_t totalKeys;
+extern int64_t keyBytes;
 #endif

 inline thread_local int64_t mallocBytesDelta = 0;
@@ -96,7 +96,7 @@ inline int firstNeqStride(const uint8_t *ap, const uint8_t *bp) {
 // GCOVR_EXCL_START
 #if defined(HAS_AVX) && !defined(__SANITIZE_THREAD__)
 __attribute__((target("avx512f,avx512bw"))) inline int
-longestCommonPrefix(const uint8_t *ap, const uint8_t *bp, int cl) {
+longestCommonPrefixLong(const uint8_t *ap, const uint8_t *bp, int cl) {
  int i = 0;
  int end = cl & ~63;
  while (i < end) {
@@ -123,7 +123,7 @@ __attribute__((target("default")))
 // GCOVR_EXCL_STOP

 inline int
-longestCommonPrefix(const uint8_t *ap, const uint8_t *bp, int cl) {
+longestCommonPrefixLong(const uint8_t *ap, const uint8_t *bp, int cl) {
  if (!(cl >= 0)) {
    __builtin_unreachable(); // GCOVR_EXCL_LINE
  }
@@ -182,4 +182,72 @@ longestCommonPrefix(const uint8_t *ap, const uint8_t *bp, int cl) {
  }

  return i;
-}
+}
+// Use in-bounds native-word loads for short common lengths. The second load
+// overlaps the first; because the first load matched, any mismatch found in
+// the overlap is impossible and the returned absolute index is still correct.
+// Precondition: cl <= 2 * sizeof(uint64_t)
+__attribute__((always_inline)) static inline int
+longestCommonPrefixShort(const uint8_t *ap, const uint8_t *bp, int cl) {
+  assert(cl <= 2 * int(sizeof(uint64_t)));
+  if (cl >= 8) {
+    uint64_t a0;
+    uint64_t b0;
+    memcpy(&a0, ap, sizeof(a0));
+    memcpy(&b0, bp, sizeof(b0));
+    if (uint64_t mismatched = a0 ^ b0) {
+      return std::countr_zero(mismatched) / 8;
+    }
+    if (cl == 8) {
+      return 8;
+    }
+
+    uint64_t a1;
+    uint64_t b1;
+    memcpy(&a1, ap + cl - 8, sizeof(a1));
+    memcpy(&b1, bp + cl - 8, sizeof(b1));
+    if (uint64_t mismatched = a1 ^ b1) {
+      return cl - 8 + std::countr_zero(mismatched) / 8;
+    }
+    return cl;
+  }
+
+  if (cl >= 4) {
+    uint32_t a0;
+    uint32_t b0;
+    memcpy(&a0, ap, sizeof(a0));
+    memcpy(&b0, bp, sizeof(b0));
+    if (uint32_t mismatched = a0 ^ b0) {
+      return std::countr_zero(mismatched) / 8;
+    }
+    if (cl == 4) {
+      return 4;
+    }
+
+    uint32_t a1;
+    uint32_t b1;
+    memcpy(&a1, ap + cl - 4, sizeof(a1));
+    memcpy(&b1, bp + cl - 4, sizeof(b1));
+    if (uint32_t mismatched = a1 ^ b1) {
+      return cl - 4 + std::countr_zero(mismatched) / 8;
+    }
+    return cl;
+  }
+
+  int i = 0;
+  while (i < cl && ap[i] == bp[i]) {
+    ++i;
+  }
+  return i;
+}
+
+__attribute__((always_inline)) static inline int
+longestCommonPrefix(const uint8_t *ap, const uint8_t *bp, int cl) {
+  if (!(cl >= 0)) {
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  }
+  if (cl <= 2 * int(sizeof(uint64_t))) {
+    return longestCommonPrefixShort(ap, bp, cl);
+  }
+  return longestCommonPrefixLong(ap, bp, cl);
+}
@@ -19,30 +19,30 @@ InstalledDir: /usr/lib/llvm-21/bin

 |               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
 |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
-|              164.29 |        6,086,873.38 |    0.0% |        3,107.03 |          604.19 |  5.142 |         558.59 |    0.0% |      1.96 | `point reads`
-|              161.05 |        6,209,395.38 |    0.1% |        3,036.76 |          592.21 |  5.128 |         539.35 |    0.0% |      1.93 | `prefix reads`
-|              239.55 |        4,174,539.38 |    0.1% |        3,722.71 |          880.68 |  4.227 |         692.00 |    0.0% |      2.86 | `range reads`
-|              354.75 |        2,818,919.14 |    0.7% |        4,523.64 |        1,304.75 |  3.467 |         720.22 |    2.0% |      4.23 | `point writes`
-|              345.32 |        2,895,878.47 |    0.1% |        4,484.57 |        1,270.31 |  3.530 |         705.00 |    1.8% |      4.12 | `prefix writes`
-|              193.48 |        5,168,547.42 |    0.1% |        2,224.10 |          711.72 |  3.125 |         377.17 |    3.3% |      2.32 | `range writes`
-|              404.89 |        2,469,777.50 |    2.4% |        6,855.96 |        1,489.70 |  4.602 |       1,227.82 |    1.3% |      0.05 | `monotonic increasing point writes`
-|          134,231.80 |            7,449.80 |    1.9% |      812,045.25 |      495,770.40 |  1.638 |     151,246.50 |    0.9% |      0.01 | `worst case for radix tree`
-|               37.80 |       26,454,311.17 |    0.4% |          701.00 |          139.14 |  5.038 |         102.00 |    0.0% |      0.01 | `create and destroy`
+|              165.93 |        6,026,520.04 |    0.1% |        3,107.03 |          610.42 |  5.090 |         558.59 |    0.0% |      1.98 | `point reads`
+|              163.08 |        6,132,046.57 |    0.1% |        3,036.76 |          599.68 |  5.064 |         539.35 |    0.0% |      1.95 | `prefix reads`
+|              250.26 |        3,995,834.21 |    0.1% |        3,722.71 |          920.25 |  4.045 |         692.00 |    0.0% |      2.99 | `range reads`
+|              359.64 |        2,780,581.63 |    0.1% |        4,523.64 |        1,322.63 |  3.420 |         720.22 |    2.1% |      4.30 | `point writes`
+|              358.20 |        2,791,726.93 |    0.1% |        4,484.57 |        1,317.23 |  3.405 |         705.00 |    2.0% |      4.28 | `prefix writes`
+|              193.91 |        5,157,081.29 |    0.1% |        2,224.10 |          713.33 |  3.118 |         377.17 |    3.3% |      2.32 | `range writes`
+|              422.03 |        2,369,513.38 |    1.1% |        6,855.96 |        1,551.55 |  4.419 |       1,227.82 |    1.4% |      0.05 | `monotonic increasing point writes`
+|          142,467.00 |            7,019.17 |    3.8% |      812,047.33 |      508,535.40 |  1.597 |     151,246.50 |    1.0% |      0.01 | `worst case for radix tree`
+|               37.66 |       26,554,217.74 |    0.8% |          701.00 |          138.80 |  5.050 |         102.00 |    0.0% |      0.01 | `create and destroy`

 ## Radix tree (this implementation)


 |               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
 |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
-|               12.89 |       77,565,115.56 |    0.1% |          244.55 |           47.43 |  5.155 |          34.21 |    0.6% |      0.15 | `point reads`
-|               15.11 |       66,162,047.76 |    0.1% |          297.79 |           55.60 |  5.356 |          43.23 |    0.4% |      0.18 | `prefix reads`
-|               36.29 |       27,559,358.29 |    0.1% |          783.16 |          133.44 |  5.869 |         109.52 |    0.2% |      0.43 | `range reads`
-|               20.53 |       48,719,405.55 |    0.1% |          381.81 |           75.51 |  5.057 |          51.04 |    0.5% |      0.25 | `point writes`
-|               39.37 |       25,402,042.40 |    0.1% |          685.00 |          144.83 |  4.730 |         106.72 |    0.3% |      0.47 | `prefix writes`
-|               43.78 |       22,843,841.63 |    0.1% |          800.40 |          161.06 |  4.970 |         127.36 |    0.1% |      0.53 | `range writes`
-|               78.37 |       12,760,008.75 |    1.0% |        1,452.61 |          288.24 |  5.040 |         278.69 |    0.1% |      0.01 | `monotonic increasing point writes`
-|          322,885.50 |            3,097.07 |    1.5% |    4,362,382.00 |    1,183,852.00 |  3.685 |     765,301.00 |    0.1% |      0.01 | `worst case for radix tree`
-|               99.99 |       10,000,718.79 |    0.4% |        1,775.00 |          367.93 |  4.824 |         288.00 |    0.0% |      0.01 | `create and destroy`
+|               12.25 |       81,664,669.29 |    0.4% |          233.49 |           44.94 |  5.196 |          31.23 |    0.7% |      0.15 | `point reads`
+|               13.35 |       74,903,950.49 |    0.1% |          271.75 |           49.10 |  5.534 |          40.21 |    0.4% |      0.16 | `prefix reads`
+|               31.37 |       31,874,583.62 |    0.1% |          700.39 |          115.43 |  6.067 |         100.83 |    0.2% |      0.37 | `range reads`
+|               19.55 |       51,153,057.80 |    0.1% |          366.51 |           71.90 |  5.098 |          47.92 |    0.5% |      0.23 | `point writes`
+|               36.92 |       27,088,344.17 |    0.1% |          645.11 |          135.72 |  4.753 |         100.31 |    0.4% |      0.44 | `prefix writes`
+|               36.84 |       27,143,553.08 |    0.1% |          691.04 |          135.42 |  5.103 |         114.01 |    0.1% |      0.45 | `range writes`
+|               74.62 |       13,400,467.72 |    0.9% |        1,418.37 |          274.20 |  5.173 |         273.39 |    0.1% |      0.01 | `monotonic increasing point writes`
+|          352,671.00 |            2,835.50 |   15.3% |    3,687,535.00 |    1,287,119.00 |  2.865 |     712,030.00 |    0.1% |      0.01 | :wavy_dash: `worst case for radix tree` (Unstable with ~1.0 iters. Increase `minEpochIterations` to e.g. 10)
+|               96.63 |       10,348,729.42 |    1.2% |        1,766.00 |          355.53 |  4.967 |         283.00 |    0.0% |      0.01 | `create and destroy`

 # "Real data" test

@@ -51,13 +51,13 @@ Point queries only. Gc ratio is the ratio of time spent doing garbage collection
 ## skip list

 ```
-Check: 4.62967 seconds, 352.195 MB/s, Add: 3.34177 seconds, 167.771 MB/s, Gc ratio: 37.9399%, Peak idle memory: 5.51852e+06
+Check: 4.69875 seconds, 347.017 MB/s, Add: 3.39475 seconds, 165.153 MB/s, Gc ratio: 38.7414%, Peak idle memory: 5.51852e+06
 ```

 ## radix tree

 ```
-Check: 1.00477 seconds, 1622.8 MB/s, Add: 1.21142 seconds, 462.808 MB/s, Gc ratio: 39.4716%, Peak idle memory: 2.0226e+06
+Check: 0.976745 seconds, 1669.37 MB/s, Add: 1.20464 seconds, 465.411 MB/s, Gc ratio: 39.4158%, Peak idle memory: 2.0226e+06
 ```

 ## hash table
@@ -65,6 +65,6 @@ Check: 1.00477 seconds, 1622.8 MB/s, Add: 1.21142 seconds, 462.808 MB/s, Gc rati
 (The hash table implementation doesn't work on range queries, and its purpose is to provide an idea of how fast point queries can be)

 ```
-Check: 0.854254 seconds, 1908.74 MB/s, Add: 0.632626 seconds, 886.232 MB/s, Gc ratio: 41.0827%, Peak idle memory: 0
+Check: 0.864589 seconds, 1885.92 MB/s, Add: 0.634604 seconds, 883.47 MB/s, Gc ratio: 41.184%, Peak idle memory: 0
 ```

@@ -0,0 +1,115 @@
+// Exercises the SHOW_MEMORY instrumentation, which the other tests don't
+// compile with (this target passes -DSHOW_MEMORY=1). The instrumentation has
+// its own consistency checks, which abort on failure, and asserts are enabled
+// by TEST_FLAGS. Only one ConflictSet may exist at a time, since the
+// accounting is process-wide.
+#include "ConflictSet.h"
+#include "Internal.h"
+
+#include <cassert>
+#include <cstdint>
+#include <string>
+
+using namespace weaselab;
+
+namespace {
+
+void writePoint(ConflictSet &cs, const std::string &key, int64_t writeVersion) {
+  ConflictSet::WriteRange w{{(const uint8_t *)key.data(), int(key.size())},
+                            {nullptr, 0}};
+  cs.addWrites(&w, 1, writeVersion);
+}
+
+ConflictSet::Result checkPoint(const ConflictSet &cs, const std::string &key,
+                               int64_t readVersion) {
+  ConflictSet::ReadRange r{{(const uint8_t *)key.data(), int(key.size())},
+                           {nullptr, 0},
+                           readVersion};
+  ConflictSet::Result result;
+  cs.check(&r, &result, 1);
+  return result;
+}
+
+// Everything a ConflictSet allocated must have been accounted for and released
+// by the time it's destroyed.
+void checkFullyReleased() {
+  assert(nodeBytes == 0);
+  assert(partialCapacityBytes == 0);
+  assert(totalKeys == 0);
+  assert(keyBytes == 0);
+  assert(mallocBytes == 0);
+}
+
+} // namespace
+
+int main() {
+  // Splitting a node replaces it, which defers the release of the old node.
+  // Releasing a deferred node must account for its capacity, and the key it
+  // held must be accounted for without walking `parent` pointers of nodes that
+  // have already been released.
+  {
+    ConflictSet cs{0};
+    writePoint(cs, "a", 1);
+    writePoint(cs, "ab", 2);
+    assert(checkPoint(cs, "a", 0) == ConflictSet::Conflict);
+    assert(checkPoint(cs, "ab", 0) == ConflictSet::Conflict);
+    assert(checkPoint(cs, "b", 0) == ConflictSet::Commit);
+    assert(nodeBytes > 0);
+    assert(keyBytes > 0);
+  }
+  checkFullyReleased();
+
+  // Exceeding the version window erases the tree mid-workload.
+  {
+    ConflictSet cs{0};
+    writePoint(cs, "abc", 1);
+    const int64_t version = int64_t(1) << 40;
+    writePoint(cs, "abc", version);
+    assert(checkPoint(cs, "abc", 0) == ConflictSet::TooOld);
+    assert(checkPoint(cs, "abc", version) == ConflictSet::Commit);
+  }
+  checkFullyReleased();
+
+  // Erase a tree with many nodes, both mid-workload and at destruction.
+  {
+    ConflictSet cs{0};
+    for (int i = 0; i < 1000; ++i) {
+      writePoint(cs, "prefix/" + std::to_string(i), i + 1);
+    }
+    const int64_t version = int64_t(1) << 40;
+    writePoint(cs, "prefix/1", version);
+    assert(checkPoint(cs, "prefix/1", version) == ConflictSet::Commit);
+
+    for (int i = 0; i < 1000; ++i) {
+      writePoint(cs, "other/" + std::to_string(i), version + i + 1);
+    }
+    assert(checkPoint(cs, "other/999", version) == ConflictSet::Conflict);
+  }
+  checkFullyReleased();
+
+  // Grow a node through every node type, so that the accounting for each node
+  // type's size is exercised, then erase the entries one at a time so that
+  // nodes are released individually rather than as part of a whole tree.
+  {
+    ConflictSet cs{0};
+    std::string key(1, '\0');
+    for (int i = 0; i < 256; ++i) {
+      key[0] = char(i);
+      writePoint(cs, key, i + 1);
+    }
+    for (int i = 0; i < 256; ++i) {
+      key[0] = char(i);
+      assert(checkPoint(cs, key, i) == ConflictSet::Conflict);
+    }
+
+    // Garbage collection erases entries that can only fail with TooOld anyway.
+    // Doing it one version at a time walks the tree, erasing, downsizing and
+    // releasing nodes as it goes.
+    for (int i = 0; i < 256; ++i) {
+      cs.setOldestVersion(i + 1);
+    }
+    key[0] = '\0';
+    assert(checkPoint(cs, key, 0) == ConflictSet::TooOld);
+  }
+  checkFullyReleased();
+}
@@ -819,10 +819,17 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
  }

  void setOldestVersion(int64_t oldestVersion) {
+    // oldestVersion becomes the max of its previous value and the argument, so
+    // an older version is a no-op. We still fall through in the equal case so
+    // that garbage collection can make progress without advancing the oldest
+    // version.
+    if (oldestVersion < this->oldestVersion) {
+      return;
+    }
+
    // This isn't 100% accurate. It overcounts if you hit the end
    gc_iterations_total.add(keyUpdates);

-    assert(oldestVersion >= this->oldestVersion);
    this->oldestVersion = oldestVersion;
    SkipList::Finger finger;
    int temp;
@@ -35,36 +35,5 @@ int main(int argc, char **argv) {
        break;
      }
    }
-
-    {
-      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("");
-    }
  }
 }
@@ -2,6 +2,7 @@

 #include <cassert>
 #include <cstdio>
+#include <limits>

 using namespace weaselab;

@@ -23,6 +24,28 @@ int main(void) {
  int64_t bytes = cs.getBytes();
  assert(bytes > 0);

+  // Regression: a write just below INT64_MAX must not overflow the nominal
+  // version window comparison and erase entries that reads still observe.
+  // Versions are only required to be >= 0, so this is valid input.
+  {
+    const int64_t imax = std::numeric_limits<int64_t>::max();
+    const int64_t v0 = imax - int64_t(1000000000);
+    ConflictSet nearMax(0);
+    ConflictSet::WriteRange w2;
+    w2.begin.p = (const uint8_t *)"key";
+    w2.begin.len = 3;
+    w2.end.len = 0;
+    nearMax.addWrites(&w2, 1, v0);
+    nearMax.addWrites(&w2, 0, imax);
+    ConflictSet::ReadRange r2;
+    r2.begin.p = (const uint8_t *)"key";
+    r2.begin.len = 3;
+    r2.end.len = 0;
+    r2.readVersion = v0 - 1;
+    nearMax.check(&r2, &result, 1);
+    assert(result == ConflictSet::Conflict);
+  }
+
  ConflictSet::MetricsV1 *metrics;
  int metricsCount;
  cs.getMetricsV1(&metrics, &metricsCount);
@@ -21,7 +21,12 @@ limitations under the License.
 #ifdef __cplusplus
 namespace weaselab {
 /** A data structure for optimistic concurrency control on ranges of
- * bitwise-lexicographically-ordered keys. All versions must be >= 0.
+ * bitwise-lexicographically-ordered keys.
+ *
+ * All versions are >= 0. `oldestVersion` is the smallest version a subsequent
+ * `check` can read and `newestVersion` is the largest version passed to
+ * `addWrites` or the constructor. `oldestVersion <= newestVersion`, and
+ * `oldestVersion >= newestVersion - two billion`.
 *
 * Thread safety:
 *   - It's safe to operate on two different ConflictSets in two different
@@ -36,8 +41,7 @@ struct __attribute__((__visibility__("default"))) ConflictSet {
    /** The result of a check which intersects a write at a version >
       readVersion */
    Conflict,
-    /** The result of a check with a readVersion older than the highest call to
-       `setOldestVersion` */
+    /** The result of a check with a readVersion older than `oldestVersion` */
    TooOld
  };

@@ -54,9 +58,6 @@ struct __attribute__((__visibility__("default"))) ConflictSet {
    /** `end` having length 0 denotes that this range is the single key {begin}.
     * Otherwise this denotes the range [begin, end), and begin must be < end */
    Key end;
-    /** `readVersion` older than the oldestVersion or the version of the
-     * latest call to `addWrites` minus two billion will result in `TooOld`.
-     * Must be <= the version of the latest call to `addWrites` */
    int64_t readVersion;
  };

@@ -68,23 +69,38 @@ struct __attribute__((__visibility__("default"))) ConflictSet {
    Key end;
  };

-  /** The result of checking reads[i] is written in results[i] */
+  /** The result of checking reads[i] is written in results[i].
+   * @pre `reads` and `results` each hold at least `count` elements; each
+   * `reads[i]` is a valid `ReadRange` (see its field documentation); each
+   * `reads[i].readVersion <= newestVersion`.
+   * @post For every i < count, `results[i]` is `TooOld` if
+   * `reads[i].readVersion < oldestVersion`; `Conflict` if any write at a
+   * version greater than `reads[i].readVersion` intersects the range
+   * `reads[i]`; and `Commit` otherwise. */
  void check(const ReadRange *reads, Result *results, int count) const;

-  /** Reads intersecting writes where readVersion < `writeVersion` will result
-   * in `Conflict` (or `TooOld`, eventually).  `writeVersion` must be greater
-   * than or equal to all previous write versions. Call `addWrites` with `count`
-   * zero to only advance the version. */
+  /** Records writes at `writeVersion`, making them visible to `check`.
+   * @pre `writes` holds at least `count` elements; every `writes[i]` is a valid
+   * `WriteRange` (see its field documentation); `count >= 0`; `writeVersion >=
+   * newestVersion`.
+   * @post Writes (writes[i], writeVersion) are recorded for every i < count,
+   * and `newestVersion` becomes `writeVersion`. `oldestVersion` advances only
+   * as far as necessary: it becomes the larger of its previous value and
+   * `newestVersion - two billion`. Calling with `count == 0` adds no write;
+   * `newestVersion` (and possibly `oldestVersion`) still advance. */
  void addWrites(const WriteRange *writes, int count, int64_t writeVersion);

-  /** Reads where readVersion < `oldestVersion` will result in `TooOld`. Must be
-   * greater than or equal to all previous oldest versions. Must be <= the
-   * version of the latest call to `addWrites` */
+  /** Reads where readVersion < `oldestVersion` will result in `TooOld`.
+   * @pre `oldestVersion >= 0` and `oldestVersion <= newestVersion`.
+   * @post `oldestVersion` becomes the maximum of its previous value and the
+   * argument; `newestVersion` is unchanged. */
  void setOldestVersion(int64_t oldestVersion);

-  /** Reads where readVersion < oldestVersion will result in `TooOld`. There are
-   * no writes initially */
-  explicit ConflictSet(int64_t oldestVersion);
+  /** Reads where readVersion < `oldestVersion` will result in `TooOld`.
+   * @pre `version >= 0`.
+   * @post No writes are recorded, and `oldestVersion == newestVersion ==
+   * version`. */
+  explicit ConflictSet(int64_t version);

  ~ConflictSet();

@@ -146,7 +162,12 @@ private:
 #else

 /** A data structure for optimistic concurrency control on ranges of
- * bitwise-lexicographically-ordered keys. All versions must be >= 0.
+ * bitwise-lexicographically-ordered keys.
+ *
+ * All versions are >= 0. `oldestVersion` is the smallest version a subsequent
+ * `check` can read and `newestVersion` is the largest version passed to
+ * `addWrites` or the constructor. `oldestVersion <= newestVersion`, and
+ * `oldestVersion >= newestVersion - two billion`.
 *
 * Thread safety:
 *   - It's safe to operate on two different ConflictSets in two different
@@ -160,11 +181,10 @@ typedef struct ConflictSet ConflictSet;
 typedef enum {
  /** The result of a check which does not intersect any conflicting writes */
  ConflictSet_Commit,
-  /** The result of a check which intersects a write at a version > readVersion
-   */
+  /** The result of a check which intersects a write at a version >
+     readVersion */
  ConflictSet_Conflict,
-  /** The result of a check with a readVersion older than the highest call to
-     `setOldestVersion` */
+  /** The result of a check with a readVersion older than `oldestVersion` */
  ConflictSet_TooOld
 } ConflictSet_Result;

@@ -181,9 +201,6 @@ typedef struct {
  /** `end` having length 0 denotes that this range is the single key {begin}.
   * Otherwise this denotes the range [begin, end), and begin must be < end */
  ConflictSet_Key end;
-  /** `readVersion` older than the oldestVersion or the version of the
-   * latest call to `addWrites` minus two billion will result in `TooOld`.
-   * Must be <= the version of the latest call to `addWrites` */
  int64_t readVersion;
 } ConflictSet_ReadRange;

@@ -195,27 +212,43 @@ typedef struct {
  ConflictSet_Key end;
 } ConflictSet_WriteRange;

-/** The result of checking reads[i] is written in results[i] */
+/** The result of checking reads[i] is written in results[i].
+ * @pre `reads` and `results` each hold at least `count` elements; each
+ * `reads[i]` is a valid `ConflictSet_ReadRange` (see its field documentation);
+ * each `reads[i].readVersion <= newestVersion`.
+ * @post For every i < count, `results[i]` is `ConflictSet_TooOld` if
+ * `reads[i].readVersion < oldestVersion`; `ConflictSet_Conflict` if any write
+ * at a version greater than `reads[i].readVersion` intersects the range
+ * `reads[i]`; and `ConflictSet_Commit` otherwise. */
 void ConflictSet_check(const ConflictSet *cs,
                       const ConflictSet_ReadRange *reads,
                       ConflictSet_Result *results, int count);

-/** Reads intersecting writes where readVersion < `writeVersion` will result
- * in `Conflict` (or `TooOld`, eventually).  `writeVersion` must be greater
- * than or equal to all previous write versions. Call `addWrites` with `count`
- * zero to only advance the version. */
+/** Records writes at `writeVersion`, making them visible to
+ * `ConflictSet_check`.
+ * @pre `writes` holds at least `count` elements; every `writes[i]` is a valid
+ * `ConflictSet_WriteRange` (see its field documentation); `count >= 0`;
+ * `writeVersion >= newestVersion`.
+ * @post Writes (writes[i], writeVersion) are recorded for every i < count,
+ * and `newestVersion` becomes `writeVersion`. `oldestVersion` advances only
+ * as far as necessary: it becomes the larger of its previous value and
+ * `newestVersion - two billion`. Calling with `count == 0` adds no write;
+ * `newestVersion` (and possibly `oldestVersion`) still advance. */
 void ConflictSet_addWrites(ConflictSet *cs,
                           const ConflictSet_WriteRange *writes, int count,
                           int64_t writeVersion);

-/** Reads where readVersion < `oldestVersion` will result in `TooOld`. Must be
- * greater than or equal to all previous oldest versions. Must be <= the
- * version of the latest call to `addWrites` */
+/** Reads where readVersion < `oldestVersion` will result in `TooOld`.
+ * @pre `oldestVersion >= 0` and `oldestVersion <= newestVersion`.
+ * @post `oldestVersion` becomes the maximum of its previous value and the
+ * argument; `newestVersion` is unchanged. */
 void ConflictSet_setOldestVersion(ConflictSet *cs, int64_t oldestVersion);

-/** Reads where readVersion < oldestVersion will result in `TooOld`. There are
- * no writes initially */
-ConflictSet *ConflictSet_create(int64_t oldestVersion);
+/** Reads where readVersion < `oldestVersion` will result in `TooOld`.
+ * @pre `version >= 0`.
+ * @post No writes are recorded, and `oldestVersion == newestVersion ==
+ * version`. */
+ConflictSet *ConflictSet_create(int64_t version);

 void ConflictSet_destroy(ConflictSet *cs);

@@ -1,48 +0,0 @@
-#pragma once
-
-#include <cstdint>
-
-#if defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
-
-// SIMD operations on potentially-indeterminate Node16::index[16] bytes.
-// Implemented in file-level assembly (simd_x86_64.S) because loading and
-// operating on indeterminate values is UB in C++ but well-defined in
-// assembly. The caller must mask the returned bitfield to
-// [0, numChildren) before using it.
-//
-// Each function returns a 16-bit bitmask in the low 16 bits of a uint32_t
-// (upper 16 bits are zero). Bit i is set iff the condition holds at index i.
-
-extern "C" {
-// Returns bit i set iff idx[i] == key
-uint32_t find_eq_16(const uint8_t idx[16], uint8_t key);
-// Returns bit i set iff idx[i] >= child
-uint32_t find_ge_16(const uint8_t idx[16], uint8_t child);
-// Returns bit i set iff begin <= idx[i] < end
-uint32_t mask_in_range_16(const uint8_t idx[16], uint8_t begin, uint8_t end);
-}
-
-#elif defined(__aarch64__) && !defined(USE_SIMD_FALLBACK)
-
-// SIMD operations on potentially-indeterminate Node16::index[16] bytes.
-// Implemented in file-level assembly (simd_aarch64.S) because loading and
-// operating on indeterminate values is UB in C++ but well-defined in
-// assembly. The caller must mask the returned bitfield to [0, numChildren)
-// before using it.
-//
-// AArch64 has no pmovmskb-equivalent, so (unlike x86-64) each function returns
-// a 64-bit "nibble mask": nibble i (bits [4i, 4i+4)) is 0xf iff the condition
-// holds at index i. Bit (4i + 3) is the high bit of byte i's result. Callers
-// locate a set lane with std::countr_zero(bitfield) / 4 and mask the valid
-// lanes with (uint64_t(1) << (numChildren * 4)) - 1.
-
-extern "C" {
-// Returns nibble i = 0xf iff idx[i] == key
-uint64_t find_eq_16(const uint8_t idx[16], uint8_t key);
-// Returns nibble i = 0xf iff idx[i] >= child
-uint64_t find_ge_16(const uint8_t idx[16], uint8_t child);
-// Returns nibble i = 0xf iff begin <= idx[i] < end
-uint64_t mask_in_range_16(const uint8_t idx[16], uint8_t begin, uint8_t end);
-}
-
-#endif
@@ -1,82 +0,0 @@
-// SIMD operations on potentially-indeterminate Node16::index[16] bytes.
-// Written in assembly so msan doesn't track the loads, and so that loading
-// and operating on indeterminate values is well-defined (unlike C++). The
-// caller is responsible for masking the returned bitfield to [0, numChildren)
-// before using it.
-//
-// Unlike x86-64 (which has pmovmskb), AArch64 has no single instruction that
-// produces a 1-bit-per-byte mask. Each function therefore returns a 64-bit
-// "nibble mask" in x0: nibble i (bits [4i, 4i+4)) is 0xf iff the condition
-// holds at index i. Bit (4i + 3) is the high bit of byte i's result. Callers
-// locate a set lane with countr_zero(bitfield) / 4 and mask the valid lanes
-// with (uint64_t(1) << (numChildren * 4)) - 1.
-//
-// AArch64 AAPCS:
-//   x0 = const uint8_t *idx  (16 bytes, may contain indeterminate data)
-//   w1 = uint8_t key         (find_eq_16, find_ge_16)
-//   w1 = uint8_t begin       (mask_in_range_16)
-//   w2 = uint8_t end         (mask_in_range_16)
-
-    .text
-
-// uint64_t find_eq_16(const uint8_t idx[16], uint8_t key)
-//   nibble i = 0xf iff idx[i] == key
-    .globl find_eq_16
-    .type  find_eq_16, %function
-find_eq_16:
-    hint  34                 // bti c
-    dup   v1.16b, w1         // broadcast key
-    ldr   q0, [x0]           // load 16 bytes (may be indeterminate)
-    cmeq  v0.16b, v0.16b, v1.16b  // 0xff for each match
-    shrn  v0.8b, v0.8h, 4   // pack 16 byte-flags into 8 nibble-pairs
-    umov  x0, v0.d[0]
-    ret
-    .size find_eq_16, .-find_eq_16
-
-// uint64_t find_ge_16(const uint8_t idx[16], uint8_t child)
-//   nibble i = 0xf iff idx[i] >= child (unsigned)
-//   cmhs gives unsigned ">=" (higher-or-same): Vd = Vn >= Vm.
-    .globl find_ge_16
-    .type  find_ge_16, %function
-find_ge_16:
-    hint  34                 // bti c
-    dup   v1.16b, w1         // broadcast child
-    ldr   q0, [x0]           // load 16 bytes
-    cmhs  v0.16b, v0.16b, v1.16b  // 0xff where idx[i] >= child (unsigned)
-    shrn  v0.8b, v0.8h, 4
-    umov  x0, v0.d[0]
-    ret
-    .size find_ge_16, .-find_ge_16
-
-// uint64_t mask_in_range_16(const uint8_t idx[16], uint8_t begin, uint8_t end)
-//   nibble i = 0xf iff begin <= idx[i] < end  (unsigned, wrapping arithmetic)
-//   Logic: (idx[i] - begin) < (end - begin), valid when end - begin < 256.
-//   cmhi gives unsigned ">" (higher): Vd = Vn > Vm. We want
-//   (end - begin) > (idx - begin), so Vn = (end - begin).
-    .globl mask_in_range_16
-    .type  mask_in_range_16, %function
-mask_in_range_16:
-    hint  34                 // bti c
-    dup   v1.16b, w1         // broadcast begin
-    dup   v2.16b, w2         // broadcast end
-    ldr   q0, [x0]           // load 16 bytes
-    sub   v0.16b, v0.16b, v1.16b  // idx - begin (wrapping)
-    sub   v2.16b, v2.16b, v1.16b  // end - begin (range size)
-    cmhi  v0.16b, v2.16b, v0.16b  // 0xff where (end-begin) > (idx-begin)
-    shrn  v0.8b, v0.8h, 4
-    umov  x0, v0.d[0]
-    ret
-    .size mask_in_range_16, .-mask_in_range_16
-
-    // Declare AArch64 branch-protection compatibility, matching what the
-    // compiler emits for -mbranch-protection=standard (BTI + PAC + GCS). This
-    // keeps the object indistinguishable from C/C++ translation units for
-    // linkers enforcing BTI (-z force-bti); the functions above only use
-    // `bti c` and no stack, so PAC/GCS compatibility holds trivially.
-
-    .aeabi_subsection aeabi_feature_and_bits, optional, ULEB128
-    .aeabi_attribute Tag_Feature_BTI, 1
-    .aeabi_attribute Tag_Feature_PAC, 1
-    .aeabi_attribute Tag_Feature_GCS, 1
-
-    .section .note.GNU-stack,"",@progbits
@@ -1,85 +0,0 @@
-// SIMD operations on potentially-indeterminate Node16::index[16] bytes.
-// Written in assembly so msan doesn't track the loads. The caller is
-// responsible for masking the returned bitfield to [0, numChildren) before
-// using it.
-//
-// All functions return a 16-bit bitmask in %eax (bit i set = condition true
-// at index i). The upper 16 bits of %eax are zero.
-//
-// System V AMD64 ABI:
-//   %rdi = const uint8_t *idx  (16 bytes)
-//   %esi = uint8_t key         (find_eq_16, find_ge_16)
-//   %sil = uint8_t begin       (mask_in_range_16)
-//   %dl  = uint8_t end         (mask_in_range_16)
-
-    .text
-
-// uint32_t find_eq_16(const uint8_t idx[16], uint8_t key)
-//   Returns bit i set if idx[i] == key
-    .globl find_eq_16
-    .type  find_eq_16, @function
-find_eq_16:
-    vmovd      %esi, %xmm1          // broadcast key
-    vpbroadcastb %xmm1, %xmm1
-    vmovdqu    (%rdi), %xmm0        // load 16 bytes (may contain indeterminate data)
-    vpcmpeqb   %xmm0, %xmm1, %xmm0  // 0xff for each match
-    vpmovmskb  %xmm0, %eax          // 16-bit bitmask
-    movzwl     %ax, %eax            // zero-extend to 32 bits
-    ret
-    .size find_eq_16, .-find_eq_16
-
-// uint32_t find_ge_16(const uint8_t idx[16], uint8_t child)
-//   Returns bit i set if idx[i] >= child
-//   x86 doesn't have a "compare unsigned >=" for bytes directly, so we use:
-//   min(key, idx[i]) == key  iff  idx[i] >= key
-    .globl find_ge_16
-    .type  find_ge_16, @function
-find_ge_16:
-    vmovd      %esi, %xmm1
-    vpbroadcastb %xmm1, %xmm1       // key broadcast
-    vmovdqu    (%rdi), %xmm0        // load 16 bytes
-    vpminub    %xmm0, %xmm1, %xmm2  // min(key, idx[i])
-    vpcmpeqb   %xmm2, %xmm1, %xmm0  // 0xff where min == key, i.e. idx[i] >= key
-    vpmovmskb  %xmm0, %eax
-    movzwl     %ax, %eax
-    ret
-    .size find_ge_16, .-find_ge_16
-
-// uint32_t mask_in_range_16(const uint8_t idx[16], uint8_t begin, uint8_t end)
-//   Returns bit i set if begin <= idx[i] < end
-//   Logic: (idx[i] - begin) < (end - begin)  [unsigned wrapping arithmetic]
-//   Equivalently: idx[i] - begin != max(idx[i] - begin, end - begin)
-//   i.e. idx[i] - begin is NOT equal to the saturated value.
-//   We compute: sub = idx - begin; result = (sub < (end-begin)) for each byte.
-//   Using: sub == max(sub, end-begin) means NOT in range.
-//   So: in_range = ~(movemask(cmpeq(sub, max(sub, range_size))))
-    .globl mask_in_range_16
-    .type  mask_in_range_16, @function
-mask_in_range_16:
-    vmovd      %esi, %xmm1          // begin
-    vpbroadcastb %xmm1, %xmm1
-    vmovd      %edx, %xmm2          // end
-    vpbroadcastb %xmm2, %xmm2
-    vmovdqu    (%rdi), %xmm0        // load 16 bytes
-    vpsubb     %xmm1, %xmm0, %xmm0  // idx - begin (wrapping)
-    vpsubb     %xmm1, %xmm2, %xmm2  // end - begin (range size)
-    vpmaxub    %xmm0, %xmm2, %xmm3  // max(idx-begin, range_size)
-    vpcmpeqb   %xmm3, %xmm0, %xmm0  // 0xff where NOT in range
-    vpmovmskb  %xmm0, %eax
-    not        %eax                 // invert: 1 = in range
-    movzwl     %ax, %eax
-    ret
-    .size mask_in_range_16, .-mask_in_range_16
-
-    .section .note.gnu.property,"a",@note
-    .p2align 3, 0x0
-    .long 4
-    .long 16
-    .long 5
-    .asciz "GNU"
-    .long 0xc0000002
-    .long 4
-    .long 0x3
-    .p2align 3, 0x0
-
-    .section .note.GNU-stack,"",@progbits
@@ -68,6 +68,54 @@ def test_hash_table_getBytes():
        assert cs.check(read(0, b"key")) == [Result.CONFLICT]


+def test_setOldestVersion_does_not_decrease():
+    # Regression test for issue #87: `setOldestVersion` sets oldestVersion to
+    # the maximum of its previous value and its argument, so passing an older
+    # version is valid input and must be a no-op. skip_list used to assert in
+    # Debug builds and, in Release builds, lower its oldest version, which
+    # resurrected a window of versions that callers were told resulted in
+    # TooOld.
+    with DebugConflictSet() as cs:
+        cs.addWrites(5, write(b"k"))
+        cs.setOldestVersion(5)
+        # Valid: `2 >= 0` and `2 <= newestVersion == 5`. oldestVersion stays 5,
+        # so 3 and 4 are still TooOld.
+        cs.setOldestVersion(2)
+        assert cs.check(read(3, b"k")) == [Result.TOO_OLD]
+        assert cs.check(read(4, b"k")) == [Result.TOO_OLD]
+        assert cs.check(read(5, b"k")) == [Result.COMMIT]
+
+    # Same, but with the entries at the old oldestVersion already garbage
+    # collected, so lowering oldestVersion would report Commit for a version
+    # that the caller was told resulted in TooOld.
+    with DebugConflictSet() as cs:
+        cs.addWrites(1, write(b"a"))
+        cs.addWrites(2)  # No writes; advances newestVersion to 2
+        cs.setOldestVersion(2)  # Garbage collects the write at version 1
+        cs.setOldestVersion(1)
+        assert cs.check(read(1, b"a")) == [Result.TOO_OLD]
+        assert cs.check(read(2, b"a")) == [Result.COMMIT]
+
+
+def test_hash_table_setOldestVersion_empties_map():
+    # Regression test for issue #78: setOldestVersion must terminate when
+    # its garbage-collection pass erases every entry in the map. Each write
+    # charges keyUpdates += 2, so with 50 writes keyUpdates = 100 and GC is
+    # enabled; erasing all entries leaves keyUpdates > 0 while the map is
+    # empty, which previously caused the GC loop to spin forever.
+    with ConflictSet(0, build_dir=build_dir, implementation="hash_table") as cs:
+        for i in range(50):
+            cs.addWrites(1, write(("k%06d" % i).encode()))
+        # All entries have version 1 <= oldestVersion 1 -> all erased.
+        cs.setOldestVersion(1)
+        # If we reach this point, setOldestVersion returned instead of hanging.
+        # The keys were erased, so reads at a valid version commit.
+        assert cs.check(read(1, b"k000000")) == [Result.COMMIT]
+        assert cs.check(read(2, b"k000000")) == [Result.COMMIT]
+        # A read older than oldestVersion is TooOld.
+        assert cs.check(read(0, b"k000000")) == [Result.TOO_OLD]
+
+
 def test_write_read_without_outer_reference():
    # Regression test for issue #42: WriteRange/ReadRange must keep their
    # backing key buffers alive, because the C library reads the pointer
@@ -203,13 +251,18 @@ def test_merge_child_node48():
        cs.addWrites(1, write(b"\x00" * 8, b"\x00" * 10))


-def test_fixup_256():
+def test_fixup_256_heterogeneous_versions():
+    # Regression test for issue #89: recomputing the maxVersion of a Node256
+    # must consider all of its children, not just those with an index < 16.
+    # Here the highest-version child ("\x10\x50") has an index >= 16, and the
+    # range write ends at the Node256, which erases that child from the range
+    # and recomputes the Node256's maxVersion.
    with DebugConflictSet() as cs:
-        cs.addWrites(0, write(bytes([1])))
        for i in range(256):
-            cs.addWrites(1, write(bytes([1, i])))
-        cs.addWrites(2, write(bytes([0]), bytes([1])))
-        cs.check(read(0, bytes([1]), bytes([2])))
+            cs.addWrites(1, write(bytes([0x10, i])))
+        cs.addWrites(10, write(bytes([0x10, 0x50])))
+        cs.addWrites(11, write(bytes([0x05]), bytes([0x10])))
+        assert cs.check(read(5, bytes([0x10, 0x50]))) == [Result.CONFLICT]


 def test_large_removal_buffer():
@@ -1,7 +1,8 @@
 #!/bin/bash

 # Disable frequency scaling
-for i in $(seq "$(nproc)") ; do sudo cat /sys/devices/system/cpu/cpu$((i-1))/cpufreq/scaling_max_freq | sudo tee /sys/devices/system/cpu/cpu$((i-1))/cpufreq/scaling_min_freq >/dev/null ; done
+# shellcheck disable=SC2002
+for i in $(seq "$(nproc)") ; do cat /sys/devices/system/cpu/cpu$((i-1))/cpufreq/scaling_max_freq | sudo tee /sys/devices/system/cpu/cpu$((i-1))/cpufreq/scaling_min_freq >/dev/null ; done

 cmake -S. -Bbuild -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_COMPILER=clang++ -G Ninja -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_CXX_FLAGS=-DNVALGRIND > /dev/null
 cmake --build build > /dev/null
Author	SHA1	Message	Date
andrew	e23018c952	Merge pull request 'Improve perf/codegen' (#94 ) from perf into main Reviewed-on: weaselab/conflict-set#94	2026-10-27 20:45:52 +00:00
andrew	b1c1f8f2ee	Merge pull request 'Replace test_fixup_256 with a regression test for the Node256 fixupMaxVersion false-Commit bug' (#90 ) from weaselbot/conflict-set:weaselbot/issue-89 into main Reviewed-on: weaselab/conflict-set#90	2026-10-27 19:33:17 +00:00
andrew	4bec2cf4eb	Merge pull request 'Make SHOW_MEMORY builds work: fix deferred node capacity assert and eraseTree use-after-free' (#91 ) from weaselbot/conflict-set:weaselbot/issue-88 into main Reviewed-on: weaselab/conflict-set#91	2026-10-27 19:32:43 +00:00
andrew	fa1701479c	Merge pull request 'SkipList: don't lower oldestVersion in setOldestVersion' (#92 ) from weaselbot/conflict-set:weaselbot/issue-87 into main Reviewed-on: weaselab/conflict-set#92	2026-10-27 19:30:11 +00:00
andrew	20e2e332e4	Merge pull request 'Skip DEBUG_VERBOSE end print for point writes (uninitialized Result::endRemaining)' (#93 ) from weaselbot/conflict-set:weaselbot/issue-86 into main Reviewed-on: weaselab/conflict-set#93	2026-10-27 19:29:26 +00:00
andrew	23d092ab69	Avoid IFUNC for short prefixes in LCP	2026-10-27 15:09:58 -04:00
andrew	84bf5707f9	Skip IFUNC call for Node3 in fixupMaxVersion	2026-10-27 14:21:58 -04:00
weaselbot	c94e9aaee6	Remove test_fixup_256, which cannot detect the bug Every child of the Node256 in test_fixup_256 is written at the same version, so max(childMaxVersion[0..15]) and max(maxOfMax) agree and the buggy code passes. test_fixup_256_heterogeneous_versions covers the same Node256-under-a-range-end shape with distinct versions, so keep only that.	2026-10-27 14:10:36 -04:00
andrew	d71d963d9a	Improve codegen for checkMaxBetweenExclusiveImpl(Node3 $ ninja -C build && nm -C -S build/CMakeFiles/conflict-set-object.dir/ConflictSet.cpp.o \| grep 'checkMaxBetweenExclusive(Node3' ... 0000000000000000 0000000000000529 T checkMaxBetweenExclusive(Node3, int, int, InternalVersionT, ReadContext) $ git stash pop ... $ ninja -C build && nm -C -S build/CMakeFiles/conflict-set-object.dir/ConflictSet.cpp.o \| grep 'checkMaxBetweenExclusive(Node3' ... 0000000000000000 0000000000000106 T checkMaxBetweenExclusive(Node3, int, int, InternalVersionT, ReadContext)	2026-10-27 14:06:58 -04:00
andrew	dabddbcb56	Use individual comparison operators instead of operator<=> This gives better codegen	2026-10-27 13:55:44 -04:00
andrew	3b4271dac1	Update readme	2026-10-27 13:44:11 -04:00
weaselbot	a792352387	Skip DEBUG_VERBOSE end print for point writes Point writes are recorded with the 2-argument interleaved_insert::Result constructor, which leaves endRemaining indeterminate, so the Phase 2 debug print in interleavedWrites read an indeterminate span and, with stack garbage left over from an earlier call, dereferenced a freed pointer. Confirmed with valgrind on a point write following a range write: "Conditional jump or move depends on uninitialised value(s)" and "Invalid read of size 1" at ConflictSet.cpp:4984, and ASan heap-use-after-free in the same place. Only print the end search path and spans for range writes, mirroring the `if (e != nullptr)` check that already guards the endInsertionPoint forwarding-chain fixup a few lines above. Production code already treats endRemaining as range-write only, so nothing else changes.	2026-10-27 13:07:00 -04:00
weaselbot	56e0673fc7	Make the skip list corpus tests opt-in, not default Review feedback: running the skip list tests on every default ctest run is a waste of time. Gate them behind a TEST_SKIP_LIST CMake option, off by default, so they remain easy to run when cross-checking SkipList.cpp against ReferenceImpl. Verified with -DTEST_SKIP_LIST=ON: all 2845 skip_list corpus tests pass; the default test list is unchanged.	2026-10-27 13:02:45 -04:00
weaselbot	80f084cc43	Compile eraseTree's search path length bookkeeping out of non-SHOW_MEMORY builds `eraseTree` only carries the search path lengths of queued nodes around so that the SHOW_MEMORY key accounting doesn't have to walk `parent` pointers of nodes that have already been released. Put the lengths on a parallel stack that only exists when SHOW_MEMORY is enabled, so that other builds keep the upstream worklist of bare `Node *` and don't compute anything extra.	2026-10-27 12:56:55 -04:00
weaselbot	c0bd10effa	Drop redundant coverage exclusion markers in the excluded tail region `getNodeSize`'s unreachable `default:` sits inside ConflictSet.cpp's `GCOVR_EXCL_START`/`GCOVR_EXCL_STOP` block, so the `GCOVR_EXCL_LINE` markers on those two lines are redundant: they cannot change the report, but they make gcovr print (WARNING) GCOVR_EXCL_LINE found on line 6002 in excluded region started on line 5535 once per line for each of the four reports the coverage job writes. Drop them, matching the rest of the excluded tail, which carries no exclusion markers. Context: the previous coverage job for this branch did not fail in any of its steps. Build, Test (5754/5754 passed), Coverage report (`gcovr --fail-under-line 100` reported 100% for ConflictSet.cpp, LongestCommonPrefix.h and Metrics.h), the upload and the test summary all reported success; the job was marked failed while tearing the job container down. The post-actions (actions/cache, actions/checkout) could no longer `docker exec` into the container and the runner's rootless podman socket was gone ("Cannot connect to the Docker daemon at unix:///run/user/992/podman/podman.sock", "Error while stop job container"); the actions/cache service on 192.168.0.110:41379 had been refusing connections for the whole run. Verified locally on this commit: Debug, gcc and `--coverage` builds of the coverage job's configuration; 5754/5754 tests pass; merged line coverage (union over the library, SHOW_MEMORY and fuzz-driver objects, honouring the GCOVR_EXCL_START/STOP regions) is 100% for all three measured files, and no GCOVR_EXCL_LINE markers remain inside an excluded region.	2026-10-26 21:47:41 -04:00
weaselbot	df986810ff	Give the SHOW_MEMORY instrumentation a single definition per function The coverage job compiles ConflictSet.cpp twice: once without SHOW_MEMORY, like the rest of the build, and once with it for the SHOW_MEMORY test. Each config previously saw a different definition of the accounting helpers: no-op `constexpr` stubs near the top of the file when SHOW_MEMORY was off, and the real definitions at the bottom when it was on. gcovr reads the .gcda files for a source file and merges them, and its default (strict) function merge mode asserts that a function name always maps to one line. Since `addNode`, `removeNode`, `addKey`, both `removeKey` overloads and `getSearchPathLength` were reported on two different lines, report generation aborted with AssertionError: Got function getSearchPathLength(Node*) on multiple lines: 99, 6002. Define those functions in exactly one place instead, and compile the accounting out from within the bodies. Then every configuration reports each function on the same line and the coverage data can be merged. The empty calls are inlined away: the Release shared library contains no calls to them. With the merge fixed, the accounting lines that only exist in the SHOW_MEMORY build show up in the report, so the coverage job's 100% line requirement covers them. Extend the SHOW_MEMORY test to grow a node through every node type (so `getNodeSize` covers every case) and to erase entries one at a time via `setOldestVersion` (so `removeKey` and the `internal_setOldestVersion` cross-check are covered). The remaining `abort()`s in the accounting are defensive checks that can never be taken on a passing run; mark them GCOVR_EXCL_LINE like the rest of the unreachable code in this file.	2026-10-26 20:34:45 -04:00
weaselbot	8b6389236a	Enable the skip list corpus tests by default driver_skip_list checks SkipList.cpp against ReferenceImpl, but its tests were disabled, so the implementation diverged from the documented API without anything noticing. The corpus tests pass in Debug and Release builds, so run them like the other implementations' tests.	2026-10-26 18:46:02 -04:00
weaselbot	b22d5f4816	SkipList: don't lower oldestVersion in setOldestVersion setOldestVersion is documented to set oldestVersion to the maximum of its previous value and its argument, so passing a version older than the current oldestVersion is valid input. SkipList.cpp instead asserted that the argument was monotonically non-decreasing, aborting Debug builds, and with NDEBUG it lowered oldestVersion. That resurrected a window of versions that callers had been told result in TooOld: reads in that window were evaluated against entries that may already have been garbage collected, so they could report Commit or Conflict instead. Leave oldestVersion alone when the argument isn't newer, mirroring HashTable.cpp. Falling through on equality keeps garbage collection making progress without advancing the oldest version. Add a scripted regression test, which bisimulates against the radix tree.	2026-10-26 18:46:00 -04:00
weaselbot	bba328f79f	Fix SHOW_MEMORY crashes on valid input `Node::getCapacity()` asserted `!releaseDeferred`, but releasing a deferred node requires its capacity, so any node replacement aborted an assertion- enabled `SHOW_MEMORY` build. `partialKeyCapacity` is not repurposed by `deferRelease`, so there is no reason to reject deferred nodes. Drop the assertion. `eraseTree` released a node before visiting its children, so the `parent` pointers walked by `removeKey` pointed to freed memory (and `deferRelease` repurposes `parent` as a list link). Carry the search path length of each node in the worklist, computed while its parent is still alive, and account for it with the new `removeKey` overload. Add a test covering the SHOW_MEMORY instrumentation, which the rest of the test suite doesn't compile with.	2026-10-26 18:32:03 -04:00
weaselbot	3d3184ef5f	Add regression test for Node256 fixupMaxVersion	2026-10-26 18:20:37 -04:00
andrew	ba781b8875	Spec that addWrites advances oldestVersion minimally The old wording only gave the lower bound oldestVersion >= newestVersion - two billion, which an implementation could satisfy by discarding more readable versions than necessary. State that oldestVersion becomes the larger of its previous value and newestVersion - two billion.	2026-10-10 14:04:17 -04:00
andrew	8621662d3c	Don't poison Node48 reverseIndex Follows the reasoning of "Remove redundant version update": a write can never lower the logical max version of a key range, so there's no read that didn't pass before that should pass after a write. The check stays exact reading the physical childMaxVersion and reverseIndex of unused slots, so scan16's whole-page reads of them are well-defined and intended. The client requests only produced valgrind false positives. Keep the initialization, drop the poisoning.	2026-10-10 13:58:58 -04:00
andrew	9c721bcdca	Stop printing metrics from TestDriver The per-corpus-file Prometheus dump was debug-only output that nothing consumes. It also floods stdout well past ctest's failed-output cap, so the real failure (e.g. a valgrind report) never made it into the CI summary.	2026-10-10 13:55:02 -04:00
andrew	4ef10b106a	Fix wrong-result bug. Previously it could incorrectly report Commit Blog post forthcoming	2026-10-09 16:34:33 -04:00
andrew	9b6dd51b85	Add a spec for fixupMaxVersion	2026-10-09 16:28:36 -04:00
andrew	647ba5beb1	Remove redundant version update A write can never lower the logical max version of a key range. There's no read that didn't pass before that should pass after a write.	2026-10-09 16:19:20 -04:00
andrew	2f9f49c692	Use doxygen pre/post tags and make public api more precise	2026-10-09 15:09:58 -04:00
andrew	6a6e36909f	Fix assert memcmp size	2026-10-09 15:05:39 -04:00
andrew	46d1f05333	Avoid signed overflow This only happens at implausibly high version numbers, but the fix is straightforward so we'll just do it anyway. Closes #84	2026-09-23 11:36:12 -04:00
andrew	8a16f9a6d6	Merge pull request 'Fix broken endInsertionPoint forwarding chain resolution in DEBUG_VERBOSE print' (#83 ) from weaselbot/conflict-set:weaselbot/issue-82 into main Reviewed-on: weaselab/conflict-set#83	2026-08-31 01:14:10 +00:00
weaselbot	9a2c64d31c	Fix broken endInsertionPoint forwarding chain resolution in DEBUG_VERBOSE print In the Phase 2 debug block of ConflictSet::Impl::interleavedWrites, the loop resolving the end insertion point's releaseDeferred/forwardTo chain followed the begin node's chain instead (e = b->forwardTo). Once b is resolved, b->forwardTo aliases the Entry union member, so e was assigned garbage derived from version bytes and the loop dereferenced it, causing a SEGV or hang whenever DEBUG_VERBOSE builds traced writes. Use the end node's own forwarding chain (e = e->forwardTo), matching the production code in the same loop body and in Phase 3.	2026-08-30 20:17:22 -04:00
andrew	ec1b476977	Merge pull request 'Fix NEON type mismatch in conflictMask16 for gcc/aarch64 + USE_64_BIT=1' (#81 ) from weaselbot/conflict-set:weaselbot/issue-80 into main Reviewed-on: http://git.weaselab.dev/weaselab/conflict-set/pulls/81	2026-08-18 17:39:48 +00:00
weaselbot	8c55e835c4	Fix NEON type mismatch in conflictMask16 for gcc/aarch64 + USE_64_BIT=1 vcgtq_s64 returns uint64x2_t but vmovn_s64 requires int64x2_t. clang accepts the implicit conversion via lax vector conversions, but gcc does not. Add an explicit vreinterpretq_s64_u64 so the code compiles with gcc on aarch64 when USE_64_BIT=1. This path is otherwise untested in CI (aarch64 matrices use USE_64_BIT=0; the 64-bit-versions matrix uses amd64 AVX compare16). Closes #80	2026-08-16 21:16:23 -04:00
andrew	af76c4d623	Merge pull request 'Fix hash_table setOldestVersion infinite loop on empty map' (#79 ) from weaselbot/conflict-set:weaselbot/issue-78 into main Reviewed-on: http://git.weaselab.dev/weaselab/conflict-set/pulls/79	2026-08-17 00:25:27 +00:00
weaselbot	6db080a34c	Fix hash_table setOldestVersion infinite loop on empty map setOldestVersion's GC pass charges keyUpdates at 2x the entry count, so when every entry has version <= oldestVersion and is erased, keyUpdates remains > 0 while the map is empty. The outer while(keyUpdates > 0) loop then reset iter to map.begin() (== end() for an empty map) and the inner for loop never executed, spinning forever. Break out of the while loop when the map is exhausted, so the call returns normally. Closes #78	2026-08-16 13:02:50 -04:00
andrew	cdbad1c013	Merge pull request 'Replace SIMD assembly with initialized Node16::index + valgrind client request' (#77 ) from client-request-instead-of-asm into main Reviewed-on: weaselab/conflict-set#77	2026-08-04 00:07:19 +00:00
andrew	6d9810fe7f	Poison Node48 reverseIndex slot on erase When a Node48 loses a child, the vacated reverseIndex slot would otherwise remain defined, letting valgrind miss reads that rely on the stale value. Mark it undefined, mirroring the existing poisoning of the vacated children slot.	2026-08-03 17:38:21 -04:00
andrew	bae2b22e95	Replace SIMD assembly with initialized Node16::index + valgrind client request Commit `6fed133` moved SIMD operations on Node16::index into file-level assembly (later extended to aarch64) to avoid reading indeterminate bytes (UB in C++). Revert that in favor of a simpler, architecture-independent fix: - Restore the original HAS_AVX / HAS_ARM_NEON intrinsics at the affected call sites, keeping the newer conflictMask16 helper and scalar fallback. - memset Node16::index in both copy constructors (from Node3 and Node48) so the [numChildren, 16) tail is always initialized, making the full 16-byte SIMD loads well-defined. This also fixes the same latent UB in the NEON path and the Node16->Node16 full-array memcpy. - Issue a VALGRIND_MAKE_MEM_UNDEFINED client request on the tail so valgrind still flags any read that relies on the unused slots rather than numChildren. - Drop simd.h, simd_x86_64.S, simd_aarch64.S, and the CMake ASM wiring.	2026-08-03 17:32:41 -04:00
andrew	b11e92ee0b	Merge pull request 'Move aarch64 Node16 SIMD index loads into assembly' (#76 ) from weaselbot/conflict-set:weaselbot/issue-68 into main Reviewed-on: weaselab/conflict-set#76	2026-08-03 13:42:45 +00:00
weaselbot	e02dd41f74	Address review: drop unused bti c landing pads, fix rationale comment	2026-08-03 09:17:28 -04:00
andrew	b0e326aacc	Merge pull request 'ci: run MSan on arm64' (#74 ) from weaselbot/conflict-set:weaselbot/issue-73 into main Reviewed-on: weaselab/conflict-set#74	2026-08-03 12:27:49 +00:00
andrew	1179c1f56e	Merge pull request 'Fix ARM NEON scan helpers truncating 64-bit versions' (#75 ) from weaselbot/conflict-set:weaselbot/issue-69 into main Reviewed-on: weaselab/conflict-set#75	2026-08-03 12:26:03 +00:00
andrew	bdbac1556d	Merge pull request 'Correct rationale in simd_x86_64.S comment' (#72 ) from weaselbot/conflict-set:weaselbot/issue-71 into main Reviewed-on: weaselab/conflict-set#72	2026-08-03 04:59:26 +00:00
andrew	0103837488	Merge pull request 'Run valgrind in arm64 release in CI' (#70 ) from valgrind-in-arm into main Reviewed-on: weaselab/conflict-set#70	2026-08-03 04:34:34 +00:00
weaselbot	6b1f597d77	Fix ARM NEON scan helpers truncating 64-bit versions When USE_64_BIT=1, InternalVersionT stores an int64_t, but the aarch64 NEON fast paths in scan16 (both the indexed Node48 variant and the Node256 variant) and checkMaxBetweenExclusiveImpl(Node16) only copied the low 32 bits of each version into uint32x4_t lanes and compared against a truncated 32-bit readVersion. This produced wrong conflict/commit decisions once any version exceeded 2^32 - 1. Extract the per-version "greater than readVersion" computation into a shared conflictMask16 helper. For 64-bit versions it uses vcgtq_s64 on the full int64 values and narrows the resulting 16-byte mask into the same nibble-packed layout the callers already consume. The 32-bit path is preserved unchanged. Closes #69	2026-08-02 23:35:32 -04:00
weaselbot	70630649d9	ci: run MSan on arm64 Add a debug-arm64 matrix entry to the test job that runs the MSan fuzz tests on an arm64 runner. The arm instrumented libc++ toolchain (22.1.8) is built against LLVM 22, so this entry installs clang-22 from apt.llvm.org while the existing amd64 entries keep clang-21. Generalize the test job matrix with runner/arch/llvm_version/msan_url fields so the runner, apt cache key, LLVM toolchain version, and MSan toolchain download URL are selected per matrix entry. Closes #73	2026-08-02 23:28:40 -04:00
weaselbot	0b12a037c4	Correct rationale in simd_x86_64.S comment The SIMD operations are written in assembly because loading and operating on indeterminate values is UB in C++ but well-defined in assembly. msan not tracking the loads is a side effect, not the reason. Closes #71	2026-08-03 02:37:18 +00:00
andrew	79113ecb88	Run valgrind in arm64 release in CI	2026-08-03 02:36:45 +00:00