Add an experimental, disabled 32 bit internal version

I think it's only missing detection for full-precision versions more
than 2e9 apart

ConflictSet.cpp

@@ -72,7 +72,39 @@ constexpr void removeKey(struct Node *) {}
 
 // ==================== BEGIN IMPLEMENTATION ====================
 
-using InternalVersionT = int64_t;
+#define INTERNAL_VERSION_32_BIT 0
+
+#if INTERNAL_VERSION_32_BIT
+struct InternalVersionT {
+  constexpr InternalVersionT() = default;
+  constexpr explicit InternalVersionT(int64_t value) : value(value) {}
+  constexpr int64_t toInt64() const { return value; }
+  constexpr auto operator<=>(const InternalVersionT &rhs) const {
+    // Maintains ordering after overflow, as long as the full-precision versions
+    // are within ~2e9 of eachother.
+    return int32_t(value - rhs.value) <=> 0;
+  }
+  constexpr bool operator==(const InternalVersionT &) const = default;
+  static thread_local InternalVersionT zero;
+
+private:
+  uint32_t value;
+};
+thread_local InternalVersionT InternalVersionT::zero;
+#else
+struct InternalVersionT {
+  constexpr InternalVersionT() = default;
+  constexpr explicit InternalVersionT(int64_t value) : value(value) {}
+  constexpr int64_t toInt64() const { return value; }
+  constexpr auto operator<=>(const InternalVersionT &rhs) const = default;
+  constexpr bool operator==(const InternalVersionT &) const = default;
+  static const InternalVersionT zero;
+
+private:
+  int64_t value;
+};
+const InternalVersionT InternalVersionT::zero{0};
+#endif
 
 struct Entry {
   InternalVersionT pointVersion;
@@ -425,7 +457,9 @@ inline void Node48::copyChildrenAndKeyFrom(const Node16 &other) {
   assert(numChildren == Node16::kMaxNodes);
   memset(index, -1, sizeof(index));
   memset(children, 0, sizeof(children));
-  memset(childMaxVersion, 0, sizeof(childMaxVersion));
+  for (auto &v : childMaxVersion) {
+    v = InternalVersionT::zero;
+  }
   memcpy(partialKey(), &other + 1, partialKeyLen);
   bitSet.init();
   nextFree = Node16::kMaxNodes;
@@ -451,7 +485,9 @@ inline void Node48::copyChildrenAndKeyFrom(const Node48 &other) {
   nextFree = other.nextFree;
   memcpy(index, other.index, sizeof(index));
   memset(children, 0, sizeof(children));
-  memset(childMaxVersion, 0, sizeof(childMaxVersion));
+  for (auto &v : childMaxVersion) {
+    v = InternalVersionT::zero;
+  }
   for (int i = 0; i < numChildren; ++i) {
     children[i] = other.children[i];
     childMaxVersion[i] = other.childMaxVersion[i];
@@ -468,7 +504,9 @@ inline void Node48::copyChildrenAndKeyFrom(const Node256 &other) {
          kNodeCopySize);
   memset(index, -1, sizeof(index));
   memset(children, 0, sizeof(children));
-  memset(childMaxVersion, 0, sizeof(childMaxVersion));
+  for (auto &v : childMaxVersion) {
+    v = InternalVersionT::zero;
+  }
   nextFree = other.numChildren;
   bitSet = other.bitSet;
   int i = 0;
@@ -496,8 +534,12 @@ inline void Node256::copyChildrenAndKeyFrom(const Node48 &other) {
          kNodeCopySize);
   bitSet = other.bitSet;
   memset(children, 0, sizeof(children));
-  memset(childMaxVersion, 0, sizeof(childMaxVersion));
+  for (auto &v : childMaxVersion) {
-  memset(maxOfMax, 0, sizeof(maxOfMax));
+    v = InternalVersionT::zero;
+  }
+  for (auto &v : maxOfMax) {
+    v = InternalVersionT::zero;
+  }
   bitSet.forEachInRange(
       [&](int c) {
         children[c] = other.children[other.index[c]];
@@ -515,7 +557,9 @@ inline void Node256::copyChildrenAndKeyFrom(const Node256 &other) {
   memcpy((char *)this + kNodeCopyBegin, (char *)&other + kNodeCopyBegin,
          kNodeCopySize);
   memset(children, 0, sizeof(children));
-  memset(childMaxVersion, 0, sizeof(childMaxVersion));
+  for (auto &v : childMaxVersion) {
+    v = InternalVersionT::zero;
+  }
   bitSet = other.bitSet;
   bitSet.forEachInRange(
       [&](int c) {
@@ -609,10 +653,14 @@ template <class T> struct BoundedFreeListAllocator {
     T *result = allocate_helper(partialKeyCapacity);
     if constexpr (!std::is_same_v<T, Node0>) {
       memset(result->children, 0, sizeof(result->children));
-      memset(result->childMaxVersion, 0, sizeof(result->childMaxVersion));
+      for (auto &v : result->childMaxVersion) {
+        v = InternalVersionT::zero;
+      }
     }
     if constexpr (std::is_same_v<T, Node48> || std::is_same_v<T, Node256>) {
-      memset(result->maxOfMax, 0, sizeof(result->maxOfMax));
+      for (auto &v : result->maxOfMax) {
+        v = InternalVersionT::zero;
+      }
     }
     return result;
   }
@@ -1416,7 +1464,7 @@ Node *erase(Node *self, NodeAllocators *allocators, ConflictSet::Impl *impl,
       parent48->index[parentIndex] = toRemoveChildrenIndex;
       parent48->reverseIndex[toRemoveChildrenIndex] = parentIndex;
     }
-    parent48->childMaxVersion[lastChildrenIndex] = 0;
+    parent48->childMaxVersion[lastChildrenIndex] = InternalVersionT::zero;
 
     --parent->numChildren;
 
@@ -1810,7 +1858,7 @@ bool checkMaxBetweenExclusive(Node *n, int begin, int end,
     auto *self = static_cast<Node3 *>(n);
     bool result = true;
     for (int i = 0; i < 3; ++i) {
-      result &= !((self->childMaxVersion[i] > readVersion) &
+      result &= !((self->childMaxVersion[i] > readVersion) &&
                   inBounds(self->index[i]));
     }
     return result;
@@ -2406,7 +2454,8 @@ insert(Node **self, std::span<const uint8_t> key, InternalVersionT writeVersion,
       child->partialKeyLen = 0;
       child->parent = *self;
       child->parentsIndex = key.front();
-      setMaxVersion(child, impl, kBegin ? writeVersion : 0);
+      setMaxVersion(child, impl,
+                    kBegin ? writeVersion : InternalVersionT::zero);
     }
 
     self = &child;
@@ -2632,38 +2681,43 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
       auto begin = std::span<const uint8_t>(r.begin.p, r.begin.len);
       auto end = std::span<const uint8_t>(r.end.p, r.end.len);
       result[i] =
-          reads[i].readVersion < oldestVersion ? TooOld
+          InternalVersionT(reads[i].readVersion) < oldestVersion ? TooOld
           : (end.size() > 0
-                 ? checkRangeRead(root, begin, end, reads[i].readVersion, this)
+                 ? checkRangeRead(root, begin, end,
-                 : checkPointRead(root, begin, reads[i].readVersion, this))
+                                  InternalVersionT(reads[i].readVersion), this)
+                 : checkPointRead(root, begin,
+                                  InternalVersionT(reads[i].readVersion), this))
               ? Commit
               : Conflict;
     }
   }
 
-  void addWrites(const WriteRange *writes, int count,
+  void addWrites(const WriteRange *writes, int count, int64_t writeVersion) {
-                 InternalVersionT writeVersion) {
+#if INTERNAL_VERSION_32_BIT
+    InternalVersionT::zero = oldestVersion;
+#endif
     for (int i = 0; i < count; ++i) {
       const auto &w = writes[i];
       auto begin = std::span<const uint8_t>(w.begin.p, w.begin.len);
       auto end = std::span<const uint8_t>(w.end.p, w.end.len);
       if (w.end.len > 0) {
         keyUpdates += 3;
-        addWriteRange(root, oldestVersion, begin, end, writeVersion,
+        addWriteRange(root, oldestVersion, begin, end,
-                      &allocators, this);
+                      InternalVersionT(writeVersion), &allocators, this);
       } else {
         keyUpdates += 2;
-        addPointWrite(root, oldestVersion, begin, writeVersion, &allocators,
+        addPointWrite(root, oldestVersion, begin,
-                      this);
+                      InternalVersionT(writeVersion), &allocators, this);
       }
     }
   }
 
-  void setOldestVersion(InternalVersionT oldestVersion) {
+  void setOldestVersion(int64_t o) {
-    if (oldestVersion <= this->oldestVersion) {
+    InternalVersionT oldestVersion{o};
-      return;
-    }
     this->oldestVersion = oldestVersion;
+#if INTERNAL_VERSION_32_BIT
+    InternalVersionT::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
@@ -2712,15 +2766,15 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     root = allocators.node0.allocate(0);
     root->numChildren = 0;
     root->parent = nullptr;
-    rootMaxVersion = oldestVersion;
+    rootMaxVersion = this->oldestVersion;
     root->entryPresent = false;
     root->partialKeyLen = 0;
 
     addKey(root);
 
     root->entryPresent = true;
-    root->entry.pointVersion = oldestVersion;
+    root->entry.pointVersion = this->oldestVersion;
-    root->entry.rangeVersion = oldestVersion;
+    root->entry.rangeVersion = this->oldestVersion;
   }
   ~Impl() {
 #if DEBUG_VERBOSE
@@ -3026,12 +3080,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, int64_t(maxVersion(n, impl)),
+                (void *)n, maxVersion(n, impl).toInt64(),
-                int64_t(n->entry.pointVersion), int64_t(n->entry.rangeVersion),
+                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, int64_t(maxVersion(n, impl)),
+                (void *)n, maxVersion(n, impl).toInt64(),
                 getPartialKeyPrintable(n).c_str(), x, y);
       }
       x += kSeparation;
@@ -3073,20 +3128,19 @@ Iterator firstGeq(Node *n, std::string_view key) {
       n, std::span<const uint8_t>((const uint8_t *)key.data(), key.size()));
 }
 
-[[maybe_unused]] int64_t checkMaxVersion(Node *root, Node *node,
+[[maybe_unused]] InternalVersionT
-                                         int64_t oldestVersion, bool &success,
+checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
-                                         ConflictSet::Impl *impl) {
+                bool &success, ConflictSet::Impl *impl) {
-  int64_t expected = 0;
+  InternalVersionT expected{0};
   if (node->entryPresent) {
-    expected = std::max<InternalVersionT>(expected, node->entry.pointVersion);
+    expected = std::max(expected, node->entry.pointVersion);
   }
   for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
     auto *child = getChildExists(node, i);
     expected = std::max(
         expected, checkMaxVersion(root, child, oldestVersion, success, impl));
     if (child->entryPresent) {
-      expected =
+      expected = std::max(expected, child->entry.rangeVersion);
-          std::max<InternalVersionT>(expected, child->entry.rangeVersion);
     }
   }
   auto key = getSearchPath(root);
@@ -3095,15 +3149,14 @@ Iterator firstGeq(Node *n, std::string_view key) {
   if (ok) {
     auto borrowed = firstGeq(root, inc);
     if (borrowed.n != nullptr) {
-      expected =
+      expected = std::max(expected, borrowed.n->entry.rangeVersion);
-          std::max<InternalVersionT>(expected, borrowed.n->entry.rangeVersion);
     }
   }
   if (maxVersion(node, impl) > oldestVersion &&
       maxVersion(node, impl) != expected) {
     fprintf(stderr, "%s has max version %" PRId64 " . Expected %" PRId64 "\n",
             getSearchPathPrintable(node).c_str(),
-            int64_t(maxVersion(node, impl)), expected);
+            maxVersion(node, impl).toInt64(), expected.toInt64());
     success = false;
   }
   return expected;
@@ -3161,7 +3214,8 @@ Iterator firstGeq(Node *n, std::string_view key) {
   }
 }
 
-[[maybe_unused]] bool checkCorrectness(Node *node, int64_t oldestVersion,
+[[maybe_unused]] bool checkCorrectness(Node *node,
+                                       InternalVersionT oldestVersion,
                                        ConflictSet::Impl *impl) {
   bool success = true;
 

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 = arbitrary.bounded(2) ? 0 : 0xfffffff0;
-  int64_t oldestVersion = 0;
+  int64_t writeVersion = oldestVersion + 100;
   ConflictSetImpl cs{oldestVersion};
   ReferenceImpl refImpl{oldestVersion};
 

test_conflict_set.py

@@ -66,6 +66,17 @@ def test_inner_full_words():
         cs.check(read(1, b"\x21", b"\xc2"))
 
 
+def test_internal_version_zero():
+    with DebugConflictSet() as cs:
+        cs.setOldestVersion(0xFFFFFFF0)
+        for i in range(24):
+            cs.addWrites(0xFFFFFFF1, write(bytes([i])))
+        for i in range(256 - 25, 256):
+            cs.addWrites(0xFFFFFFF1, write(bytes([i])))
+        cs.addWrites(0, write(b"\xff"))
+        cs.check(read(0xFFFFFFF1, b"\x00", b"\xff"))
+
+
 def test_decrease_capacity():
     # make a Node48, then a Node256
     for count in (17, 49):