Add GCOVR_EXCL_LINE

This was showing as uncovered in CI for some reason

.gitea/workflows/ci.yml

@@ -0,0 +1,306 @@
+name: CI
+
+on: [push, pull_request]
+
+env:
+  CC: clang
+  CXX: clang++
+
+jobs:
+  pre-commit:
+    runs-on: ubuntu-latest-amd64
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/cache@v4
+        with:
+          path: /var/cache/apt/archives
+          key: apt-amd64-${{ hashFiles('.gitea/workflows/ci.yml') }}
+
+      - name: Install dependencies
+        run: |
+          sudo apt-get update -qq
+          sudo apt-get install -y git nodejs pre-commit
+
+      - uses: actions/cache@v4
+        with:
+          path: ~/.cache/pre-commit
+          key: pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}
+
+      - name: Run pre-commit
+        run: |
+          git config --global --add safe.directory "$PWD"
+          pre-commit run --all-files --show-diff-on-failure
+
+  test:
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - name: 64-bit-versions
+            cmake_args: -DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1
+          - name: debug
+            cmake_args: -DCMAKE_BUILD_TYPE=Debug
+          - name: simd-fallback
+            cmake_args: -DUSE_SIMD_FALLBACK=ON
+          - name: gcc
+            cmake_args: -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++
+    runs-on: ubuntu-latest-amd64
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/cache@v4
+        with:
+          path: /var/cache/apt/archives
+          key: apt-amd64-${{ hashFiles('.gitea/workflows/ci.yml') }}
+
+      - name: Install common dependencies
+        run: |
+          sudo apt-get update -qq
+          sudo apt-get install -y \
+            build-essential ccache cmake gcc g++ libc6-dbg \
+            ninja-build python3 valgrind zstd
+          sudo curl -Ls "https://minio.weaselab.dev/public/$(uname -m)/mc.RELEASE.2025-08-13T08-35-41Z" \
+            -o /usr/local/bin/mc && sudo chmod +x /usr/local/bin/mc
+
+      - name: Install LLVM toolchain
+        if: matrix.name != 'gcc'
+        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
+          sudo apt-get update -qq
+          sudo apt-get install -y \
+            clang-21 llvm-21 lld-21 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
+          done
+
+      - name: Build
+        run: |
+          export CCACHE_DIR="$GITHUB_WORKSPACE/.ccache"
+          rm -rf build
+          cmake -S . -B build -G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache ${{ matrix.cmake_args }}
+          ninja -C build
+          ccache -s
+
+      - name: Test
+        run: |
+          cd build
+          ctest --no-compress-output --test-output-size-passed 100000 --test-output-size-failed 100000 -T Test -j "$(nproc)" --timeout 90 > /dev/null
+
+      - name: Upload test results to MinIO
+        if: always()
+        env:
+          MINIO_ACCESS_KEY: ${{ secrets.MINIO_ACCESS_KEY }}
+          MC_HOST_minio: https://${{ secrets.MINIO_ACCESS_KEY }}:${{ secrets.MINIO_SECRET_KEY }}@minio.weaselab.dev
+        run: |
+          if [ -z "$MINIO_ACCESS_KEY" ]; then
+            echo "MinIO credentials not configured; skipping upload"
+            exit 0
+          fi
+          zstd build/Testing/*/Test.xml
+          mc cp build/Testing/*/Test.xml.zst "minio/jenkins/conflict-set/${{ gitea.run_number }}/${{ matrix.name }}/"
+
+      - name: Test summary
+        if: always()
+        run: |
+          python3 ctest_summary.py build/Testing/*/Test.xml \
+            --link "https://minio.weaselab.dev/jenkins/conflict-set/${{ gitea.run_number }}/${{ matrix.name }}/Test.xml.zst" \
+            | tee -a "$GITHUB_STEP_SUMMARY"
+
+  release:
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - runner: ubuntu-latest-amd64
+            arch: amd64
+          - runner: ubuntu-latest-arm64
+            arch: arm64
+    runs-on: ${{ matrix.runner }}
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/cache@v4
+        with:
+          path: /var/cache/apt/archives
+          key: apt-${{ matrix.arch }}-${{ hashFiles('.gitea/workflows/ci.yml') }}
+
+      - name: Install dependencies
+        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
+          sudo apt-get update -qq
+          sudo apt-get install -y \
+            build-essential ccache clang-21 cmake devscripts \
+            libc6-dbg llvm-21 lld-21 mold ninja-build rpm \
+            valgrind zstd
+          sudo curl -Ls "https://minio.weaselab.dev/public/$(uname -m)/mc.RELEASE.2025-08-13T08-35-41Z" \
+            -o /usr/local/bin/mc && sudo chmod +x /usr/local/bin/mc
+          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
+          done
+
+      - name: Install paper build dependencies
+        if: matrix.arch == 'amd64'
+        run: |
+          sudo apt-get install -y \
+            biber latexmk \
+            texlive-bibtex-extra texlive-fonts-recommended \
+            texlive-latex-extra texlive-pictures
+
+      - uses: actions/cache@v4
+        with:
+          path: .ccache
+          key: ccache-release-${{ matrix.arch }}-${{ gitea.sha }}
+          restore-keys: |
+            ccache-release-${{ matrix.arch }}-
+
+      - name: Build
+        run: |
+          export CCACHE_DIR="$GITHUB_WORKSPACE/.ccache"
+          rm -rf build
+          cmake -S . -B build -G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_FLAGS=-DNVALGRIND
+          ninja -C build
+          ccache -s
+
+      - 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
+
+      - name: Package
+        run: |
+          cd build
+          cpack -G DEB
+          cpack -G RPM
+
+      - name: Build paper
+        if: matrix.arch == 'amd64'
+        run: |
+          cd paper
+          make
+
+      - name: Upload artifacts to MinIO
+        if: always()
+        shell: bash
+        env:
+          MINIO_ACCESS_KEY: ${{ secrets.MINIO_ACCESS_KEY }}
+          MC_HOST_minio: https://${{ secrets.MINIO_ACCESS_KEY }}:${{ secrets.MINIO_SECRET_KEY }}@minio.weaselab.dev
+        run: |
+          if [ -z "$MINIO_ACCESS_KEY" ]; then
+            echo "MinIO credentials not configured; skipping upload"
+            exit 0
+          fi
+          dest="minio/jenkins/conflict-set/${{ gitea.run_number }}/release-${{ matrix.arch }}/"
+          zstd build/Testing/*/Test.xml
+          mc cp build/Testing/*/Test.xml.zst "$dest"
+          if compgen -G "build/*.deb" > /dev/null; then
+            mc cp build/*.deb "$dest"
+          fi
+          if compgen -G "build/*.rpm" > /dev/null; then
+            mc cp build/*.rpm "$dest"
+          fi
+          if compgen -G "paper/*.pdf" > /dev/null; then
+            mc cp paper/*.pdf "$dest"
+          fi
+
+      - name: Test summary
+        if: always()
+        run: |
+          python3 ctest_summary.py build/Testing/*/Test.xml \
+            --link "https://minio.weaselab.dev/jenkins/conflict-set/${{ gitea.run_number }}/release-${{ matrix.arch }}/Test.xml.zst" \
+            | tee -a "$GITHUB_STEP_SUMMARY"
+
+  coverage:
+    runs-on: ubuntu-latest-amd64
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/cache@v4
+        with:
+          path: /var/cache/apt/archives
+          key: apt-amd64-${{ hashFiles('.gitea/workflows/ci.yml') }}
+
+      - name: Install dependencies
+        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
+          sudo apt-get update -qq
+          sudo apt-get install -y \
+            build-essential ccache clang-21 cmake gcovr \
+            libc6-dbg llvm-21 lld-21 mold ninja-build python3 valgrind zstd
+          sudo curl -Ls "https://minio.weaselab.dev/public/$(uname -m)/mc.RELEASE.2025-08-13T08-35-41Z" \
+            -o /usr/local/bin/mc && sudo chmod +x /usr/local/bin/mc
+          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
+          done
+
+      - uses: actions/cache@v4
+        with:
+          path: .ccache
+          key: ccache-coverage-${{ gitea.sha }}
+          restore-keys: |
+            ccache-coverage-
+
+      - name: Build
+        run: |
+          export CCACHE_DIR="$GITHUB_WORKSPACE/.ccache"
+          rm -rf build
+          cmake -S . -B build -G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
+            -DCMAKE_C_FLAGS=--coverage -DCMAKE_CXX_FLAGS=--coverage \
+            -DCMAKE_BUILD_TYPE=Debug -DDISABLE_TSAN=ON
+          ninja -C build
+          ccache -s
+
+      - name: Test
+        run: |
+          cd build
+          ctest --no-compress-output --test-output-size-passed 100000 --test-output-size-failed 100000 -T Test -j "$(nproc)" --timeout 90 > /dev/null
+
+      - name: Coverage report
+        shell: bash
+        run: |
+          gcov_args=(-f ConflictSet.cpp -f LongestCommonPrefix.h -f Metrics.h
+                     --gcov-executable "llvm-cov gcov" --exclude-noncode-lines)
+          gcovr "${gcov_args[@]}" --cobertura > build/coverage.xml
+          gcovr "${gcov_args[@]}"
+          mkdir -p build/coverage_html
+          gcovr "${gcov_args[@]}" --html-details build/coverage_html/index.html
+          gcovr "${gcov_args[@]}" --fail-under-line 100 > /dev/null
+
+      - name: Upload results to MinIO
+        if: always()
+        env:
+          MINIO_ACCESS_KEY: ${{ secrets.MINIO_ACCESS_KEY }}
+          MC_HOST_minio: https://${{ secrets.MINIO_ACCESS_KEY }}:${{ secrets.MINIO_SECRET_KEY }}@minio.weaselab.dev
+        run: |
+          if [ -z "$MINIO_ACCESS_KEY" ]; then
+            echo "MinIO credentials not configured; skipping upload"
+            exit 0
+          fi
+          dest="minio/jenkins/conflict-set/${{ gitea.run_number }}/coverage/"
+          zstd build/Testing/*/Test.xml
+          mc cp build/Testing/*/Test.xml.zst "$dest"
+          if [ -e build/coverage.xml ]; then
+            mc cp build/coverage.xml "$dest"
+          fi
+          if [ -d build/coverage_html ]; then
+            mc cp -r build/coverage_html "$dest"
+          fi
+
+      - name: Test summary
+        if: always()
+        run: |
+          python3 ctest_summary.py build/Testing/*/Test.xml \
+            --link "https://minio.weaselab.dev/jenkins/conflict-set/${{ gitea.run_number }}/coverage/Test.xml.zst" \
+            | tee -a "$GITHUB_STEP_SUMMARY"
+          echo "" | tee -a "$GITHUB_STEP_SUMMARY"
+          echo "📊 [Coverage report](https://minio.weaselab.dev/jenkins/conflict-set/${{ gitea.run_number }}/coverage/coverage_html/index.html)" | tee -a "$GITHUB_STEP_SUMMARY"

CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.18)
 project(
   conflict-set
-  VERSION 0.0.12
+  VERSION 0.0.13
   DESCRIPTION
     "A data structure for optimistic concurrency control on ranges of bitwise-lexicographically-ordered keys."
   HOMEPAGE_URL "https://git.weaselab.dev/weaselab/conflict-set"

ConflictSet.cpp

@@ -87,22 +87,42 @@ constexpr int64_t kMaxCorrectVersionWindow =
     std::numeric_limits<int32_t>::max();
 static_assert(kNominalVersionWindow <= kMaxCorrectVersionWindow);
 
+#ifndef USE_64_BIT
+#define USE_64_BIT 0
+#endif
+
 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 {
+#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;
+#if USE_64_BIT
+  static const InternalVersionT zero;
+#else
   static thread_local InternalVersionT zero;
+#endif
 
 private:
+#if USE_64_BIT
+  int64_t value;
+#else
   uint32_t value;
+#endif
 };
+#if USE_64_BIT
+const InternalVersionT InternalVersionT::zero{0};
+#else
 thread_local InternalVersionT InternalVersionT::zero;
+#endif
 
 struct Entry {
   InternalVersionT pointVersion;
@@ -518,8 +538,13 @@ std::string getSearchPath(Node *n);
 // Each node with an entry present gets a budget of kBytesPerKey. Node0 always
 // has an entry present.
 // Induction hypothesis is that each node's surplus is >= kMinNodeSurplus
+#if USE_64_BIT
+constexpr int kBytesPerKey = 144;
+constexpr int kMinNodeSurplus = 104;
+#else
 constexpr int kBytesPerKey = 112;
 constexpr int kMinNodeSurplus = 80;
+#endif
 // Cound the entry itself as a child
 constexpr int kMinChildrenNode0 = 1;
 constexpr int kMinChildrenNode3 = 2;
@@ -724,9 +749,13 @@ struct WriteContext {
     int64_t write_bytes;
   } accum;
 
+#if USE_64_BIT
+  static constexpr InternalVersionT zero{0};
+#else
   // Cache a copy of InternalVersionT::zero, so we don't need to do the TLS
   // lookup as often.
   InternalVersionT zero;
+#endif
 
   WriteContext() { memset(&accum, 0, sizeof(accum)); }
 
@@ -1563,6 +1592,9 @@ void rezero16(InternalVersionT *vs, InternalVersionT zero) {
   }
 }
 
+#if USE_64_BIT
+void rezero(Node *, InternalVersionT) {}
+#else
 void rezero(Node *n, InternalVersionT z) {
 #if DEBUG_VERBOSE && !defined(NDEBUG)
   fprintf(stderr, "rezero to %" PRId64 ": %s\n", z.toInt64(),
@@ -1605,6 +1637,7 @@ void rezero(Node *n, InternalVersionT z) {
     __builtin_unreachable(); // GCOVR_EXCL_LINE
   }
 }
+#endif
 
 void mergeWithChild(Node *&self, WriteContext *tls, ConflictSet::Impl *impl,
                     Node *&dontInvalidate, Node3 *self3) {
@@ -1987,7 +2020,14 @@ downLeftSpine:
 }
 
 #ifdef HAS_AVX
-uint32_t compare16_32bit(const InternalVersionT *vs, InternalVersionT rv) {
+uint32_t compare16(const InternalVersionT *vs, InternalVersionT rv) {
+#if USE_64_BIT
+  uint32_t compared = 0;
+  for (int i = 0; i < 16; ++i) {
+    compared |= (vs[i] > rv) << i;
+  }
+  return compared;
+#else
   uint32_t compared = 0;
   __m128i w[4]; // GCOVR_EXCL_LINE
   memcpy(w, vs, sizeof(w));
@@ -2001,15 +2041,26 @@ uint32_t compare16_32bit(const InternalVersionT *vs, InternalVersionT rv) {
                 << (i * 4);
   }
   return compared;
+#endif
 }
 
 __attribute__((target("avx512f"))) uint32_t
-compare16_32bit_avx512(const InternalVersionT *vs, InternalVersionT rv) {
+compare16_avx512(const InternalVersionT *vs, InternalVersionT rv) {
+#if USE_64_BIT
+  int64_t r;
+  memcpy(&r, &rv, sizeof(r));
+  uint32_t low =
+      _mm512_cmpgt_epi64_mask(_mm512_loadu_epi64(vs), _mm512_set1_epi64(r));
+  uint32_t high =
+      _mm512_cmpgt_epi64_mask(_mm512_loadu_epi64(vs + 8), _mm512_set1_epi64(r));
+  return low | (high << 8);
+#else
   uint32_t r;
   memcpy(&r, &rv, sizeof(r));
   return _mm512_cmpgt_epi32_mask(
       _mm512_sub_epi32(_mm512_loadu_epi32(vs), _mm512_set1_epi32(r)),
       _mm512_setzero_epi32());
+#endif
 }
 #endif
 
@@ -2066,9 +2117,9 @@ bool scan16(const InternalVersionT *vs, const uint8_t *is, int begin, int end,
 
   uint32_t compared = 0;
   if constexpr (kAVX512) {
-    compared = compare16_32bit_avx512(vs, readVersion); // GCOVR_EXCL_LINE
+    compared = compare16_avx512(vs, readVersion);
   } else {
-    compared = compare16_32bit(vs, readVersion); // GCOVR_EXCL_LINE
+    compared = compare16(vs, readVersion);
   }
   return !(compared & mask);
 
@@ -2127,9 +2178,9 @@ bool scan16(const InternalVersionT *vs, int begin, int end,
 #elif defined(HAS_AVX)
   uint32_t conflict;
   if constexpr (kAVX512) {
-    conflict = compare16_32bit_avx512(vs, readVersion); // GCOVR_EXCL_LINE
+    conflict = compare16_avx512(vs, readVersion);
   } else {
-    conflict = compare16_32bit(vs, readVersion); // GCOVR_EXCL_LINE
+    conflict = compare16(vs, readVersion);
   }
   conflict &= (1 << end) - 1;
   conflict >>= begin;
@@ -2264,12 +2315,9 @@ bool checkMaxBetweenExclusiveImpl(Node *n, int begin, int end,
 
     uint32_t compared = 0;
     if constexpr (kAVX512) {
-      compared =                                        // GCOVR_EXCL_LINE
+      compared = compare16_avx512(self->childMaxVersion, readVersion);
-          compare16_32bit_avx512(self->childMaxVersion, // GCOVR_EXCL_LINE
-                                 readVersion);          // GCOVR_EXCL_LINE
     } else {
-      compared = compare16_32bit(self->childMaxVersion,
+      compared = compare16(self->childMaxVersion, readVersion);
-                                 readVersion); // GCOVR_EXCL_LINE
     }
     return !(compared & mask) && firstRangeOk;
 
@@ -2864,8 +2912,7 @@ checkMaxBetweenExclusiveImpl<true>(Node *n, int begin, int end,
 // of the result will have `maxVersion` set to `writeVersion` as a
 // postcondition. Nodes along the search path may be invalidated. Callers must
 // ensure that the max version of the self argument is updated.
-[[nodiscard]]
+[[nodiscard]] Node **insert(Node **self, std::span<const uint8_t> key,
-Node **insert(Node **self, std::span<const uint8_t> key,
                             InternalVersionT writeVersion, WriteContext *tls) {
 
   for (; key.size() != 0; ++tls->accum.insert_iterations) {
@@ -3101,6 +3148,8 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     tls.impl = this;
     int64_t check_byte_accum = 0;
     for (int i = 0; i < count; ++i) {
+      assert(reads[i].readVersion >= 0);
+      assert(reads[i].readVersion <= newestVersionFullPrecision);
       const auto &r = reads[i];
       check_byte_accum += r.begin.len + r.end.len;
       auto begin = std::span<const uint8_t>(r.begin.p, r.begin.len);
@@ -3137,10 +3186,12 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   }
 
   void addWrites(const WriteRange *writes, int count, int64_t writeVersion) {
+#if !USE_64_BIT
     // There could be other conflict sets in the same thread. We need
     // InternalVersionT::zero to be correct for this conflict set for the
     // lifetime of the current call frame.
     InternalVersionT::zero = tls.zero = oldestVersion;
+#endif
 
     assert(writeVersion >= newestVersionFullPrecision);
     assert(tls.accum.entries_erased == 0);
@@ -3155,7 +3206,10 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 
       newestVersionFullPrecision = writeVersion;
       newest_version.set(newestVersionFullPrecision);
+      if (newestVersionFullPrecision - kNominalVersionWindow >
+          oldestVersionFullPrecision) {
         setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
+      }
       while (oldestExtantVersion <
              newestVersionFullPrecision - kMaxCorrectVersionWindow) {
         gcScanStep(1000);
@@ -3163,8 +3217,11 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     } else {
       newestVersionFullPrecision = writeVersion;
       newest_version.set(newestVersionFullPrecision);
+      if (newestVersionFullPrecision - kNominalVersionWindow >
+          oldestVersionFullPrecision) {
         setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
       }
+    }
 
     for (int i = 0; i < count; ++i) {
       const auto &w = writes[i];
@@ -3248,14 +3305,22 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     return fuel;
   }
 
-  void setOldestVersion(int64_t o) {
+  void setOldestVersion(int64_t newOldestVersion) {
-    if (o <= oldestVersionFullPrecision) {
+    assert(newOldestVersion >= 0);
+    assert(newOldestVersion <= newestVersionFullPrecision);
+    // If addWrites advances oldestVersion to keep within valid window, a
+    // subsequent setOldestVersion can be legitimately called with a version
+    // older than `oldestVersionFullPrecision`. < instead of <= so that we can
+    // do garbage collection work without advancing the oldest version.
+    if (newOldestVersion < oldestVersionFullPrecision) {
       return;
     }
-    InternalVersionT oldestVersion{o};
+    InternalVersionT oldestVersion{newOldestVersion};
-    this->oldestVersionFullPrecision = o;
+    this->oldestVersionFullPrecision = newOldestVersion;
     this->oldestVersion = oldestVersion;
+#if !USE_64_BIT
     InternalVersionT::zero = tls.zero = oldestVersion;
+#endif
 #ifdef NDEBUG
     // This is here for performance reasons, since we want to amortize the cost
     // of storing the search path as a string. In tests, we want to exercise the
@@ -3304,12 +3369,15 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     root->entry.pointVersion = this->oldestVersion;
     root->entry.rangeVersion = this->oldestVersion;
 
+#if !USE_64_BIT
     InternalVersionT::zero = tls.zero = this->oldestVersion;
+#endif
 
     // Intentionally not resetting totalBytes
   }
 
   explicit Impl(int64_t oldestVersion) {
+    assert(oldestVersion >= 0);
     init(oldestVersion);
     initMetrics();
   }
@@ -3580,7 +3648,12 @@ ConflictSet::ConflictSet(ConflictSet &&other) noexcept
     : impl(std::exchange(other.impl, nullptr)) {}
 
 ConflictSet &ConflictSet::operator=(ConflictSet &&other) noexcept {
+  if (this != &other) {
+    if (impl) {
+      internal_destroy(impl);
+    }
     impl = std::exchange(other.impl, nullptr);
+  }
   return *this;
 }
 
@@ -3704,13 +3777,13 @@ std::string getSearchPath(Node *n) {
         fprintf(file,
                 " k_%p [label=\"m=%" PRId64 " p=%" PRId64 " r=%" PRId64
                 "\n%s\", pos=\"%d,%d!\"];\n",
-                (void *)n, maxVersion(n).toInt64(),
+                (void *)n, n->parent == nullptr ? -1 : maxVersion(n).toInt64(),
                 n->entry.pointVersion.toInt64(),
                 n->entry.rangeVersion.toInt64(),
                 getPartialKeyPrintable(n).c_str(), x, y);
       } else {
         fprintf(file, " k_%p [label=\"m=%" PRId64 "\n%s\", pos=\"%d,%d!\"];\n",
-                (void *)n, maxVersion(n).toInt64(),
+                (void *)n, n->parent == nullptr ? -1 : maxVersion(n).toInt64(),
                 getPartialKeyPrintable(n).c_str(), x, y);
       }
       x += kSeparation;
@@ -3751,6 +3824,9 @@ Node *firstGeq(Node *n, std::string_view key) {
       n, std::span<const uint8_t>((const uint8_t *)key.data(), key.size()));
 }
 
+#if USE_64_BIT
+void checkVersionsGeqOldestExtant(Node *, InternalVersionT) {}
+#else
 void checkVersionsGeqOldestExtant(Node *n,
                                   InternalVersionT oldestExtantVersion) {
   if (n->entryPresent) {
@@ -3794,6 +3870,7 @@ void checkVersionsGeqOldestExtant(Node *n,
     abort();
   }
 }
+#endif
 
 [[maybe_unused]] InternalVersionT
 checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,

HashTable.cpp

@@ -119,7 +119,13 @@ ConflictSet::ConflictSet(ConflictSet &&other) noexcept
     : impl(std::exchange(other.impl, nullptr)) {}
 
 ConflictSet &ConflictSet::operator=(ConflictSet &&other) noexcept {
+  if (this != &other) {
+    if (impl) {
+      impl->~Impl();
+      safe_free(impl, sizeof(Impl));
+    }
     impl = std::exchange(other.impl, nullptr);
+  }
   return *this;
 }
 

Internal.h

@@ -20,7 +20,6 @@ using namespace weaselab;
 #include <thread>
 #include <unordered_set>
 #include <utility>
-#include <vector>
 
 #include <callgrind.h>
 

Jenkinsfile

@@ -48,6 +48,17 @@ pipeline {
                 recordIssues(tools: [clang()])
             }
         }
+        stage('64 bit versions') {
+            agent {
+                dockerfile {
+                    args '-v /home/jenkins/ccache:/ccache'
+                    reuseNode true
+                }
+            }
+            steps {
+                CleanBuildAndTest("-DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1")
+            }
+        }
         stage('Debug') {
             agent {
                 dockerfile {

LongestCommonPrefix.h

@@ -129,7 +129,7 @@ longestCommonPrefix(const uint8_t *ap, const uint8_t *bp, int cl) {
   }
 
   int i = 0;
-  int end;
+  int end; // GCOVR_EXCL_LINE
 
   // kStride * kUnrollCount at a time
   end = cl & ~(kStride * kUnrollFactor - 1);

SkipList.cpp

@@ -25,6 +25,7 @@
 
 #include <algorithm>
 #include <span>
+#include <vector>
 
 std::span<const uint8_t> keyAfter(Arena &arena, std::span<const uint8_t> key) {
   auto result =
@@ -115,15 +116,6 @@ bool operator==(const KeyInfo &lhs, const KeyInfo &rhs) {
   return !(lhs < rhs || rhs < lhs);
 }
 
-void swapSort(std::vector<KeyInfo> &points, int a, int b) {
-  if (points[b] < points[a]) {
-    KeyInfo temp;
-    temp = points[a];
-    points[a] = points[b];
-    points[b] = temp;
-  }
-}
-
 struct SortTask {
   int begin;
   int size;
@@ -183,13 +175,6 @@ void sortPoints(std::vector<KeyInfo> &points) {
   }
 }
 
-static thread_local uint32_t g_seed = 0;
-
-static inline int skfastrand() {
-  g_seed = g_seed * 1664525L + 1013904223L;
-  return g_seed;
-}
-
 static int compare(const StringRef &a, const StringRef &b) {
   int c = memcmp(a.data(), b.data(), std::min(a.size(), b.size()));
   if (c < 0)
@@ -215,21 +200,25 @@ struct ReadConflictRange {
   }
 };
 
-class SkipList {
-private:
 static constexpr int MaxLevels = 26;
 
-  int randomLevel() const {
+struct RandomLevel {
-    uint32_t i = uint32_t(skfastrand()) >> (32 - (MaxLevels - 1));
+  explicit RandomLevel(uint32_t seed) : seed(seed) {}
-    int level = 0;
+
-    while (i & 1) {
+  int next() {
-      i >>= 1;
+    int result = __builtin_clz(seed | (uint32_t(-1) >> (MaxLevels - 1)));
-      level++;
+    seed = seed * 1664525L + 1013904223L;
-    }
+    return result;
-    assert(level < MaxLevels);
-    return level;
   }
 
+private:
+  uint32_t seed;
+};
+
+class SkipList {
+private:
+  RandomLevel randomLevel{0};
+
   // Represent a node in the SkipList. The node has multiple (i.e., level)
   // pointers to other nodes, and keeps a record of the max versions for each
   // level.
@@ -426,18 +415,23 @@ public:
   }
   void swap(SkipList &other) { std::swap(header, other.header); }
 
-  void addConflictRanges(const Finger *fingers, int rangeCount,
+  // Returns the change in the number of entries
+  int64_t addConflictRanges(const Finger *fingers, int rangeCount,
                             Version version) {
+    int64_t result = rangeCount;
     for (int r = rangeCount - 1; r >= 0; r--) {
       const Finger &startF = fingers[r * 2];
       const Finger &endF = fingers[r * 2 + 1];
 
-      if (endF.found() == nullptr)
+      if (endF.found() == nullptr) {
+        ++result;
         insert(endF, endF.finger[0]->getMaxVersion(0));
+      }
 
-      remove(startF, endF);
+      result -= remove(startF, endF);
       insert(startF, version);
     }
+    return result;
   }
 
   void detectConflicts(ReadConflictRange *ranges, int count,
@@ -567,9 +561,10 @@ public:
   }
 
 private:
-  void remove(const Finger &start, const Finger &end) {
+  // Returns the number of entries removed
+  int64_t remove(const Finger &start, const Finger &end) {
     if (start.finger[0] == end.finger[0])
-      return;
+      return 0;
 
     Node *x = start.finger[0]->getNext(0);
 
@@ -578,17 +573,20 @@ private:
       if (start.finger[i] != end.finger[i])
         start.finger[i]->setNext(i, end.finger[i]->getNext(i));
 
+    int64_t result = 0;
     while (true) {
       Node *next = x->getNext(0);
       x->destroy();
+      ++result;
       if (x == end.finger[0])
         break;
       x = next;
     }
+    return result;
   }
 
   void insert(const Finger &f, Version version) {
-    int level = randomLevel();
+    int level = randomLevel.next();
     // std::cout << std::string((const char*)value,length) << " level: " <<
     // level << std::endl;
     Node *x = Node::create(f.value, level);
@@ -704,8 +702,6 @@ private:
   };
 };
 
-struct SkipListConflictSet {};
-
 struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   Impl(int64_t oldestVersion)
       : oldestVersion(oldestVersion), newestVersion(oldestVersion),
@@ -775,17 +771,20 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     StringRef values[stripeSize];
     int64_t writeVersions[stripeSize / 2];
     int ss = stringCount - (stripes - 1) * stripeSize;
+    int64_t entryDelta = 0;
     for (int s = stripes - 1; s >= 0; s--) {
       for (int i = 0; i * 2 < ss; ++i) {
         const auto &w = combinedWriteConflictRanges[s * stripeSize / 2 + i];
         values[i * 2] = w.first;
         values[i * 2 + 1] = w.second;
-        keyUpdates += 3;
       }
       skipList.find(values, fingers, temp, ss);
-      skipList.addConflictRanges(fingers, ss / 2, writeVersion);
+      entryDelta += skipList.addConflictRanges(fingers, ss / 2, writeVersion);
       ss = stripeSize;
     }
+
+    // Run gc at least 200% the rate we're inserting entries
+    keyUpdates += std::max<int64_t>(entryDelta, 0) * 2;
   }
 
   void setOldestVersion(int64_t oldestVersion) {
@@ -795,7 +794,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     int temp;
     std::span<const uint8_t> key = removalKey;
     skipList.find(&key, &finger, &temp, 1);
-    skipList.removeBefore(oldestVersion, finger, std::exchange(keyUpdates, 10));
+    skipList.removeBefore(oldestVersion, finger, std::exchange(keyUpdates, 0));
     removalArena = Arena();
     removalKey = copyToArena(
         removalArena, {finger.getValue().data(), finger.getValue().size()});
@@ -804,7 +803,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   int64_t totalBytes = 0;
 
 private:
-  int64_t keyUpdates = 10;
+  int64_t keyUpdates = 0;
   Arena removalArena;
   std::span<const uint8_t> removalKey;
   int64_t oldestVersion;
@@ -904,7 +903,12 @@ ConflictSet::ConflictSet(ConflictSet &&other) noexcept
     : impl(std::exchange(other.impl, nullptr)) {}
 
 ConflictSet &ConflictSet::operator=(ConflictSet &&other) noexcept {
+  if (this != &other) {
+    if (impl) {
+      internal_destroy(impl);
+    }
     impl = std::exchange(other.impl, nullptr);
+  }
   return *this;
 }
 

ctest_summary.py

@@ -0,0 +1,76 @@
+#!/usr/bin/env python3
+"""Summarize a CTest Test.xml as markdown.
+
+Intended for $GITHUB_STEP_SUMMARY in CI, where only the first few failures
+are shown inline (pass --link to point at the full Test.xml). Also reusable
+locally to print every failure from a downloaded Test.xml with --all.
+"""
+
+import argparse
+import base64
+import gzip
+import xml.etree.ElementTree as ET
+
+# Failure output is truncated to this many trailing characters, which is
+# usually enough to include e.g. an ASan report's summary.
+OUTPUT_TAIL_CHARS = 3000
+
+
+def test_output(test):
+    value = test.find("./Results/Measurement/Value")
+    if value is None or value.text is None:
+        return ""
+    text = value.text
+    if value.get("encoding") == "base64":
+        raw = base64.b64decode(text)
+        if value.get("compression") == "gzip":
+            raw = gzip.decompress(raw)
+        text = raw.decode(errors="replace")
+    return text
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("test_xml", help="path to a ctest Testing/*/Test.xml")
+    parser.add_argument(
+        "--inline",
+        type=int,
+        default=5,
+        help="how many failures to show inline (default 5)",
+    )
+    parser.add_argument("--all", action="store_true", help="show every failure inline")
+    parser.add_argument(
+        "--link", help="URL of the full Test.xml, linked when failures are elided"
+    )
+    args = parser.parse_args()
+
+    testing = ET.parse(args.test_xml).getroot().find("Testing")
+    tests = testing.findall("Test")
+    failed = [t for t in tests if t.get("Status") == "failed"]
+    notrun = sum(1 for t in tests if t.get("Status") == "notrun")
+
+    if not failed:
+        print(f"✅ All {len(tests) - notrun} tests passed")
+    else:
+        print(f"❌ {len(failed)} of {len(tests)} tests failed\n")
+        shown = failed if args.all else failed[: args.inline]
+        for test in shown:
+            name = test.findtext("Name")
+            output = test_output(test)[-OUTPUT_TAIL_CHARS:].strip()
+            print(f"<details><summary><code>{name}</code></summary>\n")
+            print("````")
+            print(output)
+            print("````")
+            print("</details>\n")
+        remaining = len(failed) - len(shown)
+        if remaining > 0:
+            more = f"… and {remaining} more"
+            if args.link:
+                more += f" — full list in [Test.xml]({args.link})"
+            print(more)
+    if notrun:
+        print(f"\n⚠️ {notrun} tests not run")
+
+
+if __name__ == "__main__":
+    main()

include/ConflictSet.h

@@ -54,8 +54,9 @@ 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 the oldestVersion or the version of the
+    /** `readVersion` older than the oldestVersion or the version of the
-     * latest call to `addWrites` minus two billion will result in `TooOld` */
+     * latest call to `addWrites` minus two billion will result in `TooOld`.
+     * Must be <= the version of the latest call to `addWrites` */
     int64_t readVersion;
   };
 
@@ -72,11 +73,13 @@ struct __attribute__((__visibility__("default"))) ConflictSet {
 
   /** 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. */
+   * than or equal to all previous write versions. Call `addWrites` with `count`
+   * zero to only advance the version. */
   void addWrites(const WriteRange *writes, int count, int64_t writeVersion);
 
-  /** Reads where readVersion < oldestVersion will result in `TooOld`. Must be
+  /** Reads where readVersion < `oldestVersion` will result in `TooOld`. Must be
-   * greater than or equal to all previous oldest versions. */
+   * greater than or equal to all previous oldest versions. Must be <= the
+   * version of the latest call to `addWrites` */
   void setOldestVersion(int64_t oldestVersion);
 
   /** Reads where readVersion < oldestVersion will result in `TooOld`. There are
@@ -170,8 +173,9 @@ 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 the oldestVersion or the version of the
+  /** `readVersion` older than the oldestVersion or the version of the
-   * latest call to `addWrites` minus two billion will result in `TooOld` */
+   * 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;
 
@@ -188,15 +192,17 @@ void ConflictSet_check(const ConflictSet *cs,
                        const ConflictSet_ReadRange *reads,
                        ConflictSet_Result *results, int count);
 
-/** Reads intersecting writes where readVersion < `writeVersion` will result in
+/** Reads intersecting writes where readVersion < `writeVersion` will result
- * `Conflict` (or `TooOld`, eventually).  `writeVersion` must be greater than or
+ * in `Conflict` (or `TooOld`, eventually).  `writeVersion` must be greater
- * equal to all previous write versions.  */
+ * than or equal to all previous write versions. Call `addWrites` with `count`
+ * zero to only advance the version. */
 void ConflictSet_addWrites(ConflictSet *cs,
                            const ConflictSet_WriteRange *writes, int count,
                            int64_t writeVersion);
 
-/** Reads where readVersion < oldestVersion will result in `TooOld`. Must be
+/** Reads where readVersion < `oldestVersion` will result in `TooOld`. Must be
- * greater than or equal to all previous oldest versions. */
+ * greater than or equal to all previous oldest versions. Must be <= the
+ * version of the latest call to `addWrites` */
 void ConflictSet_setOldestVersion(ConflictSet *cs, int64_t oldestVersion);
 
 /** Reads where readVersion < oldestVersion will result in `TooOld`. There are

paper/paper.tex

@@ -206,8 +206,11 @@ until we end at $a_{i} + 1$, adjacent to the first inner range.
 
 A few notes on implementation:
 \begin{itemize}
-    \item{For clarity, the above algorithm decouples the logical partitioning from the physical structure of the tree. An optimized implementation would merge adjacent prefix ranges that don't correspond to nodes in the tree as it scans, so that it only calculates the version of such merged ranges once. Additionally, our implementation stores an index of which child pointers are valid as a bitset for Node48 and Node256 to speed up this scan using techniques inspired by \cite{Lemire_2018}.}
+    \item{For clarity, the above algorithm decouples the logical partitioning from the physical structure of the tree.
-    \item{In order to avoid many costly pointer indirections, we can store the max version not in each node itself but next to each node's parent pointer. Without this, the range read performance is not competetive with the skip list.}
+          An optimized implementation would merge adjacent prefix ranges that don't correspond to nodes in the tree as it scans, so that it only calculates the version of such merged ranges once.
+          Additionally, our implementation uses SIMD instructions and instruction-level parallelism to compare many prefix ranges to the read version $r$ in parallel.}
+    \item{In order to avoid many costly pointer indirections, and to take advantage of SIMD, we can store the max version of child nodes as a dense array directly in the parent node.
+          Without this, the range read performance is not competetive with the skip list.}
     \item{An optimized implementation would visit the partition of $[a_{i}\dots a_{m}, a_{i} + 1)$ in reverse order, as it descends along the search path to $a_{i}\dots a_{m}$}
     \item{An optimized implementation would search for the common prefix first, and return early if any prefix of the common prefix has a $max \leq r$.}
 \end{itemize}

test_conflict_set.py

@@ -96,6 +96,7 @@ def test_inner_full_words():
 
 def test_internal_version_zero():
     with DebugConflictSet() as cs:
+        cs.addWrites(0xFFFFFFF0)
         cs.setOldestVersion(0xFFFFFFF0)
         for i in range(24):
             cs.addWrites(0xFFFFFFF1, write(bytes([i])))