Get back to 100% coverage

Closes #30

This is achieved by running libfuzzer with USE_32_BIT_VERSIONS={OFF,ON},
and then combining the corpora. I suspect that the problem earlier was
that we only had the 32 bit corpus but were measuring coverage for 64
bit in jenkins.

Bench.cpp

@@ -258,4 +258,107 @@ void benchConflictSet() {
   }
 }
 
-int main(void) { benchConflictSet(); }
+constexpr int kKeyLenForWorstCase = 50;
+
+ConflictSet worstCaseConflictSetForRadixRangeRead(int cardinality) {
+  ConflictSet cs{0};
+
+  for (int i = 0; i < kKeyLenForWorstCase; ++i) {
+    for (int j = 0; j < cardinality; ++j) {
+      auto b = std::vector<uint8_t>(i, 0);
+      b.push_back(j);
+      auto e = std::vector<uint8_t>(i, 255);
+      e.push_back(255 - j);
+      weaselab::ConflictSet::WriteRange w[] = {{
+                                                   {b.data(), int(b.size())},
+                                                   {nullptr, 0},
+                                               },
+                                               {
+                                                   {e.data(), int(e.size())},
+                                                   {nullptr, 0},
+                                               }};
+      std::sort(std::begin(w), std::end(w),
+                [](const auto &lhs, const auto &rhs) {
+                  int cl = std::min(lhs.begin.len, rhs.begin.len);
+                  if (cl > 0) {
+                    int c = memcmp(lhs.begin.p, rhs.begin.p, cl);
+                    if (c != 0) {
+                      return c < 0;
+                    }
+                  }
+                  return lhs.begin.len < rhs.begin.len;
+                });
+      cs.addWrites(w, sizeof(w) / sizeof(w[0]), 0);
+    }
+  }
+
+  // Defeat short-circuiting on the left
+  {
+    auto k = std::vector<uint8_t>(kKeyLenForWorstCase, 0);
+    weaselab::ConflictSet::WriteRange w[] = {
+        {
+            {k.data(), int(k.size())},
+            {nullptr, 0},
+        },
+    };
+    cs.addWrites(w, sizeof(w) / sizeof(w[0]), 1);
+  }
+
+  // Defeat short-circuiting on the right
+  {
+    auto k = std::vector<uint8_t>(kKeyLenForWorstCase, 255);
+    weaselab::ConflictSet::WriteRange w[] = {
+        {
+            {k.data(), int(k.size())},
+            {nullptr, 0},
+        },
+    };
+    cs.addWrites(w, sizeof(w) / sizeof(w[0]), 1);
+  }
+
+  return cs;
+}
+
+void benchWorstCaseForRadixRangeRead() {
+  ankerl::nanobench::Bench bench;
+
+  std::unique_ptr<ConflictSet> cs[256];
+  for (int i = 0; i < 256; ++i) {
+    cs[i] =
+        std::make_unique<ConflictSet>(worstCaseConflictSetForRadixRangeRead(i));
+  }
+
+  auto begin = std::vector<uint8_t>(kKeyLenForWorstCase - 1, 0);
+  begin.push_back(1);
+  auto end = std::vector<uint8_t>(kKeyLenForWorstCase - 1, 255);
+  end.push_back(254);
+
+  weaselab::ConflictSet::Result result;
+  weaselab::ConflictSet::ReadRange r{
+      {begin.data(), int(begin.size())}, {end.data(), int(end.size())}, 0};
+
+  bench.run("worst case for radix tree", [&]() {
+    for (int i = 0; i < 256; ++i) {
+      result = weaselab::ConflictSet::TooOld;
+      cs[i]->check(&r, &result, 1);
+      if (result != weaselab::ConflictSet::Commit) {
+        abort();
+      }
+    }
+  });
+
+  // for (int i = 0; i < 256; ++i) {
+  //   bench.run("worst case for radix tree, span " + std::to_string(i), [&]() {
+  //     result = weaselab::ConflictSet::TooOld;
+  //     cs[i]->check(&r, &result, 1);
+  //     if (result != weaselab::ConflictSet::Commit) {
+  //       abort();
+  //     }
+  //   });
+  // }
+}
+
+int main(void) {
+  benchConflictSet();
+  benchWorstCaseForRadixRangeRead();
+}

CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.18)
 project(
   conflict-set
-  VERSION 0.0.6
+  VERSION 0.0.7
   DESCRIPTION
     "A data structure for optimistic concurrency control on ranges of bitwise-lexicographically-ordered keys."
   HOMEPAGE_URL "https://git.weaselab.dev/weaselab/conflict-set"
@@ -59,6 +59,10 @@ cmake_pop_check_state()
 
 option(USE_SIMD_FALLBACK
        "Use fallback implementations of functions that use SIMD" OFF)
+option(
+  USE_32_BIT_VERSIONS
+  "Store 32 bit versions internally, and rely on versions never being different by more than 2e9"
+  OFF)
 
 # This is encouraged according to
 # https://valgrind.org/docs/manual/manual-core-adv.html#manual-core-adv.clientreq
@@ -103,6 +107,10 @@ if(NOT USE_SIMD_FALLBACK)
   endif()
 endif()
 
+if(USE_32_BIT_VERSIONS)
+  add_compile_definitions(INTERNAL_VERSION_32_BIT=1)
+endif()
+
 set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "")
 
 add_library(${PROJECT_NAME}-object OBJECT ConflictSet.cpp)

Internal.h

@@ -578,8 +578,8 @@ template <class ConflictSetImpl> struct TestDriver {
   explicit TestDriver(const uint8_t *data, size_t size)
       : arbitrary({data, size}) {}
 
-  int64_t writeVersion = 100;
-  int64_t oldestVersion = 0;
+  int64_t oldestVersion = arbitrary.bounded(2) ? 0 : 0xfffffff0;
+  int64_t writeVersion = oldestVersion + 100;
   ConflictSetImpl cs{oldestVersion};
   ReferenceImpl refImpl{oldestVersion};
 

Jenkinsfile

@@ -59,6 +59,17 @@ pipeline {
                 CleanBuildAndTest("-DUSE_SIMD_FALLBACK=ON")
             }
         }
+        stage('32-bit versions') {
+            agent {
+                dockerfile {
+                    args '-v /home/jenkins/ccache:/ccache'
+                    reuseNode true
+                }
+            }
+            steps {
+                CleanBuildAndTest("-DUSE_32_BIT_VERSIONS=ON")
+            }
+        }
         stage('Release [gcc]') {
             agent {
                 dockerfile {
@@ -110,7 +121,7 @@ pipeline {
                 sh '''
                 gcovr -f ConflictSet.cpp --cobertura > build/coverage.xml
                 '''
-                cobertura autoUpdateHealth: false, autoUpdateStability: false, coberturaReportFile: 'build/coverage.xml', conditionalCoverageTargets: '70, 0, 0', failUnhealthy: false, failUnstable: false, lineCoverageTargets: '80, 0, 0', maxNumberOfBuilds: 0, methodCoverageTargets: '80, 0, 0', onlyStable: false, sourceEncoding: 'ASCII', zoomCoverageChart: false
+                recordCoverage qualityGates: [[criticality: 'NOTE', metric: 'MODULE']], tools: [[parser: 'COBERTURA', pattern: 'build/coverage.xml']]
                 sh '''
                 gcovr -f ConflictSet.cpp --fail-under-line 100 > /dev/null
                 '''

README.md

@@ -58,27 +58,29 @@ Performance counters:
 
 ## Skip list
 
-|  ns/op |         op/s | err% | total | benchmark                           |
-| -----: | -----------: | ---: | ----: | :---------------------------------- |
-| 246.99 | 4,048,700.59 | 0.2% |  0.01 | `point reads`                       |
-| 260.16 | 3,843,784.65 | 0.1% |  0.01 | `prefix reads`                      |
-| 493.35 | 2,026,953.19 | 0.1% |  0.01 | `range reads`                       |
-| 462.05 | 2,164,289.23 | 0.6% |  0.01 | `point writes`                      |
-| 448.19 | 2,231,205.25 | 0.9% |  0.01 | `prefix writes`                     |
-| 255.83 | 3,908,845.72 | 1.5% |  0.02 | `range writes`                      |
-| 582.63 | 1,716,349.02 | 1.3% |  0.01 | `monotonic increasing point writes` |
+|               ns/op |                op/s |    err% |     total | benchmark
+|--------------------:|--------------------:|--------:|----------:|:----------
+|              256.89 |        3,892,784.92 |    0.3% |      0.01 | `point reads`
+|              272.90 |        3,664,395.04 |    0.2% |      0.01 | `prefix reads`
+|              507.22 |        1,971,549.50 |    0.7% |      0.01 | `range reads`
+|              452.66 |        2,209,181.91 |    0.5% |      0.01 | `point writes`
+|              438.09 |        2,282,619.96 |    0.4% |      0.01 | `prefix writes`
+|              253.33 |        3,947,420.36 |    2.5% |      0.02 | `range writes`
+|              574.07 |        1,741,936.71 |    0.3% |      0.01 | `monotonic increasing point writes`
+|          151,562.50 |            6,597.94 |    1.5% |      0.01 | `worst case for radix tree`
 
 ## Radix tree (this implementation)
 
-|  ns/op |          op/s | err% | total | benchmark                           |
-| -----: | ------------: | ---: | ----: | :---------------------------------- |
-|  19.42 | 51,483,206.67 | 0.3% |  0.01 | `point reads`                       |
-|  58.43 | 17,115,612.57 | 0.1% |  0.01 | `prefix reads`                      |
-| 216.09 |  4,627,766.60 | 0.2% |  0.01 | `range reads`                       |
-|  28.35 | 35,267,567.72 | 0.2% |  0.01 | `point writes`                      |
-|  43.43 | 23,026,226.17 | 0.2% |  0.01 | `prefix writes`                     |
-|  50.00 | 20,000,000.00 | 0.0% |  0.01 | `range writes`                      |
-|  92.38 | 10,824,863.69 | 4.1% |  0.01 | `monotonic increasing point writes` |
+|               ns/op |                op/s |    err% |     total | benchmark
+|--------------------:|--------------------:|--------:|----------:|:----------
+|               19.83 |       50,420,955.28 |    0.1% |      0.01 | `point reads`
+|               55.95 |       17,872,542.40 |    0.5% |      0.01 | `prefix reads`
+|               88.28 |       11,327,709.50 |    0.4% |      0.01 | `range reads`
+|               29.15 |       34,309,531.64 |    0.5% |      0.01 | `point writes`
+|               42.36 |       23,607,424.27 |    1.1% |      0.01 | `prefix writes`
+|               50.00 |       20,000,000.00 |    0.0% |      0.01 | `range writes`
+|               93.52 |       10,692,413.79 |    3.3% |      0.01 | `monotonic increasing point writes`
+|        2,388,417.00 |              418.69 |    0.4% |      0.03 | `worst case for radix tree`
 
 # "Real data" test
 

SkipList.cpp

@@ -577,7 +577,8 @@ struct SkipListConflictSet {};
 
 struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   Impl(int64_t oldestVersion)
-      : oldestVersion(oldestVersion), skipList(oldestVersion) {}
+      : oldestVersion(oldestVersion), newestVersion(oldestVersion),
+        skipList(oldestVersion) {}
   void check(const ConflictSet::ReadRange *reads, ConflictSet::Result *results,
              int count) const {
     Arena arena;
@@ -592,7 +593,8 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     }
     skipList.detectConflicts(ranges, count, results);
     for (int i = 0; i < count; ++i) {
-      if (reads[i].readVersion < oldestVersion) {
+      if (reads[i].readVersion < oldestVersion ||
+          reads[i].readVersion < newestVersion - 2e9) {
         results[i] = TooOld;
       }
     }
@@ -600,6 +602,8 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 
   void addWrites(const ConflictSet::WriteRange *writes, int count,
                  int64_t writeVersion) {
+    assert(writeVersion >= newestVersion);
+    newestVersion = writeVersion;
     Arena arena;
     const int stringCount = count * 2;
 
@@ -630,6 +634,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   }
 
   void setOldestVersion(int64_t oldestVersion) {
+    assert(oldestVersion >= this->oldestVersion);
     this->oldestVersion = oldestVersion;
     SkipList::Finger finger;
     int temp;
@@ -648,6 +653,7 @@ private:
   Arena removalArena;
   std::span<const uint8_t> removalKey;
   int64_t oldestVersion;
+  int64_t newestVersion;
   SkipList skipList;
 };
 

apple-symbol-imports.txt

@@ -1,3 +1,5 @@
+___stack_chk_fail
+___stack_chk_guard
 __tlv_bootstrap
 _abort
 _bzero

conflict_set.py

@@ -115,7 +115,9 @@ class ConflictSet:
 
     def check(self, *reads: ReadRange) -> list[Result]:
         r = (ctypes.c_int * len(reads))()
-        self._lib.ConflictSet_check(self.p, *reads, r, 1)
+        self._lib.ConflictSet_check(
+            self.p, (ReadRange * len(reads))(*reads), r, len(reads)
+        )
         return [Result(x) for x in r]
 
     def setOldestVersion(self, version: int) -> None: