From b1c1c39611a91b68839dcad44d83938e9d01ea62 Mon Sep 17 00:00:00 2001
From: Weaselbot <weaselbot@weaselab.dev>
Date: Tue, 10 Nov 2026 15:06:19 -0500
Subject: [PATCH] Report the version-window reset's releases and erasures in
 the metrics

`addWrites()` tears down the whole tree with `eraseTree()` when the version
window is exceeded, then calls `init()`, which reconstructs the `WriteContext`
and zeroes `accum`. Everything `eraseTree()` accounted for was discarded
before the metrics were reported, so `nodes_released_total` and
`entries_erased_total` never reflected the one path that frees an entire tree.

`init()` now carries `accum` across the reconstruction of `writeContext`, so
the accounting done before the reset still gets reported by `addWrites()`.

`eraseTree()` also incremented `accum.nodes_released` itself before calling
`release()`, which increments the same counter, double counting every node it
released. That was only invisible because `init()` zeroed `accum`. Drop the
redundant increment and keep the entry accounting, which `release()` does not
do.

Extend ShowMemoryTest.cpp to check that a version jump reports the nodes and
entries it erases.

Closes #100
---
 ConflictSet.cpp    |  9 ++++++++-
 ShowMemoryTest.cpp | 26 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/ConflictSet.cpp b/ConflictSet.cpp
index 3273dc2..e76fa71 100644
--- a/ConflictSet.cpp
+++ b/ConflictSet.cpp
@@ -2823,8 +2823,9 @@ void eraseTree(Node *root, WriteContext *writeContext) {
   while (toFree.size() > 0) {
     auto *n = toFree.back();
     toFree.pop_back();
+    // `release()` below accounts for the node itself, so only the entry needs
+    // to be accounted for here.
     writeContext->accum.entries_erased += n->entryPresent;
-    ++writeContext->accum.nodes_released;
 
 #if SHOW_MEMORY
     const int64_t searchPathLength = searchPathLengths.back();
@@ -5377,6 +5378,11 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   int64_t getBytes() const { return totalBytes; }
 
   void init(int64_t oldestVersion) {
+    // Metrics are reported by `addWrites()`, which calls this to reset the
+    // tree. Whatever was accounted for before the reset (in particular by
+    // `eraseTree()`) must survive the reconstruction of `writeContext` below.
+    const WriteContext::Accum accum = writeContext.accum;
+
     this->oldestVersion = InternalVersionT(oldestVersion);
     oldestVersionFullPrecision = oldestExtantVersion = oldestVersionAtGcBegin =
         newestVersionFullPrecision = oldestVersion;
@@ -5386,6 +5392,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
 
     writeContext.~WriteContext();
     new (&writeContext) WriteContext();
+    writeContext.accum = accum;
 
     // Leave removalBuffer as is
     removalKey = {};
diff --git a/ShowMemoryTest.cpp b/ShowMemoryTest.cpp
index 440cf7f..545cdba 100644
--- a/ShowMemoryTest.cpp
+++ b/ShowMemoryTest.cpp
@@ -8,6 +8,7 @@
 
 #include <cassert>
 #include <cstdint>
+#include <cstring>
 #include <string>
 
 using namespace weaselab;
@@ -30,6 +31,20 @@ ConflictSet::Result checkPoint(const ConflictSet &cs, const std::string &key,
   return result;
 }
 
+// Value of the metric named `name`.
+double metricValue(const ConflictSet &cs, const char *name) {
+  ConflictSet::MetricsV1 *metrics;
+  int count;
+  cs.getMetricsV1(&metrics, &count);
+  for (int i = 0; i < count; ++i) {
+    if (strcmp(metrics[i].name, name) == 0) {
+      return metrics[i].getValue();
+    }
+  }
+  assert(false);
+  return 0;
+}
+
 // Everything a ConflictSet allocated must have been accounted for and released
 // by the time it's destroyed.
 void checkFullyReleased() {
@@ -67,6 +82,12 @@ int main() {
     writePoint(cs, "abc", version);
     assert(checkPoint(cs, "abc", 0) == ConflictSet::TooOld);
     assert(checkPoint(cs, "abc", version) == ConflictSet::Commit);
+
+    // The erased tree held the "" entry in the root and the "abc" entry in a
+    // single child node. Erasing the tree must be reported by the metrics, and
+    // each node and entry must be counted exactly once.
+    assert(metricValue(cs, "entries_erased_total") == 2);
+    assert(metricValue(cs, "nodes_released_total") == 2);
   }
   checkFullyReleased();
 
@@ -80,6 +101,11 @@ int main() {
     writePoint(cs, "prefix/1", version);
     assert(checkPoint(cs, "prefix/1", version) == ConflictSet::Commit);
 
+    // Erasing a large tree is reported too: the 1000 entries written above
+    // plus the "" entry in the root, and at least one node per entry.
+    assert(metricValue(cs, "entries_erased_total") == 1001);
+    assert(metricValue(cs, "nodes_released_total") >= 1000);
+
     for (int i = 0; i < 1000; ++i) {
       writePoint(cs, "other/" + std::to_string(i), version + i + 1);
     }
-- 
2.43.0