Avoid some unnecessary work

This saves instructions according to cachegrind

Bench.cpp

@@ -787,13 +787,21 @@ template <class ConflictSet_> void benchConflictSet(const std::string &name) {
       ++iter;
     }
 
+    while (version < kMvccWindow) {
+      auto v = ++version;
+      for (auto &w : writes) {
+        w.writeVersion = v;
+      }
+      cs.addWrites(writes.data(), writes.size());
+    }
+
     bench.run(name + " (point writes)", [&]() {
       auto v = ++version;
       for (auto &w : writes) {
         w.writeVersion = v;
       }
       cs.addWrites(writes.data(), writes.size());
-      cs.setOldestVersion(std::max<int64_t>(version - kMvccWindow, 0));
+      cs.setOldestVersion(version - kMvccWindow);
     });
   }
 
@@ -817,7 +825,7 @@ template <class ConflictSet_> void benchConflictSet(const std::string &name) {
         w.writeVersion = v;
       }
       cs.addWrites(writes.data(), writes.size());
-      cs.setOldestVersion(std::max<int64_t>(version - kMvccWindow, 0));
+      cs.setOldestVersion(version - kMvccWindow);
     });
   }
 
@@ -841,7 +849,7 @@ template <class ConflictSet_> void benchConflictSet(const std::string &name) {
         w.writeVersion = v;
       }
       cs.addWrites(writes.data(), writes.size());
-      cs.setOldestVersion(std::max<int64_t>(version - kMvccWindow, 0));
+      cs.setOldestVersion(version - kMvccWindow);
     });
   }
 }

ConflictSet.cpp

@@ -41,6 +41,8 @@ limitations under the License.
 
 // ==================== BEGIN IMPLEMENTATION ====================
 
+constexpr int kSparseScanThreshold = 32;
+
 struct Entry {
   int64_t pointVersion;
   int64_t rangeVersion;
@@ -372,9 +374,41 @@ int getChildGeq(Node *self, int child) {
   return -1;
 }
 
-void setChildrenParents(Node *node) {
+void setChildrenParents(Node16 *n) {
-  for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
+  for (int i = 0; i < n->numChildren; ++i) {
-    getChildExists(node, i)->parent = node;
+    n->children[i]->parent = n;
+  }
+}
+
+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;
+    }
+  } else {
+    for (int i = 0; i < 256; ++i) {
+      int c = n->index[i];
+      if (c != -1) {
+        n->children[c]->parent = n;
+      }
+    }
+  }
+}
+
+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;
+    }
+  } else {
+    for (int i = 0; i < 256; ++i) {
+      auto *child = n->children[i];
+      if (child != nullptr) {
+        child->parent = n;
+      }
+    }
   }
 }
 
@@ -382,45 +416,45 @@ void setChildrenParents(Node *node) {
 // reference if null
 Node *&getOrCreateChild(Node *&self, uint8_t index,
                         NodeAllocators *allocators) {
-  if (self->type == Type::Node4) {
-    auto *self4 = static_cast<Node4 *>(self);
-    if (int i = getNodeIndex((Node16 *)self4, index); i >= 0) {
-      return self4->children[i];
-    }
-    if (self->numChildren == 4) {
-      auto *newSelf = allocators->node16.allocate();
-      memcpy((void *)newSelf, self, offsetof(Node, type));
-      memcpy(newSelf->index, self4->index, 4);
-      memcpy(newSelf->children, self4->children, 4 * sizeof(void *));
-      allocators->node4.release(self4);
-      self = newSelf;
-      setChildrenParents(self);
-      goto insert16;
-    } else {
-      ++self->numChildren;
-      for (int i = 0; i < int(self->numChildren) - 1; ++i) {
-        if (int(self4->index[i]) > int(index)) {
-          memmove(self4->index + i + 1, self4->index + i,
-                  self->numChildren - (i + 1));
-          memmove(self4->children + i + 1, self4->children + i,
-                  (self->numChildren - (i + 1)) * sizeof(void *));
-          self4->index[i] = index;
-          self4->children[i] = nullptr;
-          return self4->children[i];
-        }
-      }
-      self4->index[self->numChildren - 1] = index;
-      self4->children[self->numChildren - 1] = nullptr;
-      return self4->children[self->numChildren - 1];
-    }
-  } else if (self->type == Type::Node16) {
-  insert16:
-    auto *self16 = static_cast<Node16 *>(self);
 
-    if (int i = getNodeIndex(self16, index); i >= 0) {
+  // Fast path for if it exists already
+  if (self->type <= Type::Node16) {
+    auto *self16 = static_cast<Node16 *>(self);
+    int i = getNodeIndex(self16, index);
+    if (i >= 0) {
       return self16->children[i];
     }
+  } else if (self->type == Type::Node48) {
+    auto *self48 = static_cast<Node48 *>(self);
+    int secondIndex = self48->index[index];
+    if (secondIndex >= 0) {
+      return self48->children[secondIndex];
+    }
+  } else {
+    auto *self256 = static_cast<Node256 *>(self);
+    if (auto &result = self256->children[index]; result != nullptr) {
+      return result;
+    }
+  }
+
+  if (self->type == Type::Node4) {
+    auto *self4 = static_cast<Node4 *>(self);
+
+    if (self->numChildren == 4) {
+      auto *newSelf = allocators->node16.allocate();
+      memcpy((void *)newSelf, self, sizeof(Node4));
+      newSelf->type = Type::Node16;
+      allocators->node4.release(self4);
+      setChildrenParents(newSelf);
+      self = newSelf;
+    }
+
+    goto insert16;
+
+  } else if (self->type == Type::Node16) {
+
     if (self->numChildren == 16) {
+      auto *self16 = static_cast<Node16 *>(self);
       auto *newSelf = allocators->node48.allocate();
       memcpy((void *)newSelf, self, offsetof(Node, type));
       newSelf->nextFree = 16;
@@ -433,65 +467,64 @@ Node *&getOrCreateChild(Node *&self, uint8_t index,
       }
       assert(i == 16);
       allocators->node16.release(self16);
+      setChildrenParents(newSelf);
       self = newSelf;
-      setChildrenParents(self);
       goto insert48;
-    } else {
+    }
+
+  insert16:
+    auto *self16 = static_cast<Node16 *>(self);
+
     ++self->numChildren;
-      for (int i = 0; i < int(self->numChildren) - 1; ++i) {
+    int i = 0;
+    for (; i < int(self->numChildren) - 1; ++i) {
       if (int(self16->index[i]) > int(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 *));
+        break;
+      }
+    }
     self16->index[i] = index;
-          self16->children[i] = nullptr;
+    auto &result = self16->children[i];
-          return self16->children[i];
+    result = nullptr;
-        }
+    return result;
-      }
-      self16->index[self->numChildren - 1] = index;
-      self16->children[self->numChildren - 1] = nullptr;
-      return self16->children[self->numChildren - 1];
-    }
   } else if (self->type == Type::Node48) {
-  insert48:
-    auto *self48 = static_cast<Node48 *>(self);
-    if (int c = self48->index[index];
 
-        c >= 0) {
-      return self48->children[c];
-    }
     if (self->numChildren == 48) {
+      auto *self48 = static_cast<Node48 *>(self);
       auto *newSelf = allocators->node256.allocate();
       memcpy((void *)newSelf, self, offsetof(Node, type));
+      newSelf->bitSet = self48->bitSet;
       for (int i = 0; i < 256; ++i) {
-        if (self48->bitSet.test(i)) {
+        int c = self48->index[i];
-          newSelf->bitSet.set(i);
+        if (c >= 0) {
-          newSelf->children[i] = self48->children[self48->index[i]];
+          newSelf->children[i] = self48->children[c];
         }
       }
       allocators->node48.release(self48);
+      setChildrenParents(newSelf);
       self = newSelf;
-      setChildrenParents(self);
       goto insert256;
-    } else {
+    }
+  insert48:
+
+    auto *self48 = static_cast<Node48 *>(self);
     self48->bitSet.set(index);
     ++self->numChildren;
     assert(self48->nextFree < 48);
-      self48->index[index] = self48->nextFree;
+    int nextFree = self48->nextFree++;
-      self48->children[self48->nextFree] = nullptr;
+    self48->index[index] = nextFree;
-      return self48->children[self48->nextFree++];
+    auto &result = self48->children[nextFree];
-    }
+    result = nullptr;
+    return result;
   } else {
   insert256:
     auto *self256 = static_cast<Node256 *>(self);
-    auto *&result = self256->children[index];
-    if (result) {
-      return result;
-    }
     ++self->numChildren;
     self256->bitSet.set(index);
-    return result;
+    return self256->children[index];
   }
 }
 
@@ -532,7 +565,7 @@ void eraseChild(Node *self, uint8_t index, NodeAllocators *allocators) {
     assert(lastChildrenIndex >= 0);
     if (toRemoveChildrenIndex != lastChildrenIndex) {
       self48->children[toRemoveChildrenIndex] =
-          std::exchange(self48->children[lastChildrenIndex], nullptr);
+          self48->children[lastChildrenIndex];
       self48->index[self48->children[toRemoveChildrenIndex]->parentsIndex] =
           toRemoveChildrenIndex;
     }
@@ -811,8 +844,9 @@ bool checkPointRead(Node *n, const std::span<const uint8_t> key,
       goto downLeftSpine;
     }
 
+    auto *child = getChild(n, remaining[0]);
+    if (child == nullptr) {
       int c = getChildGeq(n, remaining[0]);
-    if (c != remaining[0]) {
       if (c >= 0) {
         n = getChildExists(n, c);
         goto downLeftSpine;
@@ -822,7 +856,7 @@ bool checkPointRead(Node *n, const std::span<const uint8_t> key,
       }
     }
 
-    n = getChildExists(n, c);
+    n = child;
     remaining = remaining.subspan(1, remaining.size() - 1);
 
     if (n->partialKeyLen > 0) {
@@ -869,7 +903,6 @@ int64_t maxBetweenExclusive(Node *n, int begin, int end) {
   assert(end <= 256);
   assert(begin < end);
   int64_t result = std::numeric_limits<int64_t>::lowest();
-  constexpr int kSparseThreshold = 32;
   {
     int c = getChildGeq(n, begin + 1);
     if (c >= 0 && c < end) {
@@ -893,7 +926,7 @@ int64_t maxBetweenExclusive(Node *n, int begin, int end) {
   }
   case Type::Node48: {
     auto *self = static_cast<Node48 *>(n);
-    if (self->numChildren < kSparseThreshold) {
+    if (self->numChildren < kSparseScanThreshold) {
       for (int i = self->bitSet.firstSetGeq(begin + 1); i < end && i >= 0;
            i = self->bitSet.firstSetGeq(i + 1)) {
         if (self->index[i] != -1) {
@@ -911,7 +944,7 @@ int64_t maxBetweenExclusive(Node *n, int begin, int end) {
   }
   case Type::Node256: {
     auto *self = static_cast<Node256 *>(n);
-    if (self->numChildren < kSparseThreshold) {
+    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);
@@ -968,8 +1001,9 @@ bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
       return maxBetweenExclusive(n, begin, end) <= readVersion;
     }
 
+    auto *child = getChild(n, remaining[0]);
+    if (child == nullptr) {
       int c = getChildGeq(n, remaining[0]);
-    if (c != remaining[0]) {
       if (c >= 0) {
         n = getChildExists(n, c);
         goto downLeftSpine;
@@ -979,7 +1013,7 @@ bool checkRangeStartsWith(Node *n, std::span<const uint8_t> key, int begin,
       }
     }
 
-    n = getChildExists(n, c);
+    n = child;
     remaining = remaining.subspan(1, remaining.size() - 1);
 
     if (n->partialKeyLen > 0) {
@@ -1066,8 +1100,9 @@ struct CheckRangeLeftSide {
         }
       }
 
+      auto *child = getChild(n, remaining[0]);
+      if (child == nullptr) {
         int c = getChildGeq(n, remaining[0]);
-      if (c != remaining[0]) {
         if (c >= 0) {
           if (searchPathLen < prefixLen) {
             n = getChildExists(n, c);
@@ -1082,7 +1117,7 @@ struct CheckRangeLeftSide {
         }
       }
 
-      n = getChildExists(n, c);
+      n = child;
       remaining = remaining.subspan(1, remaining.size() - 1);
       ++searchPathLen;
 
@@ -1208,8 +1243,9 @@ struct CheckRangeRightSide {
         return true;
       }
 
+      auto *child = getChild(n, remaining[0]);
+      if (child == nullptr) {
         int c = getChildGeq(n, remaining[0]);
-      if (c != remaining[0]) {
         if (c >= 0) {
           n = getChildExists(n, c);
           return downLeftSpine();
@@ -1218,7 +1254,7 @@ struct CheckRangeRightSide {
         }
       }
 
-      n = getChildExists(n, c);
+      n = child;
       remaining = remaining.subspan(1, remaining.size() - 1);
       ++searchPathLen;
 
@@ -1372,27 +1408,30 @@ bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
 // setting 'entry' fields and `maxVersion` on the result, which may have
 // !entryPresent. The search path of the result's parent will have
 // `maxVersion` at least `writeVersion` as a postcondition.
-[[nodiscard]] Node *insert(Node **self_, std::span<const uint8_t> key,
+template <bool kBegin>
-                           int64_t writeVersion, bool begin,
+[[nodiscard]] Node *insert(Node **self, std::span<const uint8_t> key,
-                           NodeAllocators *allocators) {
+                           int64_t writeVersion, NodeAllocators *allocators) {
-  for (;;) {
-    auto &self = *self_;
-    // Handle an existing partial key
-    int commonLen = std::min<int>(self->partialKeyLen, key.size());
-    int partialKeyIndex =
-        longestCommonPrefixPartialKey(self->partialKey, key.data(), commonLen);
-    if (partialKeyIndex < self->partialKeyLen) {
-      auto *old = self;
-      self = allocators->node4.allocate();
-      self->maxVersion = old->maxVersion;
-      self->partialKeyLen = partialKeyIndex;
-      self->parent = old->parent;
-      self->parentsIndex = old->parentsIndex;
-      memcpy(self->partialKey, old->partialKey, partialKeyIndex);
 
-      getOrCreateChild(self, old->partialKey[partialKeyIndex], allocators) =
+  for (;;) {
+
+    if ((*self)->partialKeyLen > 0) {
+      // Handle an existing partial key
+      int commonLen = std::min<int>((*self)->partialKeyLen, key.size());
+      int partialKeyIndex = longestCommonPrefixPartialKey(
+          (*self)->partialKey, key.data(), commonLen);
+      if (partialKeyIndex < (*self)->partialKeyLen) {
+        auto *old = *self;
+
+        *self = allocators->node4.allocate();
+
+        memcpy((void *)*self, old, offsetof(Node, type));
+        (*self)->partialKeyLen = partialKeyIndex;
+        (*self)->entryPresent = false;
+        (*self)->numChildren = 0;
+
+        getOrCreateChild(*self, old->partialKey[partialKeyIndex], allocators) =
             old;
-      old->parent = self;
+        old->parent = *self;
         old->parentsIndex = old->partialKey[partialKeyIndex];
 
         memmove(old->partialKey, old->partialKey + partialKeyIndex + 1,
@@ -1401,35 +1440,39 @@ bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
       }
       key = key.subspan(partialKeyIndex, key.size() - partialKeyIndex);
 
+    } else {
       // Consider adding a partial key
-    if (self->numChildren == 0 && !self->entryPresent) {
+      if ((*self)->numChildren == 0 && !(*self)->entryPresent) {
-      self->partialKeyLen = std::min<int>(key.size(), self->kPartialKeyMaxLen);
+        (*self)->partialKeyLen =
-      memcpy(self->partialKey, key.data(), self->partialKeyLen);
+            std::min<int>(key.size(), (*self)->kPartialKeyMaxLen);
-      key = key.subspan(self->partialKeyLen, key.size() - self->partialKeyLen);
+        memcpy((*self)->partialKey, key.data(), (*self)->partialKeyLen);
+        key = key.subspan((*self)->partialKeyLen,
+                          key.size() - (*self)->partialKeyLen);
+      }
     }
 
-    if (begin) {
+    if constexpr (kBegin) {
-      self->maxVersion = std::max(self->maxVersion, writeVersion);
+      (*self)->maxVersion = std::max((*self)->maxVersion, writeVersion);
     }
 
     if (key.size() == 0) {
-      return self;
+      return *self;
     }
 
-    if (!begin) {
+    if constexpr (!kBegin) {
-      self->maxVersion = std::max(self->maxVersion, writeVersion);
+      (*self)->maxVersion = std::max((*self)->maxVersion, writeVersion);
     }
 
-    auto &child = getOrCreateChild(self, key.front(), allocators);
+    auto &child = getOrCreateChild(*self, key.front(), allocators);
     if (!child) {
       child = allocators->node4.allocate();
-      child->parent = self;
+      child->parent = *self;
       child->parentsIndex = key.front();
       child->maxVersion =
-          begin ? writeVersion : std::numeric_limits<int64_t>::lowest();
+          kBegin ? writeVersion : std::numeric_limits<int64_t>::lowest();
     }
 
-    self_ = &child;
+    self = &child;
     key = key.subspan(1, key.size() - 1);
   }
 }
@@ -1455,7 +1498,7 @@ 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(&root, key, writeVersion, true, allocators);
+  auto *n = insert<true>(&root, key, writeVersion, allocators);
   if (!n->entryPresent) {
     auto *p = nextLogical(n);
     n->entryPresent = true;
@@ -1465,7 +1508,6 @@ void addPointWrite(Node *&root, int64_t oldestVersion,
         p != nullptr ? p->entry.rangeVersion : oldestVersion;
   } else {
     n->entry.pointVersion = std::max(n->entry.pointVersion, writeVersion);
-    n->maxVersion = std::max(n->maxVersion, writeVersion);
   }
 }
 
@@ -1514,7 +1556,7 @@ void addWriteRange(Node *&root, int64_t oldestVersion,
   begin = begin.subspan(consumed, begin.size() - consumed);
   end = end.subspan(consumed, end.size() - consumed);
 
-  auto *beginNode = insert(useAsRoot, begin, writeVersion, true, allocators);
+  auto *beginNode = insert<true>(useAsRoot, begin, writeVersion, allocators);
 
   const bool insertedBegin = !std::exchange(beginNode->entryPresent, true);
 
@@ -1529,7 +1571,7 @@ void addWriteRange(Node *&root, int64_t oldestVersion,
   beginNode->entry.pointVersion =
       std::max(beginNode->entry.pointVersion, writeVersion);
 
-  auto *endNode = insert(useAsRoot, end, writeVersion, false, allocators);
+  auto *endNode = insert<false>(useAsRoot, end, writeVersion, allocators);
 
   const bool insertedEnd = !std::exchange(endNode->entryPresent, true);
 
@@ -1544,7 +1586,7 @@ void addWriteRange(Node *&root, int64_t oldestVersion,
 
   if (insertedEnd) {
     // beginNode may have been invalidated
-    beginNode = insert(useAsRoot, begin, writeVersion, true, allocators);
+    beginNode = insert<true>(useAsRoot, begin, writeVersion, allocators);
   }
 
   for (beginNode = nextLogical(beginNode); beginNode != endNode;) {
@@ -1585,8 +1627,9 @@ struct FirstGeqStepwise {
         return downLeftSpine();
       }
 
+      auto *child = getChild(n, remaining[0]);
+      if (child == nullptr) {
         int c = getChildGeq(n, remaining[0]);
-      if (c != remaining[0]) {
         if (c >= 0) {
           n = getChildExists(n, c);
           return downLeftSpine();
@@ -1596,7 +1639,7 @@ struct FirstGeqStepwise {
         }
       }
 
-      n = getChildExists(n, c);
+      n = child;
       remaining = remaining.subspan(1, remaining.size() - 1);
 
       if (n->partialKeyLen > 0) {
@@ -1700,6 +1743,9 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   }
 
   void setOldestVersion(int64_t oldestVersion) {
+    if (oldestVersion <= this->oldestVersion) {
+      return;
+    }
     this->oldestVersion = oldestVersion;
     Node *prev = firstGeq(root, removalKey).n;
     // There's no way to erase removalKey without introducing a key after it