Interface change! addWrites now takes a single write version

Bench.cpp

@@ -2,7 +2,6 @@
 #include "Internal.h"
 #include <cstdint>
 #include <cstring>
-#include <string>
 
 #if SHOW_MEMORY
 void showMemory(const ConflictSet &cs);
@@ -78,10 +77,9 @@ void benchConflictSet() {
       w.begin.len = r.begin.len;
       w.end.p = r.end.p;
       w.end.len = 0;
-      w.writeVersion = version + 1;
       writes.push_back(w);
     }
-    cs.addWrites(writes.data(), writes.size());
+    cs.addWrites(writes.data(), writes.size(), version + 1);
     ++version;
   }
 
@@ -112,11 +110,10 @@ void benchConflictSet() {
       w.begin.len = begin.size();
       w.end.p = end.data();
       w.end.len = end.size();
-      w.writeVersion = version + 1;
       writes.push_back(w);
     }
+    cs.addWrites(writes.data(), kOpsPerTx, version + 1);
     ++version;
-    cs.addWrites(writes.data(), kOpsPerTx);
   }
 
   {
@@ -189,16 +186,12 @@ void benchConflictSet() {
 
     while (version < kMvccWindow) {
       auto v = ++version;
-      writes[0].writeVersion = v;
+      cs.addWrites(writes.data(), 1, v);
-      cs.addWrites(writes.data(), 1);
     }
 
     bench.run("point writes", [&]() {
       auto v = ++version;
-      for (auto &w : writes) {
+      cs.addWrites(writes.data(), writes.size(), v);
-        w.writeVersion = v;
-      }
-      cs.addWrites(writes.data(), writes.size());
       cs.setOldestVersion(version - kMvccWindow);
     });
   }
@@ -219,10 +212,7 @@ void benchConflictSet() {
 
     bench.run("prefix writes", [&]() {
       auto v = ++version;
-      for (auto &w : writes) {
+      cs.addWrites(writes.data(), writes.size(), v);
-        w.writeVersion = v;
-      }
-      cs.addWrites(writes.data(), writes.size());
       cs.setOldestVersion(version - kMvccWindow);
     });
   }
@@ -243,10 +233,7 @@ void benchConflictSet() {
 
     bench.run("range writes", [&]() {
       auto v = ++version;
-      for (auto &w : writes) {
+      cs.addWrites(writes.data(), writes.size(), v);
-        w.writeVersion = v;
-      }
-      cs.addWrites(writes.data(), writes.size());
       cs.setOldestVersion(version - kMvccWindow);
     });
   }
@@ -263,12 +250,11 @@ void benchConflictSet() {
       auto x = __builtin_bswap64(version);
       memcpy(b, &x, 8);
 
-      w.writeVersion = v;
       w.begin.p = b;
       w.begin.len = 8;
       w.end.len = 0;
       w.end.p = b;
-      cs.addWrites(&w, 1);
+      cs.addWrites(&w, 1, v);
       cs.setOldestVersion(version - kMvccWindow);
     });
   }

ConflictSet.cpp

@@ -1847,18 +1847,18 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     }
   }
 
-  void addWrites(const WriteRange *writes, int count) {
+  void addWrites(const WriteRange *writes, int count, int64_t writeVersion) {
     for (int i = 0; i < count; ++i) {
       const auto &w = writes[i];
       auto begin = std::span<const uint8_t>(w.begin.p, w.begin.len);
       auto end = std::span<const uint8_t>(w.end.p, w.end.len);
       if (w.end.len > 0) {
         keyUpdates += 3;
-        addWriteRange(root, oldestVersion, begin, end, w.writeVersion,
+        addWriteRange(root, oldestVersion, begin, end, writeVersion,
                       &allocators, this);
       } else {
         keyUpdates += 2;
-        addPointWrite(root, oldestVersion, begin, w.writeVersion, &allocators,
+        addPointWrite(root, oldestVersion, begin, writeVersion, &allocators,
                       this);
       }
     }
@@ -1954,8 +1954,9 @@ void ConflictSet::check(const ReadRange *reads, Result *results,
   return impl->check(reads, results, count);
 }
 
-void ConflictSet::addWrites(const WriteRange *writes, int count) {
+void ConflictSet::addWrites(const WriteRange *writes, int count,
-  return impl->addWrites(writes, count);
+                            int64_t writeVersion) {
+  return impl->addWrites(writes, count, writeVersion);
 }
 
 void ConflictSet::setOldestVersion(int64_t oldestVersion) {
@@ -2009,9 +2010,9 @@ ConflictSet_check(void *cs, const ConflictSet_ReadRange *reads,
   ((ConflictSet::Impl *)cs)->check(reads, results, count);
 }
 __attribute__((__visibility__("default"))) void
-ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes,
+ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes, int count,
-                      int count) {
+                      int64_t writeVersion) {
-  ((ConflictSet::Impl *)cs)->addWrites(writes, count);
+  ((ConflictSet::Impl *)cs)->addWrites(writes, count, writeVersion);
 }
 __attribute__((__visibility__("default"))) void
 ConflictSet_setOldestVersion(void *cs, int64_t oldestVersion) {
@@ -2239,29 +2240,19 @@ void printTree() {
   ConflictSet::Impl cs{writeVersion};
   ReferenceImpl refImpl{writeVersion};
   Arena arena;
-  constexpr int kNumKeys = 4;
+  ConflictSet::WriteRange write;
-  auto *write = new (arena) ConflictSet::WriteRange[kNumKeys];
+  write.begin = "and"_s;
-  write[0].begin = "and"_s;
+  write.end = "ant"_s;
-  write[0].end = "ant"_s;
+  cs.addWrites(&write, 1, ++writeVersion);
-  write[0].writeVersion = 1;
+  write.begin = "any"_s;
-
+  write.end = ""_s;
-  write[1].begin = "any"_s;
+  cs.addWrites(&write, 1, ++writeVersion);
-  write[1].end.len = 0;
+  write.begin = "are"_s;
-  write[1].writeVersion = 2;
+  write.end = ""_s;
-
+  cs.addWrites(&write, 1, ++writeVersion);
-  write[2].begin = "are"_s;
+  write.begin = "art"_s;
-  write[2].end.len = 0;
+  write.end = ""_s;
-  write[2].writeVersion = 3;
+  cs.addWrites(&write, 1, ++writeVersion);
-
-  write[3].begin = "art"_s;
-  write[3].end.len = 0;
-  write[3].writeVersion = 4;
-
-  cs.addWrites(write, kNumKeys);
-  for (int i = 0; i < kNumKeys; ++i) {
-    write[i].writeVersion = ++writeVersion;
-  }
-  cs.addWrites(write, kNumKeys);
   debugPrintDot(stdout, cs.root, &cs);
 }
 

HashTable.cpp

@@ -36,11 +36,13 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     }
   }
 
-  void addWrites(const ConflictSet::WriteRange *writes, int count) {
+  void addWrites(const ConflictSet::WriteRange *writes, int count,
+                 int64_t writeVersion) {
     for (int i = 0; i < count; ++i) {
       auto &max = map[std::string((const char *)writes[i].begin.p,
                                   writes[i].begin.len)];
-      max = std::max(max, writes[i].writeVersion);
+      assert(writeVersion >= max);
+      max = writeVersion;
       keyUpdates += 2;
     }
   }
@@ -83,8 +85,9 @@ void ConflictSet::check(const ReadRange *reads, Result *results,
   return impl->check(reads, results, count);
 }
 
-void ConflictSet::addWrites(const WriteRange *writes, int count) {
+void ConflictSet::addWrites(const WriteRange *writes, int count,
-  return impl->addWrites(writes, count);
+                            int64_t writeVersion) {
+  return impl->addWrites(writes, count, writeVersion);
 }
 
 void ConflictSet::setOldestVersion(int64_t oldestVersion) {
@@ -121,9 +124,9 @@ ConflictSet_check(void *cs, const ConflictSet_ReadRange *reads,
   ((ConflictSet::Impl *)cs)->check(reads, results, count);
 }
 __attribute__((__visibility__("default"))) void
-ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes,
+ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes, int count,
-                      int count) {
+                      int64_t writeVersion) {
-  ((ConflictSet::Impl *)cs)->addWrites(writes, count);
+  ((ConflictSet::Impl *)cs)->addWrites(writes, count, writeVersion);
 }
 __attribute__((__visibility__("default"))) void
 ConflictSet_setOldestVersion(void *cs, int64_t oldestVersion) {

Internal.h

@@ -404,7 +404,8 @@ struct ReferenceImpl {
                                                      : ConflictSet::Commit;
     }
   }
-  void addWrites(const ConflictSet::WriteRange *writes, int count) {
+  void addWrites(const ConflictSet::WriteRange *writes, int count,
+                 int64_t writeVersion) {
     for (int i = 0; i < count; ++i) {
       auto begin =
           std::string((const char *)writes[i].begin.p, writes[i].begin.len);
@@ -412,7 +413,6 @@ struct ReferenceImpl {
           writes[i].end.len == 0
               ? begin + std::string("\x00", 1)
               : std::string((const char *)writes[i].end.p, writes[i].end.len);
-      auto writeVersion = writes[i].writeVersion;
       auto prevVersion = (--writeVersionMap.upper_bound(end))->second;
       for (auto iter = writeVersionMap.lower_bound(begin),
                 endIter = writeVersionMap.lower_bound(end);
@@ -547,7 +547,6 @@ template <class ConflictSetImpl> struct TestDriver {
           ++iter;
           --rangesRemaining;
         }
-        writes[i].writeVersion = v;
 #if DEBUG_VERBOSE && !defined(NDEBUG)
         if (writes[i].end.len == 0) {
           fprintf(stderr, "Write: {%s} -> %d\n",
@@ -565,10 +564,10 @@ template <class ConflictSetImpl> struct TestDriver {
       assert(i == numPointWrites + numRangeWrites);
 
       CALLGRIND_START_INSTRUMENTATION;
-      cs.addWrites(writes, numPointWrites + numRangeWrites);
+      cs.addWrites(writes, numPointWrites + numRangeWrites, v);
       CALLGRIND_STOP_INSTRUMENTATION;
 
-      refImpl.addWrites(writes, numPointWrites + numRangeWrites);
+      refImpl.addWrites(writes, numPointWrites + numRangeWrites, v);
 
       oldestVersion = std::max<int64_t>(writeVersion - arbitrary.bounded(10),
                                         oldestVersion);

RealDataBench.cpp

@@ -96,7 +96,6 @@ int main(int argc, const char **argv) {
         // Add unconditionally so that the load doesn't actually depend on the
         // conflict rate
         ConflictSet::WriteRange w;
-        w.writeVersion = ++version;
         w.begin.p = (const uint8_t *)write.data();
         w.begin.len = write.size();
         w.end.len = 0;
@@ -104,7 +103,7 @@ int main(int argc, const char **argv) {
         addBytes += write.size();
 
         timer = now();
-        cs.addWrites(&w, 1);
+        cs.addWrites(&w, 1, ++version);
         addTime += now() - timer;
 
         write = {};

SkipList.cpp

@@ -286,7 +286,7 @@ public:
   void swap(SkipList &other) { std::swap(header, other.header); }
 
   void addConflictRanges(const Finger *fingers, int rangeCount,
-                         Version *version) {
+                         Version version) {
     for (int r = rangeCount - 1; r >= 0; r--) {
       const Finger &startF = fingers[r * 2];
       const Finger &endF = fingers[r * 2 + 1];
@@ -295,7 +295,7 @@ public:
         insert(endF, endF.finger[0]->getMaxVersion(0));
 
       remove(startF, endF);
-      insert(startF, version[r]);
+      insert(startF, version);
     }
   }
 
@@ -588,7 +588,8 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     }
   }
 
-  void addWrites(const ConflictSet::WriteRange *writes, int count) {
+  void addWrites(const ConflictSet::WriteRange *writes, int count,
+                 int64_t writeVersion) {
     Arena arena;
     const int stringCount = count * 2;
 
@@ -606,11 +607,10 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
         values[i * 2 + 1] = w.end.len > 0
                                 ? StringRef{w.end.p, size_t(w.end.len)}
                                 : keyAfter(arena, values[i * 2]);
-        writeVersions[i] = w.writeVersion;
         keyUpdates += 2;
       }
       skipList.find(values, fingers, temp, ss);
-      skipList.addConflictRanges(fingers, ss / 2, writeVersions);
+      skipList.addConflictRanges(fingers, ss / 2, writeVersion);
       ss = stripeSize;
     }
   }
@@ -640,8 +640,9 @@ void ConflictSet::check(const ReadRange *reads, Result *results,
   return impl->check(reads, results, count);
 }
 
-void ConflictSet::addWrites(const WriteRange *writes, int count) {
+void ConflictSet::addWrites(const WriteRange *writes, int count,
-  return impl->addWrites(writes, count);
+                            int64_t writeVersion) {
+  return impl->addWrites(writes, count, writeVersion);
 }
 
 void ConflictSet::setOldestVersion(int64_t oldestVersion) {
@@ -678,9 +679,9 @@ ConflictSet_check(void *cs, const ConflictSet_ReadRange *reads,
   ((ConflictSet::Impl *)cs)->check(reads, results, count);
 }
 __attribute__((__visibility__("default"))) void
-ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes,
+ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes, int count,
-                      int count) {
+                      int64_t writeVersion) {
-  ((ConflictSet::Impl *)cs)->addWrites(writes, count);
+  ((ConflictSet::Impl *)cs)->addWrites(writes, count, writeVersion);
 }
 __attribute__((__visibility__("default"))) void
 ConflictSet_setOldestVersion(void *cs, int64_t oldestVersion) {

conflict_set_c_api_test.c

@@ -10,8 +10,7 @@ int main(void) {
   w.begin.p = (const uint8_t *)"0000";
   w.begin.len = 4;
   w.end.len = 0;
-  w.writeVersion = 1;
+  ConflictSet_addWrites(cs, &w, 1, 1);
-  ConflictSet_addWrites(cs, &w, 1);
   r.begin.p = (const uint8_t *)"0000";
   r.begin.len = 4;
   r.end.len = 0;

conflict_set_cxx_api_test.cpp

@@ -8,8 +8,7 @@ int main(void) {
   w.begin.p = (const uint8_t *)"0000";
   w.begin.len = 4;
   w.end.len = 0;
-  w.writeVersion = 1;
+  cs.addWrites(&w, 1, 1);
-  cs.addWrites(&w, 1);
   ConflictSet::Result result;
   ConflictSet::ReadRange r;
   r.begin.p = (const uint8_t *)"0000";

include/ConflictSet.h

@@ -44,22 +44,21 @@ struct __attribute__((__visibility__("default"))) ConflictSet {
     Key end;
     int64_t readVersion;
   };
-  /** Denotes a set of keys to be considered written at `writeVersion` */
+  /** Denotes a set of keys to be written at `writeVersion` */
   struct WriteRange {
     Key begin;
     /** `end` having length 0 denotes that this range is the single key {begin}.
      * Otherwise this denotes the range [begin, end) */
     Key end;
-    /** Write version must be greater than all write versions in all previous
-     * calls to `addWrites` */
-    int64_t writeVersion;
   };
   /** The result of checking reads[i] is written in results[i]. */
   void check(const ReadRange *reads, Result *results, int count) const;
   /** `writes` must be sorted ascending, and must not have adjacent or
    * overlapping ranges. Reads intersecting writes where readVersion <
-   * writeVersion will result in `Conflict` (or `TooOld`, eventually) */
+   * `writeVersion` will result in `Conflict` (or `TooOld`, eventually).
-  void addWrites(const WriteRange *writes, int count);
+   * `writeVersion must be greater than all write versions in all previous
+   * calls to `addWrites` */
+  void addWrites(const WriteRange *writes, int count, int64_t writeVersion);
   /** Reads where readVersion < oldestVersion will result in `TooOld`. Must be
    * greater than any previous oldestVersion. */
   void setOldestVersion(int64_t oldestVersion);
@@ -110,25 +109,25 @@ typedef struct {
   ConflictSet_Key end;
   int64_t readVersion;
 } ConflictSet_ReadRange;
-/** Denotes a set of keys to be considered written at `writeVersion` */
+/** Denotes a set of keys to be written at `writeVersion` */
 typedef struct {
   ConflictSet_Key begin;
   /** `end` having length 0 denotes that this range is the single key {begin}.
    * Otherwise this denotes the range [begin, end) */
   ConflictSet_Key end;
-  /** Write version must be greater than all write versions in all previous
-   * calls to `addWrites` */
-  int64_t writeVersion;
 } ConflictSet_WriteRange;
 
 /** The result of checking reads[i] is written in results[i]. */
 void ConflictSet_check(ConflictSet *cs, const ConflictSet_ReadRange *reads,
                        ConflictSet_Result *results, int count);
-/** `writes` must be sorted ascending, and must not have adjacent or overlapping
+/** `writes` must be sorted ascending, and must not have adjacent or
- * ranges. Reads intersecting writes where readVersion < writeVersion will
+ * overlapping ranges. Reads intersecting writes where readVersion <
- * result in `Conflict` (or `TooOld`, eventually) */
+ * `writeVersion` will result in `Conflict` (or `TooOld`, eventually).
+ * `writeVersion must be greater than all write versions in all previous
+ * calls to `addWrites` */
 void ConflictSet_addWrites(ConflictSet *cs,
-                           const ConflictSet_WriteRange *writes, int count);
+                           const ConflictSet_WriteRange *writes, int count,
+                           int64_t writeVersion);
 /** Reads where readVersion < oldestVersion will result in `TooOld`. Must be
  * greater than any previous oldestVersion. */
 void ConflictSet_setOldestVersion(ConflictSet *cs, int64_t oldestVersion);

symbols.txt

@@ -4,7 +4,7 @@ ConflictSet_create
 ConflictSet_destroy
 ConflictSet_setOldestVersion
 _ZN11ConflictSet16setOldestVersionEl
-_ZN11ConflictSet9addWritesEPKNS_10WriteRangeEi
+_ZN11ConflictSet9addWritesEPKNS_10WriteRangeEil
 _ZN11ConflictSetaSEOS_
 _ZN11ConflictSetC1El
 _ZN11ConflictSetC1EOS_