Update InternalVersionT::zero in addWrite call

There can be other conflict sets in the same thread

Bench.cpp

@@ -358,21 +358,6 @@ void benchWorstCaseForRadixRangeRead() {
   // }
 }
 
-void benchMetrics() {
-  ankerl::nanobench::Bench bench;
-  ConflictSet cs{0};
-
-  int count;
-  ConflictSet::MetricsV1 *m;
-  cs.getMetricsV1(&m, &count);
-  bench.batch(count);
-  bench.run("fetch metric", [&]() {
-    for (int i = 0; i < count; ++i) {
-      m[i].getValue();
-    }
-  });
-}
-
 void benchCreateAndDestroy() {
   ankerl::nanobench::Bench bench;
 
@@ -382,6 +367,5 @@ void benchCreateAndDestroy() {
 int main(void) {
   benchConflictSet();
   benchWorstCaseForRadixRangeRead();
-  benchMetrics();
   benchCreateAndDestroy();
 }

ConflictSet.cpp

@@ -1111,7 +1111,8 @@ Node *&getOrCreateChild(Node *&self, uint8_t index, WriteContext *tls) {
       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(self16->children[0]));
+              (self->numChildren - (i + 1)) *
+                  sizeof(self16->children[0])); // NOLINT
       memmove(self16->childMaxVersion + i + 1, self16->childMaxVersion + i,
               (self->numChildren - (i + 1)) *
                   sizeof(self16->childMaxVersion[0]));
@@ -2792,7 +2793,7 @@ scan16<true>(const InternalVersionT *vs, int begin, int end,
              InternalVersionT readVersion);
 template __attribute__((target("avx512f"))) bool
 checkMaxBetweenExclusive<true>(Node *n, int begin, int end,
-                               InternalVersionT readVersion);
+                               InternalVersionT readVersion, ReadContext *);
 template __attribute__((target("avx512f"))) bool
 checkRangeStartsWith<true>(Node *n, std::span<const uint8_t> key, int begin,
                            int end, InternalVersionT readVersion,
@@ -3152,23 +3153,16 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
       conflicts_accum += result[i] == Conflict;
       too_olds_accum += result[i] == TooOld;
     }
-    point_read_total.add(std::exchange(tls.point_read_accum, 0));
+    point_read_total.add(tls.point_read_accum);
-    prefix_read_total.add(std::exchange(tls.prefix_read_accum, 0));
+    prefix_read_total.add(tls.prefix_read_accum);
-    range_read_total.add(std::exchange(tls.range_read_accum, 0));
+    range_read_total.add(tls.range_read_accum);
-    range_read_node_scan_total.add(
+    range_read_node_scan_total.add(tls.range_read_node_scan_accum);
-        std::exchange(tls.range_read_node_scan_accum, 0));
+    point_read_short_circuit_total.add(tls.point_read_short_circuit_accum);
-    point_read_short_circuit_total.add(
+    prefix_read_short_circuit_total.add(tls.prefix_read_short_circuit_accum);
-        std::exchange(tls.point_read_short_circuit_accum, 0));
+    range_read_short_circuit_total.add(tls.range_read_short_circuit_accum);
-    prefix_read_short_circuit_total.add(
+    point_read_iterations_total.add(tls.point_read_iterations_accum);
-        std::exchange(tls.prefix_read_short_circuit_accum, 0));
+    prefix_read_iterations_total.add(tls.prefix_read_iterations_accum);
-    range_read_short_circuit_total.add(
+    range_read_iterations_total.add(tls.range_read_iterations_accum);
-        std::exchange(tls.range_read_short_circuit_accum, 0));
-    point_read_iterations_total.add(
-        std::exchange(tls.point_read_iterations_accum, 0));
-    prefix_read_iterations_total.add(
-        std::exchange(tls.prefix_read_iterations_accum, 0));
-    range_read_iterations_total.add(
-        std::exchange(tls.range_read_iterations_accum, 0));
     commits_total.add(commits_accum);
     conflicts_total.add(conflicts_accum);
     too_olds_total.add(too_olds_accum);
@@ -3183,6 +3177,11 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   }
 
   void addWrites(const WriteRange *writes, int count, int64_t writeVersion) {
+    // There could be other conflict sets in the same thread. We need
+    // InternalVersionT::zero to be correct for this conflict set for the
+    // lifetime of the current call frame.
+    InternalVersionT::zero = oldestVersion;
+
     assert(writeVersion >= newestVersionFullPrecision);
 
     if (writeVersion > newestVersionFullPrecision + kNominalVersionWindow) {
@@ -3201,6 +3200,8 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     }
 
     double write_byte_accum = 0;
+    int point_writes_accum = 0;
+    int range_writes_accum = 0;
     for (int i = 0; i < count; ++i) {
       const auto &w = writes[i];
       write_byte_accum += w.begin.len + w.end.len;
@@ -3208,15 +3209,19 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
       auto end = std::span<const uint8_t>(w.end.p, w.end.len);
       if (w.end.len > 0) {
         keyUpdates += 3;
+        ++range_writes_accum;
         addWriteRange(root, begin, end, InternalVersionT(writeVersion), &tls,
                       this);
       } else {
         keyUpdates += 2;
+        ++point_writes_accum;
         addPointWrite(root, begin, InternalVersionT(writeVersion), &tls, this);
       }
     }
 
     memory_bytes.set(totalBytes);
+    point_writes_total.add(point_writes_accum);
+    range_writes_total.add(range_writes_accum);
     nodes_allocated_total.add(std::exchange(tls.nodes_allocated_accum, 0));
     nodes_released_total.add(std::exchange(tls.nodes_released_accum, 0));
     entries_inserted_total.add(std::exchange(tls.entries_inserted_accum, 0));
@@ -3263,6 +3268,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     if (n == nullptr) {
       removalKey = {};
       oldestExtantVersion = oldestVersionAtGcBegin;
+      oldest_extant_version.set(oldestExtantVersion);
       oldestVersionAtGcBegin = oldestVersionFullPrecision;
 #if DEBUG_VERBOSE && !defined(NDEBUG)
       fprintf(stderr,
@@ -3311,6 +3317,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         newestVersionFullPrecision = oldestVersion;
     oldest_version.set(oldestVersionFullPrecision);
     newest_version.set(newestVersionFullPrecision);
+    oldest_extant_version.set(oldestExtantVersion);
 
     tls.~WriteContext();
     new (&tls) WriteContext();
@@ -3410,13 +3417,18 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   COUNTER(too_olds_total,
           "Total number of checks where the result is \"too old\"");
   COUNTER(check_bytes_total, "Total number of key bytes checked");
+  COUNTER(point_writes_total, "Total number of point writes");
+  COUNTER(range_writes_total,
+          "Total number of range writes (includes prefix writes)");
   GAUGE(memory_bytes, "Total number of bytes in use");
   COUNTER(nodes_allocated_total,
           "The total number of physical tree nodes allocated");
   COUNTER(nodes_released_total,
           "The total number of physical tree nodes released");
   COUNTER(insert_iterations_total,
-          "The total number of iterations of the main loop for insertion");
+          "The total number of iterations of the main loop for insertion. "
+          "Includes searches where the entry already existed, and so insertion "
+          "did not take place");
   COUNTER(entries_inserted_total,
           "The total number of entries inserted in the tree");
   COUNTER(entries_erased_total,
@@ -3428,6 +3440,9 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
   GAUGE(oldest_version,
         "The lowest version that doesn't result in \"TooOld\" for checks");
   GAUGE(newest_version, "The version of the most recent call to addWrites");
+  GAUGE(
+      oldest_extant_version,
+      "A lower bound on the lowest version associated with an existing entry");
 #if MEASURE_CHECK_CPU_TIME
   COUNTER(check_cpu_seconds_total,
           "Total cpu seconds spent in a call to check");

HashTable.cpp

@@ -98,6 +98,13 @@ void ConflictSet::setOldestVersion(int64_t oldestVersion) {
 
 int64_t ConflictSet::getBytes() const { return -1; }
 
+void ConflictSet::getMetricsV1(MetricsV1 **metrics, int *count) const {
+  *metrics = nullptr;
+  *count = 0;
+}
+
+double ConflictSet::MetricsV1::getValue() const { return 0; }
+
 ConflictSet::ConflictSet(int64_t oldestVersion)
     : impl(new(safe_malloc(sizeof(Impl))) Impl{oldestVersion}) {}
 

test_conflict_set.py

@@ -56,6 +56,20 @@ def test_conflict_set():
         assert cs.check(read(0, key), read(1, key)) == [Result.TOO_OLD, Result.COMMIT]
 
 
+def test_update_zero():
+    with DebugConflictSet() as cs1:
+        with DebugConflictSet() as cs2:
+            cs1.addWrites(2, write(b""))
+            cs1.setOldestVersion(2)
+            # "zero" is now 2
+
+            # make a Node48
+            for i in range(256 - 17, 256):
+                cs2.addWrites(int(1), write(bytes([i])))
+            # Scan until first point write
+            cs2.check(read(0, b"\x00", bytes([256 - 17])))
+
+
 def test_inner_full_words():
     with DebugConflictSet() as cs:
         cs.addWrites(1, write(b"\x3f\x61"), write(b"\x81\x61"))