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 #include +#include #include 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); }