Update benchmark in README.md

Don't try to make sense of the diff - the benchmark itself changed too
Remove Node::maxVersion
2024-02-23 17:31:48 -08:00 · 2024-02-23 17:27:19 -08:00 · 2024-02-23 17:02:46 -08:00 · 2024-02-23 16:17:28 -08:00 · 2024-02-23 16:13:53 -08:00
4 changed files with 220 additions and 110 deletions
--- a/Bench.cpp
+++ b/Bench.cpp
@@ -789,10 +789,8 @@ template <class ConflictSet_> void benchConflictSet(const std::string &name) {

    while (version < kMvccWindow) {
      auto v = ++version;
-      for (auto &w : writes) {
-        w.writeVersion = v;
-      }
-      cs.addWrites(writes.data(), writes.size());
+      writes[0].writeVersion = v;
+      cs.addWrites(writes.data(), 1);
    }

    bench.run(name + " (point writes)", [&]() {
@@ -852,6 +850,28 @@ template <class ConflictSet_> void benchConflictSet(const std::string &name) {
      cs.setOldestVersion(version - kMvccWindow);
    });
  }
+
+  bench.batch(1);
+
+  {
+    bench.run(name + " (monotonic increasing point writes)", [&]() {
+      auto v = ++version;
+      ConflictSet::WriteRange w;
+
+      uint8_t b[9];
+      b[8] = 0;
+      auto x = __builtin_bswap64(version);
+      memcpy(b, &x, 8);
+
+      w.writeVersion = v;
+      w.begin.p = b;
+      w.begin.len = 8;
+      w.end.len = 0;
+      w.end.p = b;
+      cs.addWrites(&w, 1);
+      cs.setOldestVersion(version - kMvccWindow);
+    });
+  }
 }

 int main(void) {
--- a/ConflictSet.cpp
+++ b/ConflictSet.cpp
@@ -161,7 +161,6 @@ struct Node {
  Node *parent = nullptr;
  // The max write version over all keys that start with the search path up to
  // this point
-  int64_t maxVersion;
  Entry entry;
  int16_t numChildren = 0;
  bool entryPresent = false;
@@ -174,24 +173,29 @@ struct Node {
  Type type = Type::Invalid;
 };

+struct Child {
+  int64_t childMaxVersion;
+  Node *child;
+};
+
 struct Node4 : Node {
  // Sorted
  uint8_t index[16]; // 16 so that we can use the same simd index search
                     // implementation for Node4 as Node16
-  Node *children[4];
+  Child children[4];
  Node4() { this->type = Type::Node4; }
 };

 struct Node16 : Node {
  // Sorted
  uint8_t index[16];
-  Node *children[16];
+  Child children[16];
  Node16() { this->type = Type::Node16; }
 };

 struct Node48 : Node {
  BitSet bitSet;
-  Node *children[48];
+  Child children[48];
  int8_t nextFree = 0;
  int8_t index[256];
  Node48() {
@@ -202,8 +206,13 @@ struct Node48 : Node {

 struct Node256 : Node {
  BitSet bitSet;
-  Node *children[256] = {};
-  Node256() { this->type = Type::Node256; }
+  Child children[256];
+  Node256() {
+    this->type = Type::Node256;
+    for (int i = 0; i < 256; ++i) {
+      children[i].child = nullptr;
+    }
+  }
 };

 struct NodeAllocators {
@@ -275,36 +284,55 @@ int getNodeIndex(Node16 *self, uint8_t index) {
 Node *&getChildExists(Node *self, uint8_t index) {
  if (self->type <= Type::Node16) {
    auto *self16 = static_cast<Node16 *>(self);
-    return self16->children[getNodeIndex(self16, index)];
+    return self16->children[getNodeIndex(self16, index)].child;
  } else if (self->type == Type::Node48) {
    auto *self48 = static_cast<Node48 *>(self);
    assert(self48->bitSet.test(index));
-    return self48->children[self48->index[index]];
+    return self48->children[self48->index[index]].child;
  } else {
    auto *self256 = static_cast<Node256 *>(self);
-    return self256->children[index];
+    return self256->children[index].child;
  }
  __builtin_unreachable(); // GCOVR_EXCL_LINE
 }

+// Precondition - an entry for index must exist in the node
+int64_t getChildMaxVersion(Node *self, uint8_t index) {
+  if (self->type <= Type::Node16) {
+    auto *self16 = static_cast<Node16 *>(self);
+    return self16->children[getNodeIndex(self16, index)].childMaxVersion;
+  } else if (self->type == Type::Node48) {
+    auto *self48 = static_cast<Node48 *>(self);
+    assert(self48->bitSet.test(index));
+    return self48->children[self48->index[index]].childMaxVersion;
+  } else {
+    auto *self256 = static_cast<Node256 *>(self);
+    return self256->children[index].childMaxVersion;
+  }
+  __builtin_unreachable(); // GCOVR_EXCL_LINE
+}
+
+// Precondition - an entry for index must exist in the node
+int64_t &maxVersion(Node *n, ConflictSet::Impl *);
+
 Node *getChild(Node *self, uint8_t index) {
  if (self->type <= Type::Node16) {
    auto *self16 = static_cast<Node16 *>(self);
    int i = getNodeIndex(self16, index);
    if (i >= 0) {
-      return self16->children[i];
+      return self16->children[i].child;
    }
    return nullptr;
  } else if (self->type == Type::Node48) {
    auto *self48 = static_cast<Node48 *>(self);
    int secondIndex = self48->index[index];
    if (secondIndex >= 0) {
-      return self48->children[secondIndex];
+      return self48->children[secondIndex].child;
    }
    return nullptr;
  } else {
    auto *self256 = static_cast<Node256 *>(self);
-    return self256->children[index];
+    return self256->children[index].child;
  }
 }

@@ -376,7 +404,7 @@ int getChildGeq(Node *self, int child) {

 void setChildrenParents(Node16 *n) {
  for (int i = 0; i < n->numChildren; ++i) {
-    n->children[i]->parent = n;
+    n->children[i].child->parent = n;
  }
 }

@@ -384,13 +412,13 @@ void setChildrenParents(Node48 *n) {
  if (n->numChildren < kSparseScanThreshold) {
    for (int i = n->bitSet.firstSetGeq(0); i >= 0;
         i = n->bitSet.firstSetGeq(i + 1)) {
-      n->children[n->index[i]]->parent = n;
+      n->children[n->index[i]].child->parent = n;
    }
  } else {
    for (int i = 0; i < 256; ++i) {
      int c = n->index[i];
      if (c != -1) {
-        n->children[c]->parent = n;
+        n->children[c].child->parent = n;
      }
    }
  }
@@ -400,11 +428,11 @@ void setChildrenParents(Node256 *n) {
  if (n->numChildren < kSparseScanThreshold) {
    for (int i = n->bitSet.firstSetGeq(0); i >= 0;
         i = n->bitSet.firstSetGeq(i + 1)) {
-      n->children[i]->parent = n;
+      n->children[i].child->parent = n;
    }
  } else {
    for (int i = 0; i < 256; ++i) {
-      auto *child = n->children[i];
+      auto *child = n->children[i].child;
      if (child != nullptr) {
        child->parent = n;
      }
@@ -422,17 +450,17 @@ Node *&getOrCreateChild(Node *&self, uint8_t index,
    auto *self16 = static_cast<Node16 *>(self);
    int i = getNodeIndex(self16, index);
    if (i >= 0) {
-      return self16->children[i];
+      return self16->children[i].child;
    }
  } else if (self->type == Type::Node48) {
    auto *self48 = static_cast<Node48 *>(self);
    int secondIndex = self48->index[index];
    if (secondIndex >= 0) {
-      return self48->children[secondIndex];
+      return self48->children[secondIndex].child;
    }
  } else {
    auto *self256 = static_cast<Node256 *>(self);
-    if (auto &result = self256->children[index]; result != nullptr) {
+    if (auto &result = self256->children[index].child; result != nullptr) {
      return result;
    }
  }
@@ -482,12 +510,12 @@ Node *&getOrCreateChild(Node *&self, uint8_t index,
        memmove(self16->index + i + 1, self16->index + i,
                self->numChildren - (i + 1));
        memmove(self16->children + i + 1, self16->children + i,
-                (self->numChildren - (i + 1)) * sizeof(void *));
+                (self->numChildren - (i + 1)) * sizeof(Child));
        break;
      }
    }
    self16->index[i] = index;
-    auto &result = self16->children[i];
+    auto &result = self16->children[i].child;
    result = nullptr;
    return result;
  } else if (self->type == Type::Node48) {
@@ -516,7 +544,7 @@ Node *&getOrCreateChild(Node *&self, uint8_t index,
    assert(self48->nextFree < 48);
    int nextFree = self48->nextFree++;
    self48->index[index] = nextFree;
-    auto &result = self48->children[nextFree];
+    auto &result = self48->children[nextFree].child;
    result = nullptr;
    return result;
  } else {
@@ -524,7 +552,7 @@ Node *&getOrCreateChild(Node *&self, uint8_t index,
    auto *self256 = static_cast<Node256 *>(self);
    ++self->numChildren;
    self256->bitSet.set(index);
-    return self256->children[index];
+    return self256->children[index].child;
  }
 }

@@ -566,13 +594,14 @@ void eraseChild(Node *self, uint8_t index, NodeAllocators *allocators) {
    if (toRemoveChildrenIndex != lastChildrenIndex) {
      self48->children[toRemoveChildrenIndex] =
          self48->children[lastChildrenIndex];
-      self48->index[self48->children[toRemoveChildrenIndex]->parentsIndex] =
+      self48
+          ->index[self48->children[toRemoveChildrenIndex].child->parentsIndex] =
          toRemoveChildrenIndex;
    }
  } else {
    auto *self256 = static_cast<Node256 *>(self);
    self256->bitSet.reset(index);
-    self256->children[index] = nullptr;
+    self256->children[index].child = nullptr;
  }
  --self->numChildren;
  if (self->numChildren == 0 && !self->entryPresent &&
@@ -825,13 +854,13 @@ std::string getSearchPathPrintable(Node *n);
 // point or range version according to cmp, but this version short circuits as
 // soon as it can prove that there's no conflict.
 bool checkPointRead(Node *n, const std::span<const uint8_t> key,
-                    int64_t readVersion) {
+                    int64_t readVersion, ConflictSet::Impl *impl) {
 #if DEBUG_VERBOSE && !defined(NDEBUG)
  fprintf(stderr, "Check point read: %s\n", printable(key).c_str());
 #endif
  auto remaining = key;
  for (;;) {
-    if (n->maxVersion <= readVersion) {
+    if (maxVersion(n, impl) <= readVersion) {
      return true;
    }
    if (remaining.size() == 0) {
@@ -919,7 +948,7 @@ int64_t maxBetweenExclusive(Node *n, int begin, int end) {
    auto *self = static_cast<Node16 *>(n);
    for (int i = 0; i < self->numChildren && self->index[i] < end; ++i) {
      if (begin < self->index[i]) {
-        result = std::max(result, self->children[i]->maxVersion);
+        result = std::max(result, self->children[i].childMaxVersion);
      }
    }
    break;
@@ -930,13 +959,15 @@ int64_t maxBetweenExclusive(Node *n, int begin, int end) {
      for (int i = self->bitSet.firstSetGeq(begin + 1); i < end && i >= 0;
           i = self->bitSet.firstSetGeq(i + 1)) {
        if (self->index[i] != -1) {
-          result = std::max(result, self->children[self->index[i]]->maxVersion);
+          result =
+              std::max(result, self->children[self->index[i]].childMaxVersion);
        }
      }
    } else {
      for (int i = begin + 1; i < end; ++i) {
        if (self->index[i] != -1) {
-          result = std::max(result, self->children[self->index[i]]->maxVersion);
+          result =
+              std::max(result, self->children[self->index[i]].childMaxVersion);
        }
      }
    }
@@ -947,12 +978,12 @@ int64_t maxBetweenExclusive(Node *n, int begin, int end) {
    if (self->numChildren < kSparseScanThreshold) {
      for (int i = self->bitSet.firstSetGeq(begin + 1); i < end && i >= 0;
           i = self->bitSet.firstSetGeq(i + 1)) {
-        result = std::max(result, self->children[i]->maxVersion);
+        result = std::max(result, self->children[i].childMaxVersion);
      }
    } else {
      for (int i = begin + 1; i < end; ++i) {
-        if (self->children[i] != nullptr) {
-          result = std::max(result, self->children[i]->maxVersion);
+        if (self->children[i].child != nullptr) {
+          result = std::max(result, self->children[i].childMaxVersion);
        }
      }
    }
@@ -988,13 +1019,14 @@ Vector<uint8_t> getSearchPath(Arena &arena, Node *n) {
 // Return true if the max version among all keys that start with key + [child],
 // where begin < child < end, is <= readVersion
 bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
-                          int end, int64_t readVersion) {
+                          int end, int64_t readVersion,
+                          ConflictSet::Impl *impl) {
 #if DEBUG_VERBOSE && !defined(NDEBUG)
  fprintf(stderr, "%s(%02x,%02x)*\n", printable(key).c_str(), begin, end);
 #endif
  auto remaining = key;
  for (;;) {
-    if (n->maxVersion <= readVersion) {
+    if (maxVersion(n, impl) <= readVersion) {
      return true;
    }
    if (remaining.size() == 0) {
@@ -1038,7 +1070,7 @@ bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
          if (n->entryPresent && n->entry.rangeVersion > readVersion) {
            return false;
          }
-          return n->maxVersion <= readVersion;
+          return maxVersion(n, impl) <= readVersion;
        }
        return true;
      }
@@ -1062,8 +1094,9 @@ downLeftSpine:
 // that are >= key is <= readVersion
 struct CheckRangeLeftSide {
  CheckRangeLeftSide(Node *n, std::span<const uint8_t> key, int prefixLen,
-                     int64_t readVersion)
-      : n(n), remaining(key), prefixLen(prefixLen), readVersion(readVersion) {
+                     int64_t readVersion, ConflictSet::Impl *impl)
+      : n(n), remaining(key), prefixLen(prefixLen), readVersion(readVersion),
+        impl(impl) {
 #if DEBUG_VERBOSE && !defined(NDEBUG)
    fprintf(stderr, "Check range left side from %s for keys starting with %s\n",
            printable(key).c_str(),
@@ -1075,6 +1108,7 @@ struct CheckRangeLeftSide {
  std::span<const uint8_t> remaining;
  int prefixLen;
  int64_t readVersion;
+  ConflictSet::Impl *impl;
  int searchPathLen = 0;
  bool ok;

@@ -1083,13 +1117,13 @@ struct CheckRangeLeftSide {
  bool step() {
    switch (phase) {
    case Search: {
-      if (n->maxVersion <= readVersion) {
+      if (maxVersion(n, impl) <= readVersion) {
        ok = true;
        return true;
      }
      if (remaining.size() == 0) {
        assert(searchPathLen >= prefixLen);
-        ok = n->maxVersion <= readVersion;
+        ok = maxVersion(n, impl) <= readVersion;
        return true;
      }

@@ -1109,7 +1143,7 @@ struct CheckRangeLeftSide {
            return downLeftSpine();
          }
          n = getChildExists(n, c);
-          ok = n->maxVersion <= readVersion;
+          ok = maxVersion(n, impl) <= readVersion;
          return true;
        } else {
          n = nextSibling(n);
@@ -1136,7 +1170,7 @@ struct CheckRangeLeftSide {
              ok = false;
              return true;
            }
-            ok = n->maxVersion <= readVersion;
+            ok = maxVersion(n, impl) <= readVersion;
            return true;
          } else {
            n = nextSibling(n);
@@ -1155,7 +1189,7 @@ struct CheckRangeLeftSide {
            ok = false;
            return true;
          }
-          ok = n->maxVersion <= readVersion;
+          ok = maxVersion(n, impl) <= readVersion;
          return true;
        }
      }
@@ -1188,9 +1222,9 @@ struct CheckRangeLeftSide {
 // that are < key is <= readVersion
 struct CheckRangeRightSide {
  CheckRangeRightSide(Node *n, std::span<const uint8_t> key, int prefixLen,
-                      int64_t readVersion)
+                      int64_t readVersion, ConflictSet::Impl *impl)
      : n(n), key(key), remaining(key), prefixLen(prefixLen),
-        readVersion(readVersion) {
+        readVersion(readVersion), impl(impl) {
 #if DEBUG_VERBOSE && !defined(NDEBUG)
    fprintf(stderr, "Check range right side to %s for keys starting with %s\n",
            printable(key).c_str(),
@@ -1203,6 +1237,7 @@ struct CheckRangeRightSide {
  std::span<const uint8_t> remaining;
  int prefixLen;
  int64_t readVersion;
+  ConflictSet::Impl *impl;
  int searchPathLen = 0;
  bool ok;

@@ -1301,7 +1336,7 @@ struct CheckRangeRightSide {

  bool backtrack() {
    for (;;) {
-      if (searchPathLen > prefixLen && n->maxVersion > readVersion) {
+      if (searchPathLen > prefixLen && maxVersion(n, impl) > readVersion) {
        ok = false;
        return true;
      }
@@ -1333,12 +1368,13 @@ struct CheckRangeRightSide {
 };

 bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
-                    std::span<const uint8_t> end, int64_t readVersion) {
+                    std::span<const uint8_t> end, int64_t readVersion,
+                    ConflictSet::Impl *impl) {
  int lcp = longestCommonPrefix(begin.data(), end.data(),
                                std::min(begin.size(), end.size()));
  if (lcp == int(begin.size()) && end.size() == begin.size() + 1 &&
      end.back() == 0) {
-    return checkPointRead(n, begin, readVersion);
+    return checkPointRead(n, begin, readVersion, impl);
  }

  SearchStepWise search{n, begin.subspan(0, lcp)};
@@ -1347,7 +1383,7 @@ bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
    assert(getSearchPath(arena, search.n) <=>
               begin.subspan(0, lcp - search.remaining.size()) ==
           0);
-    if (search.n->maxVersion <= readVersion) {
+    if (maxVersion(search.n, impl) <= readVersion) {
      return true;
    }
    if (search.step()) {
@@ -1367,19 +1403,19 @@ bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
  lcp -= consumed;

  if (lcp == int(begin.size())) {
-    CheckRangeRightSide checkRangeRightSide{n, end, lcp, readVersion};
+    CheckRangeRightSide checkRangeRightSide{n, end, lcp, readVersion, impl};
    while (!checkRangeRightSide.step())
      ;
    return checkRangeRightSide.ok;
  }

  if (!checkRangeStartsWith(n, begin.subspan(0, lcp), begin[lcp], end[lcp],
-                            readVersion)) {
+                            readVersion, impl)) {
    return false;
  }

-  CheckRangeLeftSide checkRangeLeftSide{n, begin, lcp + 1, readVersion};
-  CheckRangeRightSide checkRangeRightSide{n, end, lcp + 1, readVersion};
+  CheckRangeLeftSide checkRangeLeftSide{n, begin, lcp + 1, readVersion, impl};
+  CheckRangeRightSide checkRangeRightSide{n, end, lcp + 1, readVersion, impl};

  for (;;) {
    bool leftDone = checkRangeLeftSide.step();
@@ -1410,7 +1446,8 @@ bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
 // `maxVersion` at least `writeVersion` as a postcondition.
 template <bool kBegin>
 [[nodiscard]] Node *insert(Node **self, std::span<const uint8_t> key,
-                           int64_t writeVersion, NodeAllocators *allocators) {
+                           int64_t writeVersion, NodeAllocators *allocators,
+                           ConflictSet::Impl *impl) {

  for (;;) {

@@ -1421,6 +1458,7 @@ template <bool kBegin>
          (*self)->partialKey, key.data(), commonLen);
      if (partialKeyIndex < (*self)->partialKeyLen) {
        auto *old = *self;
+        int64_t oldMaxVersion = maxVersion(old, impl);

        *self = allocators->node4.allocate();

@@ -1433,6 +1471,7 @@ template <bool kBegin>
            old;
        old->parent = *self;
        old->parentsIndex = old->partialKey[partialKeyIndex];
+        maxVersion(old, impl) = oldMaxVersion;

        memmove(old->partialKey, old->partialKey + partialKeyIndex + 1,
                old->partialKeyLen - (partialKeyIndex + 1));
@@ -1452,7 +1491,8 @@ template <bool kBegin>
    }

    if constexpr (kBegin) {
-      (*self)->maxVersion = std::max((*self)->maxVersion, writeVersion);
+      auto &m = maxVersion(*self, impl);
+      m = std::max(m, writeVersion);
    }

    if (key.size() == 0) {
@@ -1460,7 +1500,8 @@ template <bool kBegin>
    }

    if constexpr (!kBegin) {
-      (*self)->maxVersion = std::max((*self)->maxVersion, writeVersion);
+      auto &m = maxVersion(*self, impl);
+      m = std::max(m, writeVersion);
    }

    auto &child = getOrCreateChild(*self, key.front(), allocators);
@@ -1468,7 +1509,7 @@ template <bool kBegin>
      child = allocators->node4.allocate();
      child->parent = *self;
      child->parentsIndex = key.front();
-      child->maxVersion =
+      maxVersion(child, impl) =
          kBegin ? writeVersion : std::numeric_limits<int64_t>::lowest();
    }

@@ -1497,13 +1538,13 @@ void destroyTree(Node *root) {

 void addPointWrite(Node *&root, int64_t oldestVersion,
                   std::span<const uint8_t> key, int64_t writeVersion,
-                   NodeAllocators *allocators) {
-  auto *n = insert<true>(&root, key, writeVersion, allocators);
+                   NodeAllocators *allocators, ConflictSet::Impl *impl) {
+  auto *n = insert<true>(&root, key, writeVersion, allocators, impl);
  if (!n->entryPresent) {
    auto *p = nextLogical(n);
    n->entryPresent = true;
    n->entry.pointVersion = writeVersion;
-    n->maxVersion = writeVersion;
+    maxVersion(n, impl) = writeVersion;
    n->entry.rangeVersion =
        p != nullptr ? p->entry.rangeVersion : oldestVersion;
  } else {
@@ -1513,13 +1554,15 @@ void addPointWrite(Node *&root, int64_t oldestVersion,

 void addWriteRange(Node *&root, int64_t oldestVersion,
                   std::span<const uint8_t> begin, std::span<const uint8_t> end,
-                   int64_t writeVersion, NodeAllocators *allocators) {
+                   int64_t writeVersion, NodeAllocators *allocators,
+                   ConflictSet::Impl *impl) {

  int lcp = longestCommonPrefix(begin.data(), end.data(),
                                std::min(begin.size(), end.size()));
  if (lcp == int(begin.size()) && end.size() == begin.size() + 1 &&
      end.back() == 0) {
-    return addPointWrite(root, oldestVersion, begin, writeVersion, allocators);
+    return addPointWrite(root, oldestVersion, begin, writeVersion, allocators,
+                         impl);
  }
  auto remaining = begin.subspan(0, lcp);

@@ -1540,7 +1583,8 @@ void addWriteRange(Node *&root, int64_t oldestVersion,
      break;
    }

-    n->maxVersion = std::max(n->maxVersion, writeVersion);
+    auto &m = maxVersion(n, impl);
+    m = std::max(m, writeVersion);

    remaining = remaining.subspan(n->partialKeyLen + 1,
                                  remaining.size() - (n->partialKeyLen + 1));
@@ -1556,7 +1600,8 @@ void addWriteRange(Node *&root, int64_t oldestVersion,
  begin = begin.subspan(consumed, begin.size() - consumed);
  end = end.subspan(consumed, end.size() - consumed);

-  auto *beginNode = insert<true>(useAsRoot, begin, writeVersion, allocators);
+  auto *beginNode =
+      insert<true>(useAsRoot, begin, writeVersion, allocators, impl);

  const bool insertedBegin = !std::exchange(beginNode->entryPresent, true);

@@ -1565,13 +1610,14 @@ void addWriteRange(Node *&root, int64_t oldestVersion,
    beginNode->entry.rangeVersion =
        p != nullptr ? p->entry.rangeVersion : oldestVersion;
    beginNode->entry.pointVersion = writeVersion;
-    beginNode->maxVersion = writeVersion;
+    maxVersion(beginNode, impl) = writeVersion;
  }
-  beginNode->maxVersion = std::max(beginNode->maxVersion, writeVersion);
+  auto &m = maxVersion(beginNode, impl);
+  m = std::max(m, writeVersion);
  beginNode->entry.pointVersion =
      std::max(beginNode->entry.pointVersion, writeVersion);

-  auto *endNode = insert<false>(useAsRoot, end, writeVersion, allocators);
+  auto *endNode = insert<false>(useAsRoot, end, writeVersion, allocators, impl);

  const bool insertedEnd = !std::exchange(endNode->entryPresent, true);

@@ -1579,14 +1625,14 @@ void addWriteRange(Node *&root, int64_t oldestVersion,
    auto *p = nextLogical(endNode);
    endNode->entry.pointVersion =
        p != nullptr ? p->entry.rangeVersion : oldestVersion;
-    endNode->maxVersion =
-        std::max(endNode->maxVersion, endNode->entry.pointVersion);
+    auto &m = maxVersion(endNode, impl);
+    m = std::max(m, endNode->entry.pointVersion);
  }
  endNode->entry.rangeVersion = writeVersion;

  if (insertedEnd) {
    // beginNode may have been invalidated
-    beginNode = insert<true>(useAsRoot, begin, writeVersion, allocators);
+    beginNode = insert<true>(useAsRoot, begin, writeVersion, allocators, impl);
  }

  for (beginNode = nextLogical(beginNode); beginNode != endNode;) {
@@ -1711,7 +1757,7 @@ Iterator firstGeq(Node *n, std::string_view key) {

 struct __attribute__((visibility("hidden"))) ConflictSet::Impl {

-  void check(const ReadRange *reads, Result *result, int count) const {
+  void check(const ReadRange *reads, Result *result, int count) {
    for (int i = 0; i < count; ++i) {
      const auto &r = reads[i];
      auto begin = std::span<const uint8_t>(r.begin.p, r.begin.len);
@@ -1719,8 +1765,8 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
      result[i] =
          reads[i].readVersion < oldestVersion ? TooOld
          : (end.size() > 0
-                 ? checkRangeRead(root, begin, end, reads[i].readVersion)
-                 : checkPointRead(root, begin, reads[i].readVersion))
+                 ? checkRangeRead(root, begin, end, reads[i].readVersion, this)
+                 : checkPointRead(root, begin, reads[i].readVersion, this))
              ? Commit
              : Conflict;
    }
@@ -1734,10 +1780,11 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
      if (w.end.len > 0) {
        keyUpdates += 2;
        addWriteRange(root, oldestVersion, begin, end, w.writeVersion,
-                      &allocators);
+                      &allocators, this);
      } else {
        keyUpdates += 1;
-        addPointWrite(root, oldestVersion, begin, w.writeVersion, &allocators);
+        addPointWrite(root, oldestVersion, begin, w.writeVersion, &allocators,
+                      this);
      }
    }
  }
@@ -1780,7 +1827,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
  explicit Impl(int64_t oldestVersion) : oldestVersion(oldestVersion) {
    // Insert ""
    root = allocators.node4.allocate();
-    root->maxVersion = oldestVersion;
+    rootMaxVersion = oldestVersion;
    root->entry.pointVersion = oldestVersion;
    root->entry.rangeVersion = oldestVersion;
    root->entryPresent = true;
@@ -1794,9 +1841,31 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
  int64_t keyUpdates = 0;

  Node *root;
+  int64_t rootMaxVersion;
  int64_t oldestVersion;
 };

+// Precondition - an entry for index must exist in the node
+int64_t &maxVersion(Node *n, ConflictSet::Impl *impl) {
+  int index = n->parentsIndex;
+  n = n->parent;
+  if (n == nullptr) {
+    return impl->rootMaxVersion;
+  }
+  if (n->type <= Type::Node16) {
+    auto *n16 = static_cast<Node16 *>(n);
+    int i = getNodeIndex(n16, index);
+    return n16->children[i].childMaxVersion;
+  } else if (n->type == Type::Node48) {
+    auto *n48 = static_cast<Node48 *>(n);
+    assert(n48->bitSet.test(index));
+    return n48->children[n48->index[index]].childMaxVersion;
+  } else {
+    auto *n256 = static_cast<Node256 *>(n);
+    return n256->children[index].childMaxVersion;
+  }
+}
+
 // ==================== END IMPLEMENTATION ====================

 // GCOVR_EXCL_START
@@ -1928,13 +1997,15 @@ std::string getSearchPath(Node *n) {
  return std::string((const char *)result.data(), result.size());
 }

-[[maybe_unused]] void debugPrintDot(FILE *file, Node *node) {
+[[maybe_unused]] void debugPrintDot(FILE *file, Node *node,
+                                    ConflictSet::Impl *impl) {

  constexpr int kSeparation = 3;

  struct DebugDotPrinter {

-    explicit DebugDotPrinter(FILE *file) : file(file) {}
+    explicit DebugDotPrinter(FILE *file, ConflictSet::Impl *impl)
+        : file(file), impl(impl) {}

    void print(Node *n, int y = 0) {
      assert(n != nullptr);
@@ -1942,12 +2013,12 @@ std::string getSearchPath(Node *n) {
        fprintf(file,
                " k_%p [label=\"m=%" PRId64 " p=%" PRId64 " r=%" PRId64
                "\n%s\", pos=\"%d,%d!\"];\n",
-                (void *)n, n->maxVersion, n->entry.pointVersion,
+                (void *)n, maxVersion(n, impl), n->entry.pointVersion,
                n->entry.rangeVersion, getPartialKeyPrintable(n).c_str(), x, y);
      } else {
        fprintf(file, " k_%p [label=\"m=%" PRId64 "\n%s\", pos=\"%d,%d!\"];\n",
-                (void *)n, n->maxVersion, getPartialKeyPrintable(n).c_str(), x,
-                y);
+                (void *)n, maxVersion(n, impl),
+                getPartialKeyPrintable(n).c_str(), x, y);
      }
      x += kSeparation;
      for (int child = getChildGeq(n, 0); child >= 0;
@@ -1959,12 +2030,13 @@ std::string getSearchPath(Node *n) {
    }
    int x = 0;
    FILE *file;
+    ConflictSet::Impl *impl;
  };

  fprintf(file, "digraph ConflictSet {\n");
  fprintf(file, " node [shape = box];\n");
  assert(node != nullptr);
-  DebugDotPrinter printer{file};
+  DebugDotPrinter printer{file, impl};
  printer.print(node);
  fprintf(file, "}\n");
 }
@@ -1983,15 +2055,16 @@ void checkParentPointers(Node *node, bool &success) {
 }

 [[maybe_unused]] int64_t checkMaxVersion(Node *root, Node *node,
-                                         int64_t oldestVersion, bool &success) {
+                                         int64_t oldestVersion, bool &success,
+                                         ConflictSet::Impl *impl) {
  int64_t expected = std::numeric_limits<int64_t>::lowest();
  if (node->entryPresent) {
    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));
+    expected = std::max(
+        expected, checkMaxVersion(root, child, oldestVersion, success, impl));
    if (child->entryPresent) {
      expected = std::max(expected, child->entry.rangeVersion);
    }
@@ -2005,9 +2078,21 @@ void checkParentPointers(Node *node, bool &success) {
      expected = std::max(expected, borrowed.n->entry.rangeVersion);
    }
  }
-  if (node->maxVersion > oldestVersion && node->maxVersion != expected) {
+  if (node->parent != nullptr &&
+      getChildMaxVersion(node->parent, node->parentsIndex) !=
+          maxVersion(node, impl)) {
+    fprintf(stderr,
+            "%s has max version %" PRId64
+            " . But parent has child max version %" PRId64 "\n",
+            getSearchPathPrintable(node).c_str(), maxVersion(node, impl),
+            getChildMaxVersion(node->parent, node->parentsIndex));
+    success = false;
+  }
+  if (maxVersion(node, impl) > oldestVersion &&
+      maxVersion(node, impl) != expected) {
    fprintf(stderr, "%s has max version %" PRId64 " . Expected %" PRId64 "\n",
-            getSearchPathPrintable(node).c_str(), node->maxVersion, expected);
+            getSearchPathPrintable(node).c_str(), maxVersion(node, impl),
+            expected);
    success = false;
  }
  return expected;
@@ -2029,11 +2114,12 @@ void checkParentPointers(Node *node, bool &success) {
  return total;
 }

-bool checkCorrectness(Node *node, int64_t oldestVersion) {
+bool checkCorrectness(Node *node, int64_t oldestVersion,
+                      ConflictSet::Impl *impl) {
  bool success = true;

  checkParentPointers(node, success);
-  checkMaxVersion(node, node, oldestVersion, success);
+  checkMaxVersion(node, node, oldestVersion, success, impl);
  checkEntriesExist(node, success);

  return success;
@@ -2064,7 +2150,7 @@ void printTree() {
    write[i].writeVersion = ++writeVersion;
  }
  cs.addWrites(write, kNumKeys);
-  debugPrintDot(stdout, cs.root);
+  debugPrintDot(stdout, cs.root, &cs);
 }

 int main(void) {
@@ -2081,16 +2167,17 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
  for (;;) {
    bool done = driver.next();
    if (!driver.ok) {
-      debugPrintDot(stdout, driver.cs.root);
+      debugPrintDot(stdout, driver.cs.root, &driver.cs);
      fflush(stdout);
      abort();
    }
 #if DEBUG_VERBOSE && !defined(NDEBUG)
    fprintf(stderr, "Check correctness\n");
 #endif
-    bool success = checkCorrectness(driver.cs.root, driver.cs.oldestVersion);
+    bool success =
+        checkCorrectness(driver.cs.root, driver.cs.oldestVersion, &driver.cs);
    if (!success) {
-      debugPrintDot(stdout, driver.cs.root);
+      debugPrintDot(stdout, driver.cs.root, &driver.cs);
      fflush(stdout);
      abort();
    }
--- a/1
+++ b/1
@@ -13,6 +13,7 @@ def CleanBuildAndTest(String cmakeArgs) {
        cd build
        ctest --no-compress-output --test-output-size-passed 100000 --test-output-size-failed 100000 -T Test -j `nproc` --timeout 90
        zstd Testing/*/Test.xml
+        ./conflict_set_bench
        '''
    }
    xunit tools: [CTest(pattern: 'build/Testing/*/Test.xml')], reduceLog: false, skipPublishingChecks: false
--- a/README.md
+++ b/README.md
@@ -56,15 +56,17 @@ Performance counters:

 |               ns/op |                op/s |    err% |     total | benchmark
 |--------------------:|--------------------:|--------:|----------:|:----------
-|              263.10 |        3,800,889.33 |    1.6% |      0.64 | `skip list (point reads)`
-|              256.36 |        3,900,800.96 |    1.7% |      0.61 | `skip list (prefix reads)`
-|              526.95 |        1,897,714.85 |    1.6% |      1.23 | `skip list (range reads)`
-|              546.54 |        1,829,676.97 |    0.5% |      1.31 | `skip list (point writes)`
-|              539.23 |        1,854,511.73 |    0.5% |      1.29 | `skip list (prefix writes)`
-|              266.68 |        3,749,799.99 |    0.6% |      0.65 | `skip list (range writes)`
-|               18.31 |       54,612,706.60 |    0.4% |      0.04 | `radix tree (point reads)`
-|               48.78 |       20,498,870.77 |    0.2% |      0.12 | `radix tree (prefix reads)`
-|              347.16 |        2,880,474.86 |    0.9% |      0.83 | `radix tree (range reads)`
-|               34.80 |       28,734,706.67 |    3.6% |      0.09 | `radix tree (point writes)`
-|               70.34 |       14,216,970.16 |    1.4% |      0.17 | `radix tree (prefix writes)`
-|               82.41 |       12,134,555.86 |    1.0% |      0.20 | `radix tree (range writes)`
+|              325.60 |        3,071,225.77 |    4.8% |      0.77 | `skip list (point reads)`
+|              297.15 |        3,365,278.10 |    1.7% |      0.72 | `skip list (prefix reads)`
+|              408.79 |        2,446,222.23 |    1.0% |      1.03 | `skip list (range reads)`
+|              261.88 |        3,818,471.08 |    1.3% |      0.73 | `skip list (point writes)`
+|              253.54 |        3,944,191.08 |    0.1% |      0.61 | `skip list (prefix writes)`
+|              258.73 |        3,865,078.52 |    0.8% |      0.62 | `skip list (range writes)`
+|              489.56 |        2,042,648.19 |    1.8% |      0.01 | `skip list (monotonic increasing point writes)`
+|               14.83 |       67,446,579.75 |    0.1% |      0.04 | `radix tree (point reads)`
+|               59.68 |       16,756,917.37 |    0.1% |      0.14 | `radix tree (prefix reads)`
+|              287.32 |        3,480,485.22 |    1.2% |      0.69 | `radix tree (range reads)`
+|               46.59 |       21,461,855.59 |    0.2% |      0.12 | `radix tree (point writes)`
+|               83.70 |       11,946,755.99 |    0.1% |      0.20 | `radix tree (prefix writes)`
+|              100.75 |        9,925,723.26 |    0.6% |      0.25 | `radix tree (range writes)`
+|              118.37 |        8,448,345.29 |    0.6% |      0.01 | `radix tree (monotonic increasing point writes)`
Author	SHA1	Message	Date
Andrew Noyes	70e3377eac	Update benchmark in README.md All checks were successful Tests / Release [gcc] total: 471, passed: 471 Details GNU C Compiler (gcc) \|Total\|New\|Outstanding\|Fixed\|Trend \|:-:\|:-:\|:-:\|:-:\|:-: \|0\|0\|0\|0\|:clap: Reference build: <a href="https://jenkins.weaselab.dev/job/weaselab/job/conflict-set/job/main/37//gcc">weaselab » conflict-set » main #37</a> Details Tests / Coverage total: 469, passed: 469 Details weaselab/conflict-set/pipeline/head This commit looks good Details Don't try to make sense of the diff - the benchmark itself changed too	2024-02-23 17:31:48 -08:00
Andrew Noyes	adaa652d0d	Remove Node::maxVersion	2024-02-23 17:27:19 -08:00
Andrew Noyes	921005edb3	Add childMaxVersion	2024-02-23 17:02:46 -08:00
Andrew Noyes	d43a8a5907	Run benchmark in jenkins	2024-02-23 16:17:28 -08:00
Andrew Noyes	588e8eb87f	Add monotonic increasing point writes workload	2024-02-23 16:13:53 -08:00