Check more preconditions in Debug mode

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.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
-          compare16_32bit_avx512(self->childMaxVersion, // GCOVR_EXCL_LINE
-                                 readVersion);          // GCOVR_EXCL_LINE
+      compared = compare16_avx512(self->childMaxVersion, readVersion);
     } else {
-      compared = compare16_32bit(self->childMaxVersion,
-                                 readVersion); // GCOVR_EXCL_LINE
+      compared = compare16(self->childMaxVersion, readVersion);
     }
     return !(compared & mask) && firstRangeOk;
 
@@ -2864,9 +2912,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]]
-Node **insert(Node **self, std::span<const uint8_t> key,
-              InternalVersionT writeVersion, WriteContext *tls) {
+[[nodiscard]] Node **insert(Node **self, std::span<const uint8_t> key,
+                            InternalVersionT writeVersion, WriteContext *tls) {
 
   for (; key.size() != 0; ++tls->accum.insert_iterations) {
     self = &getOrCreateChild(*self, key, writeVersion, tls);
@@ -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);
-      setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
+      if (newestVersionFullPrecision - kNominalVersionWindow >
+          oldestVersionFullPrecision) {
+        setOldestVersion(newestVersionFullPrecision - kNominalVersionWindow);
+      }
       while (oldestExtantVersion <
              newestVersionFullPrecision - kMaxCorrectVersionWindow) {
         gcScanStep(1000);
@@ -3163,7 +3217,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) {
@@ -3248,14 +3305,22 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     return fuel;
   }
 
-  void setOldestVersion(int64_t o) {
-    if (o <= oldestVersionFullPrecision) {
+  void setOldestVersion(int64_t newOldestVersion) {
+    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};
-    this->oldestVersionFullPrecision = o;
+    InternalVersionT oldestVersion{newOldestVersion};
+    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();
   }
@@ -3704,13 +3772,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 +3819,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 +3865,7 @@ void checkVersionsGeqOldestExtant(Node *n,
     abort();
   }
 }
+#endif
 
 [[maybe_unused]] InternalVersionT
 checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,

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 {

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,20 +200,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;
-
-  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;
-  }
+  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
@@ -426,18 +415,23 @@ public:
   }
   void swap(SkipList &other) { std::swap(header, other.header); }
 
-  void addConflictRanges(const Finger *fingers, int rangeCount,
-                         Version version) {
+  // 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;

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
-     * latest call to `addWrites` minus two billion will result in `TooOld` */
+    /** `readVersion` older than the oldestVersion or the version of the
+     * latest call to `addWrites` minus two billion will result in `TooOld`.
+     * Must be <= the version of the latest call to `addWrites` */
     int64_t readVersion;
   };
 
@@ -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
-   * greater than or equal to all previous oldest versions. */
+  /** Reads where readVersion < `oldestVersion` will result in `TooOld`. Must be
+   * greater than or equal to all previous oldest versions. Must be <= the
+   * version of the latest call to `addWrites` */
   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
-   * latest call to `addWrites` minus two billion will result in `TooOld` */
+  /** `readVersion` older than the oldestVersion or the version of the
+   * latest call to `addWrites` minus two billion will result in `TooOld`.
+   * Must be <= the version of the latest call to `addWrites` */
   int64_t readVersion;
 } ConflictSet_ReadRange;
 
@@ -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
- * `Conflict` (or `TooOld`, eventually).  `writeVersion` must be greater than or
- * equal to all previous write versions.  */
+/** Reads intersecting writes where readVersion < `writeVersion` will result
+ * in `Conflict` (or `TooOld`, eventually).  `writeVersion` must be greater
+ * than or equal to all previous write versions. Call `addWrites` with `count`
+ * zero to only advance the version. */
 void ConflictSet_addWrites(ConflictSet *cs,
                            const ConflictSet_WriteRange *writes, int count,
                            int64_t writeVersion);
 
-/** Reads where readVersion < oldestVersion will result in `TooOld`. Must be
- * greater than or equal to all previous oldest versions. */
+/** Reads where readVersion < `oldestVersion` will result in `TooOld`. Must be
+ * greater than or equal to all previous oldest versions. Must be <= the
+ * version of the latest call to `addWrites` */
 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{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.}
+    \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 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])))