Bound memory, and disable free list for now

CC #9

ConflictSet.cpp

@@ -263,8 +263,8 @@ struct Node256 : Node {
 
 // Bound memory usage following the analysis in the ART paper
 
-constexpr int kBytesPerKey = 86;
-constexpr int kMinChildrenNode4 = 2;
+constexpr int kBytesPerKey = 121;
+constexpr int kMinChildrenNode4 = 1;
 constexpr int kMinChildrenNode16 = 5;
 constexpr int kMinChildrenNode48 = 17;
 constexpr int kMinChildrenNode256 = 49;
@@ -273,11 +273,10 @@ static_assert(sizeof(Node256) < kMinChildrenNode256 * kBytesPerKey);
 static_assert(sizeof(Node48) < kMinChildrenNode48 * kBytesPerKey);
 static_assert(sizeof(Node16) < kMinChildrenNode16 * kBytesPerKey);
 static_assert(sizeof(Node4) < kMinChildrenNode4 * kBytesPerKey);
+static_assert(sizeof(Node0) < kBytesPerKey);
 
-// Bounds memory usage in free list, but does not account for memory for partial
-// keys.
-template <class T, int64_t kMemoryBound = (1 << 20),
-          int64_t kMaxIndividual = (1 << 10)>
+// TODO revive freeList? It's making bounding memory usage tricky
+template <class T, int64_t kMemoryBound = 0, int64_t kMaxIndividual = (1 << 10)>
 struct BoundedFreeListAllocator {
   static_assert(sizeof(T) >= sizeof(void *));
   static_assert(std::derived_from<T, Node>);
@@ -465,6 +464,8 @@ int64_t getChildMaxVersion(Node *self, uint8_t index) {
 // Precondition - an entry for index must exist in the node
 int64_t &maxVersion(Node *n, ConflictSet::Impl *);
 
+Node *&getRoot(ConflictSet::Impl *);
+
 Node *getChild(Node *self, uint8_t index) {
   if (self->type <= Type::Node16) {
     auto *self16 = static_cast<Node16 *>(self);
@@ -552,6 +553,12 @@ int getChildGeq(Node *self, int child) {
   return -1;
 }
 
+void setChildrenParents(Node4 *n) {
+  for (int i = 0; i < n->numChildren; ++i) {
+    n->children[i].child->parent = n;
+  }
+}
+
 void setChildrenParents(Node16 *n) {
   for (int i = 0; i < n->numChildren; ++i) {
     n->children[i].child->parent = n;
@@ -706,8 +713,254 @@ Node *&getOrCreateChild(Node *&self, uint8_t index,
   }
 }
 
+namespace {
+std::string getSearchPathPrintable(Node *n);
+}
+
+void maybeDecreaseCapacity(Node *&self, Node *&root,
+                           NodeAllocators *allocators) {
+  if (self->numChildren > 0 &&
+      self->numChildren * self->partialKeyLen >= self->partialKeyCapacity) {
+    return;
+  }
+
+  int newCapacity =
+      std::max(self->partialKeyLen, self->numChildren * self->partialKeyLen);
+
+  switch (self->type) {
+  case Type::Node0: {
+    auto *self0 = (Node0 *)self;
+    auto *newSelf = allocators->node0.allocate(newCapacity);
+    memcpy((char *)newSelf + kNodeCopyBegin, (char *)self + kNodeCopyBegin,
+           kNodeCopySize);
+    memcpy(newSelf->partialKey(), self0->partialKey(), self->partialKeyLen);
+    (self->parent ? getChildExists(self->parent, self->parentsIndex) : root) =
+        newSelf;
+    allocators->node0.release(self0);
+    self = newSelf;
+  } break;
+  case Type::Node4: {
+    auto *self4 = (Node4 *)self;
+    auto *newSelf = allocators->node4.allocate(newCapacity);
+    memcpy((char *)newSelf + kNodeCopyBegin, (char *)self + kNodeCopyBegin,
+           kNodeCopySize);
+    memcpy(newSelf->partialKey(), self4->partialKey(), self->partialKeyLen);
+    // TODO replace with memcpy?
+    for (int i = 0; i < 4; ++i) {
+      newSelf->index[i] = self4->index[i];
+      newSelf->children[i] = self4->children[i];
+    }
+    (self->parent ? getChildExists(self->parent, self->parentsIndex) : root) =
+        newSelf;
+    setChildrenParents(newSelf);
+    allocators->node4.release(self4);
+    self = newSelf;
+  } break;
+  case Type::Node16: {
+    auto *self16 = (Node16 *)self;
+    auto *newSelf = allocators->node16.allocate(newCapacity);
+    memcpy((char *)newSelf + kNodeCopyBegin, (char *)self + kNodeCopyBegin,
+           kNodeCopySize);
+    memcpy(newSelf->partialKey(), self16->partialKey(), self->partialKeyLen);
+    // TODO replace with memcpy?
+    for (int i = 0; i < 16; ++i) {
+      newSelf->index[i] = self16->index[i];
+      newSelf->children[i] = self16->children[i];
+    }
+    (self->parent ? getChildExists(self->parent, self->parentsIndex) : root) =
+        newSelf;
+    setChildrenParents(newSelf);
+    allocators->node16.release(self16);
+    self = newSelf;
+  } break;
+  case Type::Node48: {
+    auto *self48 = (Node48 *)self;
+    auto *newSelf = allocators->node48.allocate(newCapacity);
+    memcpy((char *)newSelf + kNodeCopyBegin, (char *)self + kNodeCopyBegin,
+           kNodeCopySize);
+    memcpy(newSelf->partialKey(), self48->partialKey(), self->partialKeyLen);
+    newSelf->bitSet = self48->bitSet;
+    newSelf->bitSet.forEachInRange(
+        [&](int c) {
+          int index = newSelf->nextFree;
+          newSelf->index[c] = index;
+          newSelf->children[index] = self48->children[self48->index[c]];
+          ++newSelf->nextFree;
+        },
+        0, 256);
+    (self->parent ? getChildExists(self->parent, self->parentsIndex) : root) =
+        newSelf;
+    setChildrenParents(newSelf);
+    allocators->node48.release(self48);
+    self = newSelf;
+  } break;
+  case Type::Node256: {
+    auto *self256 = (Node256 *)self;
+    auto *newSelf = allocators->node256.allocate(newCapacity);
+    memcpy((char *)newSelf + kNodeCopyBegin, (char *)self + kNodeCopyBegin,
+           kNodeCopySize);
+    memcpy(newSelf->partialKey(), self256->partialKey(), self->partialKeyLen);
+    newSelf->bitSet = self256->bitSet;
+    newSelf->bitSet.forEachInRange(
+        [&](int c) { newSelf->children[c] = self256->children[c]; }, 0, 256);
+    (self->parent ? getChildExists(self->parent, self->parentsIndex) : root) =
+        newSelf;
+    setChildrenParents(newSelf);
+    allocators->node256.release(self256);
+    self = newSelf;
+  } break;
+  }
+}
+
+// TODO fuse into erase child so we don't need to repeat branches on type
+void maybeDownsize(Node *&self, Node *&root, NodeAllocators *allocators,
+                   ConflictSet::Impl *impl) {
+  switch (self->type) {
+  case Type::Node0:
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  case Type::Node4: {
+    auto *self4 = (Node4 *)self;
+    if (self->numChildren == 0) {
+      auto *newSelf = allocators->node0.allocate(self->partialKeyLen);
+      memcpy((char *)newSelf + kNodeCopyBegin, (char *)self + kNodeCopyBegin,
+             kNodeCopySize);
+      memcpy(newSelf->partialKey(), self4->partialKey(), self->partialKeyLen);
+
+      if (self->parent == nullptr) {
+        root = newSelf;
+      } else {
+        getChildExists(self->parent, self->parentsIndex) = newSelf;
+      }
+
+      allocators->node4.release(self4);
+      self = newSelf;
+    } else if (self->numChildren == 1) {
+      if (!self->entryPresent) {
+        auto *child = self4->children[0].child;
+        int minCapacity = self4->partialKeyLen + 1 + child->partialKeyLen;
+
+        if (minCapacity > child->partialKeyCapacity) {
+          // TODO resize child? It seems to be quite challenging to implement,
+          // since callers would now have to account for erase invalidating
+          // other nodes. We could lower kBytesPerKey by doing this though.
+          return;
+        }
+
+        // Merge partial key with child
+#if DEBUG_VERBOSE && !defined(NDEBUG)
+        fprintf(stderr, "Merge %s into %s\n",
+                getSearchPathPrintable(self).c_str(),
+                getSearchPathPrintable(child).c_str());
+#endif
+
+        int64_t childMaxVersion = maxVersion(child, impl);
+
+        // Construct new partial key for child
+        memmove(child->partialKey() + self4->partialKeyLen + 1,
+                child->partialKey(), child->partialKeyLen);
+        memcpy(child->partialKey(), self4->partialKey(), self->partialKeyLen);
+        child->partialKey()[self4->partialKeyLen] = self4->index[0];
+        child->partialKeyLen += 1 + self4->partialKeyLen;
+
+        child->parent = self->parent;
+        child->parentsIndex = self->parentsIndex;
+        if (self->parent == nullptr) {
+          root = child;
+        } else {
+          getChildExists(self->parent, self->parentsIndex) = child;
+        }
+
+        // Max versions are stored in the parent, so we need to update it now
+        // that we have a new parent.
+        maxVersion(child, impl) = childMaxVersion;
+
+        allocators->node4.release(self4);
+        self = child;
+      }
+    }
+  } break;
+  case Type::Node16:
+    if (self->numChildren < kMinChildrenNode16) {
+      auto *self16 = (Node16 *)self;
+      auto *newSelf = allocators->node4.allocate(self->partialKeyLen);
+      memcpy((char *)newSelf + kNodeCopyBegin, (char *)self + kNodeCopyBegin,
+             kNodeCopySize);
+      memcpy(newSelf->partialKey(), self16->partialKey(), self->partialKeyLen);
+      // TODO replace with memcpy?
+      for (int i = 0; i < 4; ++i) {
+        newSelf->index[i] = self16->index[i];
+        newSelf->children[i] = self16->children[i];
+      }
+      allocators->node16.release(self16);
+      setChildrenParents(newSelf);
+      if (newSelf->parent == nullptr) {
+        root = newSelf;
+      } else {
+        getChildExists(newSelf->parent, newSelf->parentsIndex) = newSelf;
+      }
+      self = newSelf;
+    }
+    break;
+  case Type::Node48:
+    if (self->numChildren < kMinChildrenNode48) {
+      auto *self48 = (Node48 *)self;
+      auto *newSelf = allocators->node16.allocate(self->partialKeyLen);
+      memcpy((char *)newSelf + kNodeCopyBegin, (char *)self + kNodeCopyBegin,
+             kNodeCopySize);
+      memcpy(newSelf->partialKey(), self48->partialKey(), self->partialKeyLen);
+
+      int i = 0;
+      self48->bitSet.forEachInRange(
+          [&](int c) {
+            newSelf->index[i] = c;
+            newSelf->children[i] = self48->children[self48->index[c]];
+            ++i;
+          },
+          0, 256);
+
+      allocators->node48.release(self48);
+      setChildrenParents(newSelf);
+      if (newSelf->parent == nullptr) {
+        root = newSelf;
+      } else {
+        getChildExists(newSelf->parent, newSelf->parentsIndex) = newSelf;
+      }
+      self = newSelf;
+    }
+    break;
+  case Type::Node256:
+    if (self->numChildren < kMinChildrenNode256) {
+      auto *self256 = (Node256 *)self;
+      auto *newSelf = allocators->node48.allocate(self->partialKeyLen);
+      memcpy((char *)newSelf + kNodeCopyBegin, (char *)self + kNodeCopyBegin,
+             kNodeCopySize);
+      memcpy(newSelf->partialKey(), self256->partialKey(), self->partialKeyLen);
+
+      newSelf->bitSet = self256->bitSet;
+      newSelf->bitSet.forEachInRange(
+          [&](int c) {
+            newSelf->index[c] = newSelf->nextFree;
+            newSelf->children[newSelf->nextFree] = self256->children[c];
+            ++newSelf->nextFree;
+          },
+          0, 256);
+
+      allocators->node256.release(self256);
+      setChildrenParents(newSelf);
+      if (newSelf->parent == nullptr) {
+        root = newSelf;
+      } else {
+        getChildExists(newSelf->parent, newSelf->parentsIndex) = newSelf;
+      }
+      self = newSelf;
+    }
+    break;
+  }
+}
+
 // Precondition - an entry for index must exist in the node
-void eraseChild(Node *self, uint8_t index, NodeAllocators *allocators) {
+void eraseChild(Node *self, uint8_t index, NodeAllocators *allocators,
+                Node *&root, ConflictSet::Impl *impl) {
   auto *child = getChildExists(self, index);
   switch (child->type) {
   case Type::Node0:
@@ -733,7 +986,7 @@ void eraseChild(Node *self, uint8_t index, NodeAllocators *allocators) {
     memmove(self16->index + nodeIndex, self16->index + nodeIndex + 1,
             sizeof(self16->index[0]) * (self->numChildren - (nodeIndex + 1)));
     memmove(self16->children + nodeIndex, self16->children + nodeIndex + 1,
-            sizeof(self16->children[0]) * // NOLINT
+            sizeof(self16->children[0]) *
                 (self->numChildren - (nodeIndex + 1)));
   } else if (self->type == Type::Node48) {
     auto *self48 = static_cast<Node48 *>(self);
@@ -757,7 +1010,10 @@ void eraseChild(Node *self, uint8_t index, NodeAllocators *allocators) {
   --self->numChildren;
   if (self->numChildren == 0 && !self->entryPresent &&
       self->parent != nullptr) {
-    eraseChild(self->parent, self->parentsIndex, allocators);
+    eraseChild(self->parent, self->parentsIndex, allocators, root, impl);
+  } else {
+    maybeDownsize(self, root, allocators, impl);
+    maybeDecreaseCapacity(self, root, allocators);
   }
 }
 
@@ -982,10 +1238,6 @@ struct SearchStepWise {
   }
 };
 
-namespace {
-std::string getSearchPathPrintable(Node *n);
-}
-
 // Logically this is the same as performing firstGeq and then checking against
 // point or range version according to cmp, but this version short circuits as
 // soon as it can prove that there's no conflict.
@@ -1582,7 +1834,7 @@ template <bool kBegin>
         auto *old = *self;
         int64_t oldMaxVersion = maxVersion(old, impl);
 
-        *self = allocators->node4.allocate(partialKeyIndex);
+        *self = allocators->node0.allocate(partialKeyIndex);
 
         memcpy((char *)*self + kNodeCopyBegin, (char *)old + kNodeCopyBegin,
                kNodeCopySize);
@@ -1601,6 +1853,19 @@ template <bool kBegin>
         memmove(old->partialKey(), old->partialKey() + partialKeyIndex + 1,
                 old->partialKeyLen - (partialKeyIndex + 1));
         old->partialKeyLen -= partialKeyIndex + 1;
+
+        [[maybe_unused]] int oldCap = old->partialKeyCapacity;
+        maybeDecreaseCapacity(old, getRoot(impl), allocators);
+
+#if DEBUG_VERBOSE && !defined(NDEBUG)
+        if (old->partialKeyCapacity < oldCap) {
+          fprintf(stderr,
+                  "%s: Length: %d, capacity: %d, numChildren: %d, oldCapacity: "
+                  "%d\n",
+                  getSearchPathPrintable(old).c_str(), old->partialKeyLen,
+                  old->partialKeyCapacity, old->numChildren, oldCap);
+        }
+#endif
       }
       key = key.subspan(partialKeyIndex, key.size() - partialKeyIndex);
 
@@ -1773,7 +2038,7 @@ void addWriteRange(Node *&root, int64_t oldestVersion,
     beginNode = nextLogical(beginNode);
     old->entryPresent = false;
     if (old->numChildren == 0 && old->parent != nullptr) {
-      eraseChild(old->parent, old->parentsIndex, allocators);
+      eraseChild(old->parent, old->parentsIndex, allocators, root, impl);
     }
   }
 }
@@ -1904,7 +2169,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         assert(n->entry.rangeVersion <= oldestVersion);
         prev->entryPresent = false;
         if (prev->numChildren == 0 && prev->parent != nullptr) {
-          eraseChild(prev->parent, prev->parentsIndex, &allocators);
+          eraseChild(prev->parent, prev->parentsIndex, &allocators, root, this);
         }
       }
 
@@ -1957,6 +2222,8 @@ int64_t &maxVersion(Node *n, ConflictSet::Impl *impl) {
   }
 }
 
+Node *&getRoot(ConflictSet::Impl *impl) { return impl->root; }
+
 // ==================== END IMPLEMENTATION ====================
 
 // GCOVR_EXCL_START
@@ -2228,6 +2495,48 @@ Iterator firstGeq(Node *n, std::string_view key) {
   return total;
 }
 
+[[maybe_unused]] void checkMemoryBoundInvariants(Node *node, bool &success) {
+  int minNumChildren;
+  switch (node->type) {
+  case Type::Node0:
+    minNumChildren = 0;
+    break;
+  case Type::Node4:
+    minNumChildren = kMinChildrenNode4;
+    break;
+  case Type::Node16:
+    minNumChildren = kMinChildrenNode16;
+    break;
+  case Type::Node48:
+    minNumChildren = kMinChildrenNode48;
+    break;
+  case Type::Node256:
+    minNumChildren = kMinChildrenNode256;
+    break;
+  }
+  if (node->numChildren < minNumChildren) {
+    fprintf(stderr,
+            "%s has %d children, which is less than the minimum required %d\n",
+            getSearchPathPrintable(node).c_str(), node->numChildren,
+            minNumChildren);
+    success = false;
+  }
+  if (node->numChildren > 0 &&
+      node->numChildren * node->partialKeyLen < node->partialKeyCapacity) {
+    fprintf(stderr,
+            "%s has %d children, partial key length %d, and partial key "
+            "capacity %d. It's required that nodes with children have children "
+            "* length >= capacity\n",
+            getSearchPathPrintable(node).c_str(), node->numChildren,
+            node->partialKeyLen, node->partialKeyCapacity);
+    success = false;
+  }
+  for (int i = getChildGeq(node, 0); i >= 0; i = getChildGeq(node, i + 1)) {
+    auto *child = getChildExists(node, i);
+    checkMemoryBoundInvariants(child, success);
+  }
+}
+
 bool checkCorrectness(Node *node, int64_t oldestVersion,
                       ConflictSet::Impl *impl) {
   bool success = true;
@@ -2235,6 +2544,7 @@ bool checkCorrectness(Node *node, int64_t oldestVersion,
   checkParentPointers(node, success);
   checkMaxVersion(node, node, oldestVersion, success, impl);
   checkEntriesExist(node, success);
+  checkMemoryBoundInvariants(node, success);
 
   return success;
 }