Update stale comments

low bits of lcg have low periods

CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.18)
 project(
   conflict-set
-  VERSION 0.0.12
+  VERSION 0.0.14
   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

@@ -17,9 +17,9 @@ limitations under the License.
 #include "ConflictSet.h"
 #include "Internal.h"
 #include "LongestCommonPrefix.h"
+#include "Metrics.h"
 
 #include <algorithm>
-#include <atomic>
 #include <bit>
 #include <cassert>
 #include <cstddef>
@@ -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;
@@ -553,39 +578,6 @@ static_assert(kBytesPerKey - sizeof(Node0) >= kMinNodeSurplus);
 
 constexpr int64_t kFreeListMaxMemory = 1 << 20;
 
-struct Metric {
-  Metric *prev;
-  const char *name;
-  const char *help;
-  ConflictSet::MetricsV1::Type type;
-  std::atomic<int64_t> value;
-
-protected:
-  Metric(ConflictSet::Impl *impl, const char *name, const char *help,
-         ConflictSet::MetricsV1::Type type);
-};
-
-struct Gauge : private Metric {
-  Gauge(ConflictSet::Impl *impl, const char *name, const char *help)
-      : Metric(impl, name, help, ConflictSet::MetricsV1::Gauge) {}
-
-  void set(int64_t value) {
-    this->value.store(value, std::memory_order_relaxed);
-  }
-};
-
-struct Counter : private Metric {
-  Counter(ConflictSet::Impl *impl, const char *name, const char *help)
-      : Metric(impl, name, help, ConflictSet::MetricsV1::Counter) {}
-  // Expensive. Accumulate locally and then call add instead of repeatedly
-  // calling add.
-  void add(int64_t value) {
-    assert(value >= 0);
-    static_assert(std::atomic<int64_t>::is_always_lock_free);
-    this->value.fetch_add(value, std::memory_order_relaxed);
-  }
-};
-
 template <class T> struct BoundedFreeListAllocator {
 
   static_assert(sizeof(T) >= sizeof(void *));
@@ -724,9 +716,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 +1559,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 +1604,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 +1987,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 +2008,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 +2084,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 +2145,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 +2282,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;
 
@@ -2495,38 +2510,19 @@ downLeftSpine:
 namespace {
 // Return true if the max version among all keys that start with key[:prefixLen]
 // that are >= key is <= readVersion
-struct CheckRangeLeftSide {
+bool checkRangeLeftSide(Node *n, std::span<const uint8_t> key, int prefixLen,
-  CheckRangeLeftSide(Node *n, std::span<const uint8_t> key, int prefixLen,
+                        InternalVersionT readVersion, ReadContext *tls) {
-                     InternalVersionT readVersion, ReadContext *tls)
+  auto remaining = key;
-      : n(n), remaining(key), prefixLen(prefixLen), readVersion(readVersion),
-        impl(tls->impl), tls(tls) {
-#if DEBUG_VERBOSE && !defined(NDEBUG)
-    fprintf(stderr, "Check range left side from %s for keys starting with %s\n",
-            printable(key).c_str(),
-            printable(key.subspan(0, prefixLen)).c_str());
-#endif
-  }
-
-  Node *n;
-  std::span<const uint8_t> remaining;
-  int prefixLen;
-  InternalVersionT readVersion;
-  ConflictSet::Impl *impl;
-  ReadContext *tls;
   int searchPathLen = 0;
-  bool ok;
+  for (;; ++tls->range_read_iterations_accum) {
-
-  bool step() {
     if (remaining.size() == 0) {
       assert(searchPathLen >= prefixLen);
-      ok = maxVersion(n) <= readVersion;
+      return maxVersion(n) <= readVersion;
-      return true;
     }
 
     if (searchPathLen >= prefixLen) {
       if (!checkMaxBetweenExclusive(n, remaining[0], 256, readVersion, tls)) {
-        ok = false;
+        return false;
-        return true;
       }
     }
 
@@ -2536,18 +2532,16 @@ struct CheckRangeLeftSide {
       if (c != nullptr) {
         if (searchPathLen < prefixLen) {
           n = c;
-          return downLeftSpine();
+          goto downLeftSpine;
         }
         n = c;
-        ok = maxVersion(n) <= readVersion;
+        return maxVersion(n) <= readVersion;
-        return true;
       } else {
         n = nextSibling(n);
         if (n == nullptr) {
-          ok = true;
           return true;
         }
-        return downLeftSpine();
+        goto downLeftSpine;
       }
     }
 
@@ -2563,21 +2557,18 @@ struct CheckRangeLeftSide {
         auto c = n->partialKey()[i] <=> remaining[i];
         if (c > 0) {
           if (searchPathLen < prefixLen) {
-            return downLeftSpine();
+            goto downLeftSpine;
           }
           if (n->entryPresent && n->entry.rangeVersion > readVersion) {
-            ok = false;
+            return false;
-            return true;
           }
-          ok = maxVersion(n) <= readVersion;
+          return maxVersion(n) <= readVersion;
-          return true;
         } else {
           n = nextSibling(n);
           if (n == nullptr) {
-            ok = true;
             return true;
           }
-          return downLeftSpine();
+          goto downLeftSpine;
         }
       }
       if (commonLen == n->partialKeyLen) {
@@ -2586,83 +2577,47 @@ struct CheckRangeLeftSide {
       } else if (n->partialKeyLen > int(remaining.size())) {
         assert(searchPathLen >= prefixLen);
         if (n->entryPresent && n->entry.rangeVersion > readVersion) {
-          ok = false;
+          return false;
-          return true;
         }
-        ok = maxVersion(n) <= readVersion;
+        return maxVersion(n) <= readVersion;
-        return true;
       }
     }
     if (maxV <= readVersion) {
-      ok = true;
       return true;
     }
-    return false;
   }
-
+downLeftSpine:
-  bool downLeftSpine() {
+  for (; !n->entryPresent; n = getFirstChildExists(n)) {
-    for (; !n->entryPresent; n = getFirstChildExists(n)) {
-    }
-    ok = n->entry.rangeVersion <= readVersion;
-    return true;
   }
-};
+  return n->entry.rangeVersion <= readVersion;
+}
 
 // Return true if the max version among all keys that start with key[:prefixLen]
 // that are < key is <= readVersion
-struct CheckRangeRightSide {
+bool checkRangeRightSide(Node *n, std::span<const uint8_t> key, int prefixLen,
-  CheckRangeRightSide(Node *n, std::span<const uint8_t> key, int prefixLen,
+                         InternalVersionT readVersion, ReadContext *tls) {
-                      InternalVersionT readVersion, ReadContext *tls)
+  auto remaining = key;
-      : n(n), key(key), remaining(key), prefixLen(prefixLen),
-        readVersion(readVersion), impl(tls->impl), tls(tls) {
-#if DEBUG_VERBOSE && !defined(NDEBUG)
-    fprintf(stderr, "Check range right side to %s for keys starting with %s\n",
-            printable(key).c_str(),
-            printable(key.subspan(0, prefixLen)).c_str());
-#endif
-  }
-
-  Node *n;
-  std::span<const uint8_t> key;
-  std::span<const uint8_t> remaining;
-  int prefixLen;
-  InternalVersionT readVersion;
-  ConflictSet::Impl *impl;
-  ReadContext *tls;
   int searchPathLen = 0;
-  bool ok;
-
-  bool step() {
-#if DEBUG_VERBOSE && !defined(NDEBUG)
-    fprintf(stderr,
-            "Search path: %s, searchPathLen: %d, prefixLen: %d, remaining: "
-            "%s\n",
-            getSearchPathPrintable(n).c_str(), searchPathLen, prefixLen,
-            printable(remaining).c_str());
-#endif
 
+  for (;; ++tls->range_read_iterations_accum) {
     assert(searchPathLen <= int(key.size()));
-
     if (remaining.size() == 0) {
-      return downLeftSpine();
+      goto downLeftSpine;
     }
 
     if (searchPathLen >= prefixLen) {
       if (n->entryPresent && n->entry.pointVersion > readVersion) {
-        ok = false;
+        return false;
-        return true;
       }
 
       if (!checkMaxBetweenExclusive(n, -1, remaining[0], readVersion, tls)) {
-        ok = false;
+        return false;
-        return true;
       }
     }
 
     if (searchPathLen > prefixLen && n->entryPresent &&
         n->entry.rangeVersion > readVersion) {
-      ok = false;
+      return false;
-      return true;
     }
 
     auto *child = getChild(n, remaining[0]);
@@ -2670,9 +2625,9 @@ struct CheckRangeRightSide {
       auto c = getChildGeq(n, remaining[0]);
       if (c != nullptr) {
         n = c;
-        return downLeftSpine();
+        goto downLeftSpine;
       } else {
-        return backtrack();
+        goto backtrack;
       }
     }
 
@@ -2688,57 +2643,48 @@ struct CheckRangeRightSide {
         ++searchPathLen;
         auto c = n->partialKey()[i] <=> remaining[i];
         if (c > 0) {
-          return downLeftSpine();
+          goto downLeftSpine;
         } else {
           if (searchPathLen > prefixLen && n->entryPresent &&
               n->entry.rangeVersion > readVersion) {
-            ok = false;
+            return false;
-            return true;
           }
-          return backtrack();
+          goto backtrack;
         }
       }
       if (commonLen == n->partialKeyLen) {
         // partial key matches
         remaining = remaining.subspan(commonLen, remaining.size() - commonLen);
       } else if (n->partialKeyLen > int(remaining.size())) {
-        return downLeftSpine();
+        goto downLeftSpine;
-      }
-    }
-    return false;
-  }
-
-  bool backtrack() {
-    for (;;) {
-      // searchPathLen > prefixLen implies n is not the root
-      if (searchPathLen > prefixLen && maxVersion(n) > readVersion) {
-        ok = false;
-        return true;
-      }
-      if (n->parent == nullptr) {
-        ok = true;
-        return true;
-      }
-      auto next = getChildGeq(n->parent, n->parentsIndex + 1);
-      if (next == nullptr) {
-        searchPathLen -= 1 + n->partialKeyLen;
-        n = n->parent;
-      } else {
-        searchPathLen -= n->partialKeyLen;
-        n = next;
-        searchPathLen += n->partialKeyLen;
-        return downLeftSpine();
       }
     }
   }
-
+backtrack:
-  bool downLeftSpine() {
+  for (;;) {
-    for (; !n->entryPresent; n = getFirstChildExists(n)) {
+    // searchPathLen > prefixLen implies n is not the root
+    if (searchPathLen > prefixLen && maxVersion(n) > readVersion) {
+      return false;
+    }
+    if (n->parent == nullptr) {
+      return true;
+    }
+    auto next = getChildGeq(n->parent, n->parentsIndex + 1);
+    if (next == nullptr) {
+      searchPathLen -= 1 + n->partialKeyLen;
+      n = n->parent;
+    } else {
+      searchPathLen -= n->partialKeyLen;
+      n = next;
+      searchPathLen += n->partialKeyLen;
+      goto downLeftSpine;
     }
-    ok = n->entry.rangeVersion <= readVersion;
-    return true;
   }
-};
+downLeftSpine:
+  for (; !n->entryPresent; n = getFirstChildExists(n)) {
+  }
+  return n->entry.rangeVersion <= readVersion;
+}
 } // namespace
 
 bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
@@ -2759,8 +2705,8 @@ bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
 
   auto remaining = begin.subspan(0, lcp);
   Arena arena;
-  // If the common prefix isn't a prefix of any physical entry in the tree, we
+
-  // can go to "downLeftSpine"
+  // Advance down common prefix, but stay on a physical path in the tree
   for (;; ++tls->range_read_iterations_accum) {
     assert(getSearchPath(arena, n) <=>
                begin.subspan(0, lcp - remaining.size()) ==
@@ -2801,47 +2747,17 @@ bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
   lcp -= consumed;
 
   if (lcp == int(begin.size())) {
-    CheckRangeRightSide checkRangeRightSide{n, end, lcp, readVersion, tls};
+    return checkRangeRightSide(n, end, lcp, readVersion, tls);
-    while (!checkRangeRightSide.step())
-      ;
-    return checkRangeRightSide.ok;
   }
 
-  if (!checkRangeStartsWith(n, begin.subspan(0, lcp), begin[lcp], end[lcp],
+  // This makes it safe to check maxVersion within checkRangeLeftSide. If this
-                            readVersion, tls)) {
-    return false;
-  }
-
-  // This makes it safe to check maxVersion within CheckRangeLeftSide. If this
   // were false, then we would have returned above since lcp == begin.size().
   assert(!(n->parent == nullptr && begin.size() == 0));
-  CheckRangeLeftSide checkRangeLeftSide{n, begin, lcp + 1, readVersion, tls};
-  CheckRangeRightSide checkRangeRightSide{n, end, lcp + 1, readVersion, tls};
 
-  for (;;) {
+  return checkRangeStartsWith(n, begin.subspan(0, lcp), begin[lcp], end[lcp],
-    bool leftDone = checkRangeLeftSide.step();
+                              readVersion, tls) &&
-    bool rightDone = checkRangeRightSide.step();
+         checkRangeLeftSide(n, begin, lcp + 1, readVersion, tls) &&
-    if (!leftDone && !rightDone) {
+         checkRangeRightSide(n, end, lcp + 1, readVersion, tls);
-      tls->range_read_iterations_accum += 2;
-      continue;
-    }
-    if (leftDone && rightDone) {
-      break;
-    } else if (leftDone) {
-      while (!checkRangeRightSide.step()) {
-        ++tls->range_read_iterations_accum;
-      }
-      break;
-    } else {
-      assert(rightDone);
-      while (!checkRangeLeftSide.step()) {
-        ++tls->range_read_iterations_accum;
-      }
-    }
-    break;
-  }
-
-  return checkRangeLeftSide.ok && checkRangeRightSide.ok;
 }
 
 #ifdef __x86_64__
@@ -2864,9 +2780,8 @@ 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) {
-              InternalVersionT writeVersion, WriteContext *tls) {
 
   for (; key.size() != 0; ++tls->accum.insert_iterations) {
     self = &getOrCreateChild(*self, key, writeVersion, tls);
@@ -3101,6 +3016,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 +3054,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 +3074,10 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 
       newestVersionFullPrecision = writeVersion;
       newest_version.set(newestVersionFullPrecision);
-      setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
+      if (newestVersionFullPrecision - kNominalVersionWindow >
+          oldestVersionFullPrecision) {
+        setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
+      }
       while (oldestExtantVersion <
              newestVersionFullPrecision - kMaxCorrectVersionWindow) {
         gcScanStep(1000);
@@ -3163,7 +3085,10 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     } else {
       newestVersionFullPrecision = writeVersion;
       newest_version.set(newestVersionFullPrecision);
-      setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
+      if (newestVersionFullPrecision - kNominalVersionWindow >
+          oldestVersionFullPrecision) {
+        setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
+      }
     }
 
     for (int i = 0; i < count; ++i) {
@@ -3185,7 +3110,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
                           0) *
         2;
 
-    memory_bytes.set(totalBytes);
     point_writes_total.add(tls.accum.point_writes);
     range_writes_total.add(tls.accum.range_writes);
     nodes_allocated_total.add(tls.accum.nodes_allocated);
@@ -3248,14 +3172,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
@@ -3266,7 +3198,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 #endif
     keyUpdates = gcScanStep(keyUpdates);
 
-    memory_bytes.set(totalBytes);
     nodes_allocated_total.add(std::exchange(tls.accum.nodes_allocated, 0));
     nodes_released_total.add(std::exchange(tls.accum.nodes_released, 0));
     entries_inserted_total.add(std::exchange(tls.accum.entries_inserted, 0));
@@ -3304,14 +3235,17 @@ 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();
+    metrics = initMetrics(metricsList, metricsCount);
   }
   ~Impl() {
     eraseTree(root, &tls);
@@ -3334,23 +3268,12 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 
   MetricsV1 *metrics;
   int metricsCount = 0;
-  void initMetrics() {
+  Metric *metricsList = nullptr;
-    metrics = (MetricsV1 *)safe_malloc(metricsCount * sizeof(metrics[0]));
-    for (auto [i, m] = std::make_tuple(metricsCount - 1, metricList); i >= 0;
-         --i, m = m->prev) {
-      metrics[i].name = m->name;
-      metrics[i].help = m->help;
-      metrics[i].p = m;
-      metrics[i].type = m->type;
-    }
-  }
-
-  Metric *metricList = nullptr;
 
 #define GAUGE(name, help)                                                      \
-  Gauge name { this, #name, help }
+  Gauge name { metricsList, metricsCount, #name, help }
 #define COUNTER(name, help)                                                    \
-  Counter name { this, #name, help }
+  Counter name { metricsList, metricsCount, #name, help }
   // ==================== METRICS DEFINITIONS ====================
   COUNTER(point_read_total, "Total number of point reads checked");
   COUNTER(point_read_short_circuit_total,
@@ -3416,13 +3339,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   }
 };
 
-Metric::Metric(ConflictSet::Impl *impl, const char *name, const char *help,
-               ConflictSet::MetricsV1::Type type)
-    : prev(std::exchange(impl->metricList, this)), name(name), help(help),
-      type(type), value(0) {
-  ++impl->metricsCount;
-}
-
 Node *&getInTree(Node *n, ConflictSet::Impl *impl) {
   return n->parent == nullptr ? impl->root
                               : getChildExists(n->parent, n->parentsIndex);
@@ -3441,6 +3357,7 @@ void internal_addWrites(ConflictSet::Impl *impl,
   mallocBytesDelta = 0;
   impl->addWrites(writes, count, writeVersion);
   impl->totalBytes += mallocBytesDelta;
+  impl->memory_bytes.set(impl->totalBytes);
 #if SHOW_MEMORY
   if (impl->totalBytes != mallocBytes) {
     abort();
@@ -3452,6 +3369,7 @@ void internal_setOldestVersion(ConflictSet::Impl *impl, int64_t oldestVersion) {
   mallocBytesDelta = 0;
   impl->setOldestVersion(oldestVersion);
   impl->totalBytes += mallocBytesDelta;
+  impl->memory_bytes.set(impl->totalBytes);
 #if SHOW_MEMORY
   if (impl->totalBytes != mallocBytes) {
     abort();
@@ -3704,13 +3622,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 +3669,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 +3715,7 @@ void checkVersionsGeqOldestExtant(Node *n,
     abort();
   }
 }
+#endif
 
 [[maybe_unused]] InternalVersionT
 checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
@@ -4001,6 +3923,62 @@ struct __attribute__((visibility("default"))) PeakPrinter {
 
 #ifdef ENABLE_MAIN
 
+#define ANKERL_NANOBENCH_IMPLEMENT
+#include "third_party/nanobench.h"
+
+template <int kN> void benchRezero() {
+  static_assert(kN % 16 == 0);
+  ankerl::nanobench::Bench bench;
+  InternalVersionT vs[kN];
+  InternalVersionT zero;
+  bench.run("rezero" + std::to_string(kN), [&]() {
+    bench.doNotOptimizeAway(vs);
+    bench.doNotOptimizeAway(zero);
+    for (int i = 0; i < kN; i += 16) {
+      rezero16(vs + i, zero);
+    }
+  });
+}
+
+template <int kN> void benchScan1() {
+  static_assert(kN % 16 == 0);
+  ankerl::nanobench::Bench bench;
+  InternalVersionT vs[kN];
+  uint8_t is[kN];
+  uint8_t begin;
+  uint8_t end;
+  InternalVersionT v;
+  bench.run("scan" + std::to_string(kN), [&]() {
+    bench.doNotOptimizeAway(vs);
+    bench.doNotOptimizeAway(is);
+    bench.doNotOptimizeAway(begin);
+    bench.doNotOptimizeAway(end);
+    bench.doNotOptimizeAway(v);
+    for (int i = 0; i < kN; i += 16) {
+      scan16</*kAVX512=*/true>(vs + i, is + i, begin, end, v);
+    }
+  });
+}
+
+template <int kN> void benchScan2() {
+  static_assert(kN % 16 == 0);
+  ankerl::nanobench::Bench bench;
+  InternalVersionT vs[kN];
+  uint8_t is[kN];
+  uint8_t begin;
+  uint8_t end;
+  InternalVersionT v;
+  bench.run("scan" + std::to_string(kN), [&]() {
+    bench.doNotOptimizeAway(vs);
+    bench.doNotOptimizeAway(begin);
+    bench.doNotOptimizeAway(end);
+    bench.doNotOptimizeAway(v);
+    for (int i = 0; i < kN; i += 16) {
+      scan16</*kAVX512=*/true>(vs + i, begin, end, v);
+    }
+  });
+}
+
 void printTree() {
   int64_t writeVersion = 0;
   ConflictSet::Impl cs{writeVersion};
@@ -4022,7 +4000,7 @@ void printTree() {
   debugPrintDot(stdout, cs.root, &cs);
 }
 
-int main(void) { printTree(); }
+int main(void) { benchScan1<16>(); }
 #endif
 
 #ifdef ENABLE_FUZZ

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 {
@@ -118,7 +129,7 @@ pipeline {
             }
             steps {
                 script {
-                    filter_args = "-f ConflictSet.cpp -f LongestCommonPrefix.h"
+                    filter_args = "-f ConflictSet.cpp -f LongestCommonPrefix.h -f Metrics.h"
                 }
                 CleanBuildAndTest("-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_C_FLAGS=--coverage -DCMAKE_CXX_FLAGS=--coverage -DCMAKE_BUILD_TYPE=Debug -DDISABLE_TSAN=ON")
                 sh """

Metrics.h

@@ -0,0 +1,64 @@
+#pragma once
+
+#include "ConflictSet.h"
+#include "Internal.h"
+
+#include <assert.h>
+#include <atomic>
+#include <tuple>
+
+struct Metric {
+  Metric *prev;
+  const char *name;
+  const char *help;
+  weaselab::ConflictSet::MetricsV1::Type type;
+  std::atomic<int64_t> value;
+
+protected:
+  Metric(Metric *&metricList, int &metricsCount, const char *name,
+         const char *help, weaselab::ConflictSet::MetricsV1::Type type)
+      : prev(std::exchange(metricList, this)), name(name), help(help),
+        type(type), value(0) {
+    ++metricsCount;
+  }
+};
+
+struct Gauge : private Metric {
+  Gauge(Metric *&metricList, int &metricsCount, const char *name,
+        const char *help)
+      : Metric(metricList, metricsCount, name, help,
+               weaselab::ConflictSet::MetricsV1::Gauge) {}
+
+  void set(int64_t value) {
+    this->value.store(value, std::memory_order_relaxed);
+  }
+};
+
+struct Counter : private Metric {
+  Counter(Metric *&metricList, int &metricsCount, const char *name,
+          const char *help)
+      : Metric(metricList, metricsCount, name, help,
+               weaselab::ConflictSet::MetricsV1::Counter) {}
+  // Expensive. Accumulate locally and then call add instead of repeatedly
+  // calling add.
+  void add(int64_t value) {
+    assert(value >= 0);
+    static_assert(std::atomic<int64_t>::is_always_lock_free);
+    this->value.fetch_add(value, std::memory_order_relaxed);
+  }
+};
+
+inline weaselab::ConflictSet::MetricsV1 *initMetrics(Metric *metricsList,
+                                                     int metricsCount) {
+  weaselab::ConflictSet::MetricsV1 *metrics =
+      (weaselab::ConflictSet::MetricsV1 *)safe_malloc(metricsCount *
+                                                      sizeof(metrics[0]));
+  for (auto [i, m] = std::make_tuple(metricsCount - 1, metricsList); i >= 0;
+       --i, m = m->prev) {
+    metrics[i].name = m->name;
+    metrics[i].help = m->help;
+    metrics[i].p = m;
+    metrics[i].type = m->type;
+  }
+  return metrics;
+}

README.md

@@ -24,15 +24,15 @@ Hardware for all benchmarks is an AMD Ryzen 9 7900 with (2x32GB) 5600MT/s CL28-3
 
 |               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
 |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
-|               11.18 |       89,455,125.34 |    0.6% |          185.37 |           57.08 |  3.248 |          41.51 |    0.4% |      0.01 | `point reads`
+|               12.88 |       77,653,350.77 |    0.5% |          185.37 |           64.45 |  2.876 |          41.51 |    0.4% |      0.01 | `point reads`
-|               14.53 |       68,800,688.89 |    0.4% |          282.41 |           74.80 |  3.776 |          55.06 |    0.3% |      0.01 | `prefix reads`
+|               14.67 |       68,179,354.49 |    0.1% |          271.44 |           73.40 |  3.698 |          53.70 |    0.3% |      0.01 | `prefix reads`
-|               36.54 |       27,367,576.87 |    0.2% |          798.06 |          188.90 |  4.225 |         141.69 |    0.2% |      0.01 | `range reads`
+|               34.84 |       28,701,444.36 |    0.3% |          715.74 |          175.27 |  4.084 |         127.30 |    0.2% |      0.01 | `range reads`
-|               16.69 |       59,912,106.02 |    0.6% |          314.57 |           86.29 |  3.645 |          39.84 |    0.4% |      0.01 | `point writes`
+|               17.12 |       58,422,988.28 |    0.2% |          314.30 |           86.11 |  3.650 |          39.82 |    0.4% |      0.01 | `point writes`
-|               30.09 |       33,235,744.07 |    0.5% |          591.33 |          155.92 |  3.793 |          82.69 |    0.2% |      0.01 | `prefix writes`
+|               31.42 |       31,830,804.65 |    0.1% |          591.06 |          158.07 |  3.739 |          82.67 |    0.2% |      0.01 | `prefix writes`
-|               35.77 |       27,956,388.03 |    1.4% |          682.25 |          187.63 |  3.636 |          96.12 |    0.1% |      0.01 | `range writes`
+|               37.37 |       26,759,432.70 |    2.2% |          681.98 |          188.95 |  3.609 |          96.10 |    0.1% |      0.01 | `range writes`
-|               74.04 |       13,505,408.41 |    2.7% |        1,448.95 |          392.10 |  3.695 |         260.53 |    0.1% |      0.01 | `monotonic increasing point writes`
+|               76.72 |       13,035,140.63 |    2.3% |        1,421.28 |          387.17 |  3.671 |         257.76 |    0.1% |      0.01 | `monotonic increasing point writes`
-|          330,984.50 |            3,021.29 |    1.9% |    3,994,153.50 |    1,667,309.00 |  2.396 |     806,019.50 |    0.0% |      0.01 | `worst case for radix tree`
+|          297,452.00 |            3,361.89 |    0.9% |    3,508,083.00 |    1,500,834.67 |  2.337 |     727,525.33 |    0.1% |      0.01 | `worst case for radix tree`
-|               92.46 |       10,814,961.65 |    0.5% |        1,800.00 |          463.41 |  3.884 |         297.00 |    0.0% |      0.01 | `create and destroy`
+|               87.70 |       11,402,490.60 |    1.0% |        1,795.00 |          442.09 |  4.060 |         297.00 |    0.0% |      0.01 | `create and destroy`
 
 # "Real data" test
 

ServerBench.cpp

@@ -21,7 +21,7 @@
 
 std::atomic<int64_t> transactions;
 
-constexpr int kBaseSearchDepth = 32;
+constexpr int kBaseSearchDepth = 115;
 constexpr int kWindowSize = 10000000;
 
 std::string numToKey(int64_t num) {

SkipList.cpp

@@ -22,9 +22,11 @@
 
 #include "ConflictSet.h"
 #include "Internal.h"
+#include "Metrics.h"
 
 #include <algorithm>
 #include <span>
+#include <vector>
 
 std::span<const uint8_t> keyAfter(Arena &arena, std::span<const uint8_t> key) {
   auto result =
@@ -115,15 +117,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 +176,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,20 +201,24 @@ struct ReadConflictRange {
   }
 };
 
+static constexpr int MaxLevels = 26;
+
+struct RandomLevel {
+  explicit RandomLevel(uint32_t seed) : seed(seed) {}
+
+  int next() {
+    int result = __builtin_clz(seed | (uint32_t(-1) >> (MaxLevels - 1)));
+    seed = seed * 1664525L + 1013904223L;
+    return result;
+  }
+
+private:
+  uint32_t seed;
+};
+
 class SkipList {
 private:
-  static constexpr int MaxLevels = 26;
+  RandomLevel randomLevel{0};
-
-  int randomLevel() const {
-    uint32_t i = uint32_t(skfastrand()) >> (32 - (MaxLevels - 1));
-    int level = 0;
-    while (i & 1) {
-      i >>= 1;
-      level++;
-    }
-    assert(level < MaxLevels);
-    return level;
-  }
 
   // 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
@@ -426,27 +416,33 @@ 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
-                         Version version) {
+  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,
+  // Return number of iterations of main loop
-                       ConflictSet::Result *transactionConflictStatus) const {
+  int detectConflicts(ReadConflictRange *ranges, int count,
+                      ConflictSet::Result *transactionConflictStatus) const {
     const int M = 16;
     int nextJob[M];
     CheckMax inProgress[M];
     if (!count)
-      return;
+      return 0;
 
     int started = std::min(M, count);
     for (int i = 0; i < started; i++) {
@@ -457,8 +453,9 @@ public:
 
     int prevJob = started - 1;
     int job = 0;
+    int iters = 0;
     // vtune: 340 parts
-    while (true) {
+    for (;; ++iters) {
       if (inProgress[job].advance()) {
         if (started == count) {
           if (prevJob == job)
@@ -474,6 +471,7 @@ public:
       prevJob = job;
       job = nextJob[job];
     }
+    return iters;
   }
 
   void find(const StringRef *values, Finger *results, int *temp, int count) {
@@ -567,9 +565,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 +577,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,17 +706,27 @@ private:
   };
 };
 
-struct SkipListConflictSet {};
+struct ReadContext {
+  int64_t commits_accum = 0;
+  int64_t conflicts_accum = 0;
+  int64_t too_olds_accum = 0;
+  int64_t check_bytes_accum = 0;
+};
 
 struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   Impl(int64_t oldestVersion)
       : oldestVersion(oldestVersion), newestVersion(oldestVersion),
-        skipList(oldestVersion) {}
+        skipList(oldestVersion) {
+    metrics = initMetrics(metricsList, metricsCount);
+  }
+  ~Impl() { safe_free(metrics, metricsCount * sizeof(metrics[0])); }
   void check(const ConflictSet::ReadRange *reads, ConflictSet::Result *results,
-             int count) const {
+             int count) {
+    ReadContext tls;
     Arena arena;
     auto *ranges = new (arena) ReadConflictRange[count];
     for (int i = 0; i < count; ++i) {
+      tls.check_bytes_accum += reads[i].begin.len + reads[i].end.len;
       ranges[i].begin = {reads[i].begin.p, size_t(reads[i].begin.len)};
       ranges[i].end = reads[i].end.len > 0
                           ? StringRef{reads[i].end.p, size_t(reads[i].end.len)}
@@ -722,13 +734,22 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
       ranges[i].version = reads[i].readVersion;
       results[i] = ConflictSet::Commit;
     }
-    skipList.detectConflicts(ranges, count, results);
+    int iters = skipList.detectConflicts(ranges, count, results);
     for (int i = 0; i < count; ++i) {
       if (reads[i].readVersion < oldestVersion ||
           reads[i].readVersion < newestVersion - 2e9) {
         results[i] = TooOld;
       }
+      tls.commits_accum += results[i] == Commit;
+      tls.conflicts_accum += results[i] == Conflict;
+      tls.too_olds_accum += results[i] == TooOld;
     }
+    range_read_iterations_total.add(iters);
+    range_read_total.add(count);
+    commits_total.add(tls.commits_accum);
+    conflicts_total.add(tls.conflicts_accum);
+    too_olds_total.add(tls.too_olds_accum);
+    check_bytes_total.add(tls.check_bytes_accum);
   }
 
   void addWrites(const ConflictSet::WriteRange *writes, int count,
@@ -775,27 +796,33 @@ 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) {
+    // 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;
     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()});
@@ -803,8 +830,56 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 
   int64_t totalBytes = 0;
 
+  MetricsV1 *metrics;
+  int metricsCount = 0;
+  Metric *metricsList = nullptr;
+
+#define GAUGE(name, help)                                                      \
+  Gauge name { metricsList, metricsCount, #name, help }
+#define COUNTER(name, help)                                                    \
+  Counter name { metricsList, metricsCount, #name, help }
+  // ==================== METRICS DEFINITIONS ====================
+  COUNTER(range_read_total, "Total number of range reads checked");
+  COUNTER(range_read_iterations_total,
+          "Total number of iterations of the main loops for range read checks");
+  COUNTER(commits_total,
+          "Total number of checks where the result is \"commit\"");
+  COUNTER(conflicts_total,
+          "Total number of checks where the result is \"conflict\"");
+  COUNTER(too_olds_total,
+          "Total number of checks where the result is \"too old\"");
+  COUNTER(check_bytes_total, "Total number of key bytes checked");
+  GAUGE(memory_bytes, "Total number of bytes in use");
+  COUNTER(nodes_allocated_total,
+          "The total number of physical tree nodes allocated");
+  COUNTER(nodes_released_total,
+          "The total number of physical tree nodes released");
+  COUNTER(insert_iterations_total,
+          "The total number of iterations of the main loop for insertion. "
+          "Includes searches where the entry already existed, and so insertion "
+          "did not take place");
+  COUNTER(entries_inserted_total,
+          "The total number of entries inserted in the tree");
+  COUNTER(entries_erased_total,
+          "The total number of entries erased from the tree");
+  COUNTER(
+      gc_iterations_total,
+      "The total number of iterations of the main loop for garbage collection");
+  COUNTER(write_bytes_total, "Total number of key bytes in calls to addWrites");
+  GAUGE(oldest_version,
+        "The lowest version that doesn't result in \"TooOld\" for checks");
+  GAUGE(newest_version, "The version of the most recent call to addWrites");
+  // ==================== END METRICS DEFINITIONS ====================
+#undef GAUGE
+#undef COUNTER
+
+  void getMetricsV1(MetricsV1 **metrics, int *count) {
+    *metrics = this->metrics;
+    *count = metricsCount;
+  }
+
 private:
-  int64_t keyUpdates = 10;
+  int64_t keyUpdates = 0;
   Arena removalArena;
   std::span<const uint8_t> removalKey;
   int64_t oldestVersion;
@@ -825,6 +900,7 @@ void internal_addWrites(ConflictSet::Impl *impl,
   mallocBytesDelta = 0;
   impl->addWrites(writes, count, writeVersion);
   impl->totalBytes += mallocBytesDelta;
+  impl->memory_bytes.set(impl->totalBytes);
 #if SHOW_MEMORY
   if (impl->totalBytes != mallocBytes) {
     abort();
@@ -836,6 +912,7 @@ void internal_setOldestVersion(ConflictSet::Impl *impl, int64_t oldestVersion) {
   mallocBytesDelta = 0;
   impl->setOldestVersion(oldestVersion);
   impl->totalBytes += mallocBytesDelta;
+  impl->memory_bytes.set(impl->totalBytes);
 #if SHOW_MEMORY
   if (impl->totalBytes != mallocBytes) {
     abort();
@@ -859,12 +936,11 @@ int64_t internal_getBytes(ConflictSet::Impl *impl) { return impl->totalBytes; }
 
 void internal_getMetricsV1(ConflictSet::Impl *impl,
                            ConflictSet::MetricsV1 **metrics, int *count) {
-  *metrics = nullptr;
+  return impl->getMetricsV1(metrics, count);
-  *count = 0;
 }
 
 double internal_getMetricValue(const ConflictSet::MetricsV1 *metric) {
-  return 0;
+  return ((Metric *)metric->p)->value.load(std::memory_order_relaxed);
 }
 
 void ConflictSet::check(const ReadRange *reads, Result *results,

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])))