Interface change! Add ConflictSet::getBytes

Closes #12

ConflictSet.cpp

@@ -216,6 +216,7 @@ struct Node {
   /* end section that's copied to the next node */
 
   uint8_t *partialKey();
+  size_t size() const;
 
   Type getType() const { return type; }
   int32_t getCapacity() const { return partialKeyCapacity; }
@@ -249,6 +250,8 @@ struct Node0 : Node {
 
   void copyChildrenAndKeyFrom(const Node0 &other);
   void copyChildrenAndKeyFrom(const struct Node3 &other);
+
+  size_t size() const { return sizeof(Node0) + getCapacity(); }
 };
 
 struct Node3 : Node {
@@ -262,6 +265,8 @@ struct Node3 : Node {
   void copyChildrenAndKeyFrom(const Node0 &other);
   void copyChildrenAndKeyFrom(const Node3 &other);
   void copyChildrenAndKeyFrom(const struct Node16 &other);
+
+  size_t size() const { return sizeof(Node3) + getCapacity(); }
 };
 
 struct Node16 : Node {
@@ -275,6 +280,8 @@ struct Node16 : Node {
   void copyChildrenAndKeyFrom(const Node3 &other);
   void copyChildrenAndKeyFrom(const Node16 &other);
   void copyChildrenAndKeyFrom(const struct Node48 &other);
+
+  size_t size() const { return sizeof(Node16) + getCapacity(); }
 };
 
 struct Node48 : Node {
@@ -290,6 +297,8 @@ struct Node48 : Node {
   void copyChildrenAndKeyFrom(const Node16 &other);
   void copyChildrenAndKeyFrom(const Node48 &other);
   void copyChildrenAndKeyFrom(const struct Node256 &other);
+
+  size_t size() const { return sizeof(Node48) + getCapacity(); }
 };
 
 struct Node256 : Node {
@@ -299,6 +308,8 @@ struct Node256 : Node {
   uint8_t *partialKey() { return (uint8_t *)(this + 1); }
   void copyChildrenAndKeyFrom(const Node48 &other);
   void copyChildrenAndKeyFrom(const Node256 &other);
+
+  size_t size() const { return sizeof(Node256) + getCapacity(); }
 };
 
 inline void Node0::copyChildrenAndKeyFrom(const Node0 &other) {
@@ -535,7 +546,7 @@ template <class T> struct BoundedFreeListAllocator {
       } else {
         // The intent is to filter out too-small nodes in the freelist
         removeNode(n);
-        safe_free(n);
+        safe_free(n, sizeof(T) + n->partialKeyCapacity);
       }
     }
 
@@ -550,7 +561,7 @@ template <class T> struct BoundedFreeListAllocator {
     static_assert(std::is_trivially_destructible_v<T>);
     if (freeListBytes >= kFreeListMaxMemory) {
       removeNode(p);
-      return safe_free(p);
+      return safe_free(p, sizeof(T) + p->partialKeyCapacity);
     }
     memcpy((void *)p, &freeList, sizeof(freeList));
     freeList = p;
@@ -561,10 +572,10 @@ template <class T> struct BoundedFreeListAllocator {
   ~BoundedFreeListAllocator() {
     for (void *iter = freeList; iter != nullptr;) {
       VALGRIND_MAKE_MEM_DEFINED(iter, sizeof(iter));
-      auto *tmp = iter;
+      auto *tmp = (T *)iter;
       memcpy(&iter, iter, sizeof(void *));
-      removeNode(((T *)tmp));
+      removeNode((tmp));
-      safe_free(tmp);
+      safe_free(tmp, sizeof(T) + tmp->partialKeyCapacity);
     }
   }
 
@@ -590,6 +601,23 @@ uint8_t *Node::partialKey() {
   }
 }
 
+size_t Node::size() const {
+  switch (type) {
+  case Type_Node0:
+    return ((Node0 *)this)->size();
+  case Type_Node3:
+    return ((Node3 *)this)->size();
+  case Type_Node16:
+    return ((Node16 *)this)->size();
+  case Type_Node48:
+    return ((Node48 *)this)->size();
+  case Type_Node256:
+    return ((Node256 *)this)->size();
+  default:                   // GCOVR_EXCL_LINE
+    __builtin_unreachable(); // GCOVR_EXCL_LINE
+  }
+}
+
 struct NodeAllocators {
   BoundedFreeListAllocator<Node0> node0;
   BoundedFreeListAllocator<Node3> node3;
@@ -1012,7 +1040,7 @@ void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
       allocators->node0.release(self0);
     } else {
       removeNode(self0);
-      safe_free(self0);
+      safe_free(self0, self0->size());
     }
     self = newSelf;
   } break;
@@ -1025,7 +1053,7 @@ void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
       allocators->node3.release(self3);
     } else {
       removeNode(self3);
-      safe_free(self3);
+      safe_free(self3, self3->size());
     }
     self = newSelf;
   } break;
@@ -1038,7 +1066,7 @@ void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
       allocators->node16.release(self16);
     } else {
       removeNode(self16);
-      safe_free(self16);
+      safe_free(self16, self16->size());
     }
     self = newSelf;
   } break;
@@ -1051,7 +1079,7 @@ void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
       allocators->node48.release(self48);
     } else {
       removeNode(self48);
-      safe_free(self48);
+      safe_free(self48, self48->size());
     }
     self = newSelf;
   } break;
@@ -1064,7 +1092,7 @@ void freeAndMakeCapacityAtLeast(Node *&self, int capacity,
       allocators->node256.release(self256);
     } else {
       removeNode(self256);
-      safe_free(self256);
+      safe_free(self256, self256->size());
     }
     self = newSelf;
   } break;
@@ -2178,7 +2206,7 @@ void destroyTree(Node *root) {
       assert(c != nullptr);
       toFree.push_back(c);
     }
-    safe_free(n);
+    safe_free(n, n->size());
   }
 }
 
@@ -2435,10 +2463,10 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     }
     if (n == nullptr) {
       removalKey = {};
-      return;
+    } else {
+      removalKeyArena = Arena();
+      removalKey = getSearchPath(removalKeyArena, n);
     }
-    removalKeyArena = Arena();
-    removalKey = getSearchPath(removalKeyArena, n);
   }
 
   explicit Impl(int64_t oldestVersion) : oldestVersion(oldestVersion) {
@@ -2467,6 +2495,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   Node *root;
   int64_t rootMaxVersion;
   int64_t oldestVersion;
+  int64_t totalBytes = 0;
 };
 
 // Precondition - an entry for index must exist in the node
@@ -2520,20 +2549,39 @@ void ConflictSet::check(const ReadRange *reads, Result *results,
 
 void ConflictSet::addWrites(const WriteRange *writes, int count,
                             int64_t writeVersion) {
-  return impl->addWrites(writes, count, writeVersion);
+  mallocBytesDelta = 0;
+  impl->addWrites(writes, count, writeVersion);
+  impl->totalBytes += mallocBytesDelta;
+#if SHOW_MEMORY
+  if (impl->totalBytes != mallocBytes) {
+    abort();
+  }
+#endif
 }
 
 void ConflictSet::setOldestVersion(int64_t oldestVersion) {
-  return impl->setOldestVersion(oldestVersion);
+  mallocBytesDelta = 0;
+  impl->setOldestVersion(oldestVersion);
+  impl->totalBytes += mallocBytesDelta;
+#if SHOW_MEMORY
+  if (impl->totalBytes != mallocBytes) {
+    abort();
+  }
+#endif
 }
 
+int64_t ConflictSet::getBytes() const { return impl->totalBytes; }
+
 ConflictSet::ConflictSet(int64_t oldestVersion)
-    : impl(new (safe_malloc(sizeof(Impl))) Impl{oldestVersion}) {}
+    : impl((mallocBytesDelta = 0,
+            new (safe_malloc(sizeof(Impl))) Impl{oldestVersion})) {
+  impl->totalBytes += mallocBytesDelta;
+}
 
 ConflictSet::~ConflictSet() {
   if (impl) {
     impl->~Impl();
-    safe_free(impl);
+    safe_free(impl, sizeof(*impl));
   }
 }
 
@@ -2573,7 +2621,12 @@ ConflictSet_create(int64_t oldestVersion) {
 __attribute__((__visibility__("default"))) void ConflictSet_destroy(void *cs) {
   using Impl = ConflictSet::Impl;
   ((Impl *)cs)->~Impl();
-  safe_free(cs);
+  safe_free(cs, sizeof(Impl));
+}
+__attribute__((__visibility__("default"))) int64_t
+ConflictSet_getBytes(void *cs) {
+  using Impl = ConflictSet::Impl;
+  return ((Impl *)cs)->totalBytes;
 }
 }
 

HashTable.cpp

@@ -96,13 +96,15 @@ void ConflictSet::setOldestVersion(int64_t oldestVersion) {
   return impl->setOldestVersion(oldestVersion);
 }
 
+int64_t ConflictSet::getBytes() const { return -1; }
+
 ConflictSet::ConflictSet(int64_t oldestVersion)
     : impl(new (safe_malloc(sizeof(Impl))) Impl{oldestVersion}) {}
 
 ConflictSet::~ConflictSet() {
   if (impl) {
     impl->~Impl();
-    safe_free(impl);
+    safe_free(impl, sizeof(Impl));
   }
 }
 
@@ -142,6 +144,11 @@ ConflictSet_create(int64_t oldestVersion) {
 __attribute__((__visibility__("default"))) void ConflictSet_destroy(void *cs) {
   using Impl = ConflictSet::Impl;
   ((Impl *)cs)->~Impl();
-  safe_free(cs);
+  safe_free(cs, sizeof(Impl));
+}
+__attribute__((__visibility__("default"))) int64_t
+ConflictSet_getBytes(void *cs) {
+  using Impl = ConflictSet::Impl;
+  return -1;
 }
 }

Internal.h

@@ -56,23 +56,24 @@ operator<=>(const std::span<const uint8_t> &lhs,
 #if SHOW_MEMORY
 inline int64_t mallocBytes = 0;
 inline int64_t peakMallocBytes = 0;
-constexpr auto kIntMallocHeaderSize = 16;
 #endif
 
+inline thread_local int64_t mallocBytesDelta = 0;
+
 // malloc that aborts on OOM and thus always returns a non-null pointer. Must be
 // paired with `safe_free`.
 __attribute__((always_inline)) inline void *safe_malloc(size_t s) {
+  mallocBytesDelta += s;
 #if SHOW_MEMORY
   mallocBytes += s;
   if (mallocBytes > peakMallocBytes) {
     peakMallocBytes = mallocBytes;
   }
-  void *p = malloc(s + kIntMallocHeaderSize);
+  void *p = malloc(s);
   if (p == nullptr) {
     abort();
   }
-  memcpy(p, &s, sizeof(s));
+  return p;
-  return (char *)p + kIntMallocHeaderSize;
 #else
   void *p = malloc(s);
   if (p == nullptr) {
@@ -86,12 +87,11 @@ __attribute__((always_inline)) inline void *safe_malloc(size_t s) {
 //
 // There's nothing safer about this than free. Only called safe_free for
 // symmetry with safe_malloc.
-__attribute__((always_inline)) inline void safe_free(void *p) {
+__attribute__((always_inline)) inline void safe_free(void *p, size_t s) {
+  mallocBytesDelta -= s;
 #if SHOW_MEMORY
-  size_t s;
-  memcpy(&s, (char *)p - kIntMallocHeaderSize, sizeof(s));
   mallocBytes -= s;
-  free((char *)p - kIntMallocHeaderSize);
+  free(p);
 #else
   free(p);
 #endif
@@ -179,7 +179,7 @@ inline Arena::Arena(int initialSize) : impl(nullptr) {
 inline void onDestroy(Arena::ArenaImpl *impl) {
   while (impl) {
     auto *prev = impl->prev;
-    safe_free(impl);
+    safe_free(impl, sizeof(Arena::ArenaImpl) + impl->capacity);
     impl = prev;
   }
 }

RealDataBench.cpp

@@ -40,6 +40,8 @@ int main(int argc, const char **argv) {
   int64_t version = 0;
   double timer = 0;
 
+  int64_t peakMemory = 0;
+
   for (int i = 1; i < argc; ++i) {
     int fd = open(argv[i], O_RDONLY);
     struct stat st;
@@ -109,6 +111,10 @@ int main(int argc, const char **argv) {
         write = {};
         reads.clear();
 
+        if (cs.getBytes() > peakMemory) {
+          peakMemory = cs.getBytes();
+        }
+
         timer = now();
         cs.setOldestVersion(version - 10000);
         gcTime += now() - timer;
@@ -118,8 +124,9 @@ int main(int argc, const char **argv) {
     close(fd);
   }
 
-  printf(
+  printf("Check: %g seconds, %g MB/s, Add: %g seconds, %g MB/s, Gc ratio: "
-      "Check: %g seconds, %g MB/s, Add: %g seconds, %g MB/s, Gc ratio: %g%%\n",
+         "%g%%, Peak idle memory: %g\n",
-      checkTime, checkBytes / checkTime * 1e-6, addTime,
+         checkTime, checkBytes / checkTime * 1e-6, addTime,
-      addBytes / addTime * 1e-6, gcTime / (gcTime + addTime) * 1e2);
+         addBytes / addTime * 1e-6, gcTime / (gcTime + addTime) * 1e2,
+         double(peakMemory));
 }

SkipList.cpp

@@ -149,7 +149,7 @@ private:
       setMaxVersion(level, v);
     }
 
-    void destroy() { safe_free(this); }
+    void destroy() { safe_free(this, getNodeSize()); }
 
   private:
     int getNodeSize() const {
@@ -627,6 +627,8 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         removalArena, {finger.getValue().data(), finger.getValue().size()});
   }
 
+  int64_t totalBytes = 0;
+
 private:
   int64_t keyUpdates = 10;
   Arena removalArena;
@@ -637,25 +639,44 @@ private:
 
 void ConflictSet::check(const ReadRange *reads, Result *results,
                         int count) const {
-  return impl->check(reads, results, count);
+  impl->check(reads, results, count);
 }
 
 void ConflictSet::addWrites(const WriteRange *writes, int count,
                             int64_t writeVersion) {
-  return impl->addWrites(writes, count, writeVersion);
+  mallocBytesDelta = 0;
+  impl->addWrites(writes, count, writeVersion);
+  impl->totalBytes += mallocBytesDelta;
+#if SHOW_MEMORY
+  if (impl->totalBytes != mallocBytes) {
+    abort();
+  }
+#endif
 }
 
 void ConflictSet::setOldestVersion(int64_t oldestVersion) {
-  return impl->setOldestVersion(oldestVersion);
+  mallocBytesDelta = 0;
+  impl->setOldestVersion(oldestVersion);
+  impl->totalBytes += mallocBytesDelta;
+#if SHOW_MEMORY
+  if (impl->totalBytes != mallocBytes) {
+    abort();
+  }
+#endif
 }
 
+int64_t ConflictSet::getBytes() const { return impl->totalBytes; }
+
 ConflictSet::ConflictSet(int64_t oldestVersion)
-    : impl(new (safe_malloc(sizeof(Impl))) Impl{oldestVersion}) {}
+    : impl((mallocBytesDelta = 0,
+            new (safe_malloc(sizeof(Impl))) Impl{oldestVersion})) {
+  impl->totalBytes += mallocBytesDelta;
+}
 
 ConflictSet::~ConflictSet() {
   if (impl) {
     impl->~Impl();
-    safe_free(impl);
+    safe_free(impl, sizeof(Impl));
   }
 }
 
@@ -695,7 +716,12 @@ ConflictSet_create(int64_t oldestVersion) {
 __attribute__((__visibility__("default"))) void ConflictSet_destroy(void *cs) {
   using Impl = ConflictSet::Impl;
   ((Impl *)cs)->~Impl();
-  safe_free(cs);
+  safe_free(cs, sizeof(Impl));
+}
+__attribute__((__visibility__("default"))) int64_t
+ConflictSet_getBytes(void *cs) {
+  using Impl = ConflictSet::Impl;
+  return ((Impl *)cs)->totalBytes;
 }
 }
 

include/ConflictSet.h

@@ -84,6 +84,9 @@ struct __attribute__((__visibility__("default"))) ConflictSet {
 
   ~ConflictSet();
 
+  /** Returns the total bytes in use by this ConflictSet */
+  int64_t getBytes() const;
+
 #if __cplusplus > 199711L
   ConflictSet(ConflictSet &&) noexcept;
   ConflictSet &operator=(ConflictSet &&) noexcept;
@@ -169,4 +172,7 @@ ConflictSet *ConflictSet_create(int64_t oldestVersion);
 
 void ConflictSet_destroy(ConflictSet *cs);
 
+/** Returns the total bytes in use by this ConflictSet */
+int64_t ConflictSet_getBytes(ConflictSet *cs);
+
 #endif