Fix clangd warnings

Previously it was reading some of the keys from unversioned storage.

Also add some more benchmarks

Bench.cpp

@@ -3,6 +3,34 @@
 
 #include <nanobench.h>
 
+void iterBench(const Facade &facade, int64_t version,
+               const std::string &context) {
+  ankerl::nanobench::Bench bench;
+  bench.minEpochIterations(10000);
+
+  const auto begin = facade.versioned.begin(version);
+  const auto end = facade.versioned.end(version);
+  auto iter = begin;
+
+  bench.run("*iter (" + context + ")", [&] { bench.doNotOptimizeAway(*iter); });
+
+  iter = begin;
+  bench.run("++iter (" + context + ")", [&] {
+    ++iter;
+    if (iter == end) {
+      iter = begin;
+    }
+  });
+
+  iter = end;
+  bench.run("--iter (" + context + ")", [&] {
+    --iter;
+    if (iter == begin) {
+      iter = end;
+    }
+  });
+}
+
 void monotonicallyIncreasing() {
   constexpr int kWindow = 1000;
   ankerl::nanobench::Bench bench;
@@ -25,27 +53,7 @@ void monotonicallyIncreasing() {
   });
 
   const auto v = facade.getVersion() - kWindow / 2;
-  const auto begin = facade.versioned.begin(v);
-  const auto end = facade.versioned.end(v);
-  auto iter = begin;
-
-  bench.run("*iter", [&] { bench.doNotOptimizeAway(*iter); });
-
-  iter = begin;
-  bench.run("++iter", [&] {
-    ++iter;
-    if (iter == end) {
-      iter = begin;
-    }
-  });
-
-  iter = end;
-  bench.run("--iter", [&] {
-    --iter;
-    if (iter == begin) {
-      iter = end;
-    }
-  });
+  iterBench(facade, v, "monotonically increasing");
 
   bench.run("begin", [&] { facade.versioned.begin(v); });
 
@@ -56,6 +64,27 @@ void monotonicallyIncreasing() {
   });
 
   bench.run("end", [&] { facade.versioned.end(v); });
+
+  {
+    ankerl::nanobench::Bench bench;
+    bench.batch(kWindow);
+
+    bench.run("Facade monotonically-increasing read forward", [&]() {
+      if (facade.viewAt(facade.getVersion())
+              .rangeRead(String(), String({0xff}), kWindow, false)
+              .size() != kWindow) {
+        abort();
+      }
+    });
+
+    bench.run("Facade monotonically-increasing read reverse", [&]() {
+      if (facade.viewAt(facade.getVersion())
+              .rangeRead(String(), String({0xff}), kWindow, true)
+              .size() != kWindow) {
+        abort();
+      }
+    });
+  }
 }
 
 void bulkFirstGeq() {
@@ -103,7 +132,66 @@ void bulkFirstGeq() {
             [&] { versionedMap.firstGeq(keys, iterators, kNumQueries); });
 }
 
+void facadeVersionedOnlyRead() {
+  Facade facade{0};
+  constexpr int kNumKeys = 1000;
+  ankerl::nanobench::Bench bench;
+  bench.batch(kNumKeys);
+
+  const int64_t beginBytes = __builtin_bswap64(0);
+  const int64_t endBytes = __builtin_bswap64(kNumKeys);
+  const String begin{(const uint8_t *)&beginBytes, 8};
+  const String end{(const uint8_t *)&endBytes, 8};
+
+  // Insert clear in entire range, so that all reads are served from versioned,
+  // logically and hopefully physically.
+  {
+    weaselab::VersionedMap::Mutation mutations[] = {
+        {begin.data(), int(begin.size()), end.data(), int(end.size()),
+         weaselab::VersionedMap::Clear},
+    };
+    facade.addMutations(mutations, sizeof(mutations) / sizeof(mutations[0]),
+                        facade.getVersion() + 1);
+  }
+
+  Arena arena;
+  weaselab::VersionedMap::Mutation *mutations =
+      new (arena) weaselab::VersionedMap::Mutation[kNumKeys];
+  for (int i = 0; i < kNumKeys; ++i) {
+    const int64_t k = __builtin_bswap64(i);
+    uint8_t *buf = new (arena) uint8_t[8];
+    memcpy(buf, &k, 8);
+    mutations[i] = {buf, 8, buf, 8, weaselab::VersionedMap::Set};
+  }
+
+  // Add keys
+  facade.addMutations(mutations, kNumKeys, facade.getVersion() + 1);
+
+  // Populate the unversioned map
+  facade.addMutations(mutations, kNumKeys, facade.getVersion() + 1);
+  facade.setOldestVersion(facade.getVersion() - 1, /*force*/ true);
+
+  iterBench(facade, facade.getVersion(), "adjacent sets/clears");
+
+  bench.run("Facade versioned-only read forward", [&]() {
+    if (facade.viewAt(facade.getVersion())
+            .rangeRead(begin, end, kNumKeys, false)
+            .size() != kNumKeys) {
+      abort();
+    }
+  });
+
+  bench.run("Facade versioned-only read reverse", [&]() {
+    if (facade.viewAt(facade.getVersion())
+            .rangeRead(begin, end, kNumKeys, true)
+            .size() != kNumKeys) {
+      abort();
+    }
+  });
+}
+
 int main() {
   monotonicallyIncreasing();
   bulkFirstGeq();
+  facadeVersionedOnlyRead();
 }

Facade.h

@@ -53,12 +53,13 @@ struct Facade {
         do {
           --iter;
           auto m = *iter;
-          const auto mBegin =
-              weaselab::VersionedMap::Key{m.param1, m.param1Len};
-          const auto mEnd =
+          auto mBegin = weaselab::VersionedMap::Key{m.param1, m.param1Len};
+          auto mEnd =
               m.type == weaselab::VersionedMap::Set || m.param2Len == 0
                   ? weaselab::VersionedMap::Key{m.param1, m.param1Len + 1}
                   : weaselab::VersionedMap::Key{m.param2, m.param2Len};
+
+          // Read from unversioned down to mEnd
           if (unversionedIter != beginIter) {
             --unversionedIter;
             for (; unversionedIter->first >= mEnd && limit > 0;
@@ -73,6 +74,12 @@ struct Facade {
           if (limit == 0) {
             return result;
           }
+
+          bool pointMutation =
+              m.type == weaselab::VersionedMap::Set || m.param2Len == 0;
+
+          // Read from versioned until non-adjacent
+        readVersionedReverse:
           switch (m.type) {
           case weaselab::VersionedMap::Set: {
             result.emplace_back(String(mBegin.p, mBegin.len),
@@ -85,7 +92,31 @@ struct Facade {
           case weaselab::VersionedMap::Clear:
             break;
           }
-          if (m.type == weaselab::VersionedMap::Set || m.param2Len == 0) {
+
+          if (iter != endIter) {
+            --iter;
+            auto tmpM = *iter;
+            const auto tmpMBegin =
+                weaselab::VersionedMap::Key{tmpM.param1, tmpM.param1Len};
+            const auto tmpMEnd =
+                tmpM.type == weaselab::VersionedMap::Set || tmpM.param2Len == 0
+                    ? weaselab::VersionedMap::Key{tmpM.param1,
+                                                  tmpM.param1Len + 1}
+                    : weaselab::VersionedMap::Key{tmpM.param2, tmpM.param2Len};
+            if (tmpMEnd >= mBegin) {
+              // Adjacent with last (temporally) mutation
+              mBegin = tmpMBegin;
+              mEnd = tmpMEnd;
+              m = tmpM;
+              pointMutation = false;
+              goto readVersionedReverse;
+            } else {
+              ++iter;
+            }
+          }
+
+          // Advance unversioned iter
+          if (pointMutation) {
             if (unversionedIter != facade->unversioned.end() &&
                 unversionedIter->first < mBegin) {
               ++unversionedIter;
@@ -112,16 +143,17 @@ struct Facade {
         return result;
       } else {
         auto unversionedIter = facade->unversioned.lower_bound(begin);
-        for (auto iter = versionedIter[0]; iter != versionedIter[1]; ++iter) {
+        for (auto iter = versionedIter[0]; iter != versionedIter[1];) {
           auto m = *iter;
-          const auto mBegin =
-              weaselab::VersionedMap::Key{m.param1, m.param1Len};
-          const auto mEnd =
+          auto mBegin = weaselab::VersionedMap::Key{m.param1, m.param1Len};
+          auto mEnd =
               m.type == weaselab::VersionedMap::Set || m.param2Len == 0
                   ? weaselab::VersionedMap::Key{m.param1, m.param1Len + 1}
                   : weaselab::VersionedMap::Key{m.param2, m.param2Len};
+
+          // Read from unversioned up to mBegin
           for (; unversionedIter != facade->unversioned.end() &&
-                 (unversionedIter->first <=> mBegin) < 0 && limit > 0;) {
+                 unversionedIter->first < mBegin && limit > 0;) {
             result.push_back(*unversionedIter);
             --limit;
             ++unversionedIter;
@@ -129,6 +161,12 @@ struct Facade {
           if (limit == 0) {
             return result;
           }
+
+          bool pointMutation =
+              m.type == weaselab::VersionedMap::Set || m.param2Len == 0;
+
+          // Read from versioned until non-adjacent
+        readVersionedForward:
           switch (m.type) {
           case weaselab::VersionedMap::Set: {
             result.emplace_back(String(mBegin.p, mBegin.len),
@@ -141,19 +179,42 @@ struct Facade {
           case weaselab::VersionedMap::Clear:
             break;
           }
-          if (m.type == weaselab::VersionedMap::Set || m.param2Len == 0) {
+          ++iter;
+          if (iter != versionedIter[1]) {
+            auto tmpM = *iter;
+            if (weaselab::VersionedMap::Key{tmpM.param1, tmpM.param1Len} <=
+                mEnd) {
+              // Adjacent with last mutation
+              mBegin = weaselab::VersionedMap::Key{tmpM.param1, tmpM.param1Len};
+              mEnd = tmpM.type == weaselab::VersionedMap::Set ||
+                             tmpM.param2Len == 0
+                         ? weaselab::VersionedMap::Key{tmpM.param1,
+                                                       tmpM.param1Len + 1}
+                         : weaselab::VersionedMap::Key{tmpM.param2,
+                                                       tmpM.param2Len};
+              m = tmpM;
+              pointMutation = false;
+              goto readVersionedForward;
+            }
+          }
+
+          // Advance unversioned iter
+          if (pointMutation) {
+            // Point mutation
             if (unversionedIter != facade->unversioned.end() &&
-                (unversionedIter->first <=> mBegin) == 0) {
+                unversionedIter->first <= mBegin) {
               ++unversionedIter;
             }
             assert(unversionedIter ==
                    facade->unversioned.lower_bound(String(mEnd.p, mEnd.len)));
           } else {
+            // Range mutation
             unversionedIter =
                 facade->unversioned.lower_bound(String(mEnd.p, mEnd.len));
           }
         }
 
+        // Finish reading from unversioned
         for (; unversionedIter != facade->unversioned.end() &&
                unversionedIter->first < end && limit > 0;
              ++unversionedIter) {
@@ -178,14 +239,16 @@ struct Facade {
     versioned.addMutations(mutations, numMutations, version);
   }
 
-  void setOldestVersion(int64_t version) {
+  void setOldestVersion(int64_t version, bool force = false) {
     // Don't scan and apply mutations every time setOldestVersion is called.
 
     oldestVersion = version;
-    if (version >= nextPurgeVersion) {
-      nextPurgeVersion = versioned.getVersion();
-    } else {
-      return;
+    if (!force) {
+      if (version >= nextPurgeVersion) {
+        nextPurgeVersion = versioned.getVersion();
+      } else {
+        return;
+      }
     }
 
     for (auto iter = versioned.begin(version), end = versioned.end(version);

FacadeTest.cpp

@@ -3,11 +3,11 @@
 #include "VersionedMap.h"
 #include <inttypes.h>
 
-inline weaselab::VersionedMap::Key operator"" _k(const char *str, size_t size) {
+inline weaselab::VersionedMap::Key operator""_k(const char *str, size_t size) {
   return {reinterpret_cast<const uint8_t *>(str), int(size)};
 }
 
-inline String operator"" _s(const char *str, size_t size) {
+inline String operator""_s(const char *str, size_t size) {
   return String{reinterpret_cast<const uint8_t *>(str), size};
 }
 
@@ -92,4 +92,4 @@ int main() {
       printMutation(*iter);
     }
   }
-}
+}

VersionedMap.cpp

@@ -1220,20 +1220,20 @@ VersionedMap::Iterator::operator*() const {
 
 void materializeMutations(VersionedMap::Iterator::Impl *impl,
                           const Entry *prev) {
-  if (prev == nullptr) {
-    Finger copy;
-    impl->finger.copyTo(copy);
-    impl->map->move<std::memory_order_acquire, false>(copy, impl->version);
-    if (copy.searchPathSize() > 0) {
-      prev = impl->map->mm.base[copy.backNode()].entry;
-    } else {
-      assert(!impl->map->mm.base[impl->finger.backNode()].entry->clearTo());
-    }
-  }
-
   const auto &entry = *impl->map->mm.base[impl->finger.backNode()].entry;
   impl->mutationCount = 0;
   if (entry.clearTo()) {
+    if (prev == nullptr) {
+      Finger copy;
+      impl->finger.copyTo(copy);
+      impl->map->move<std::memory_order_acquire, false>(copy, impl->version);
+      if (copy.searchPathSize() > 0) {
+        prev = impl->map->mm.base[copy.backNode()].entry;
+      } else {
+        assert(!impl->map->mm.base[impl->finger.backNode()].entry->clearTo());
+      }
+    }
+
     impl->mutations[impl->mutationCount++] = {
         prev->getKey(),
         entry.getKey(),