From b5dbf4a0492e5658c42ab9ecdd472c1bbb8b7e44 Mon Sep 17 00:00:00 2001
From: Andrew Noyes <andrew@weaselab.dev>
Date: Thu, 23 May 2024 17:27:55 -0700
Subject: [PATCH] Avoid copies in Facade

---
 Facade.h               | 36 +++++++++++++++++++++---------------
 KeyCompare.h           | 25 +++++++++++++++++++++++++
 include/VersionedMap.h |  2 ++
 3 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/Facade.h b/Facade.h
index aaf1b00..2c3722f 100644
--- a/Facade.h
+++ b/Facade.h
@@ -1,14 +1,12 @@
 #pragma once
 
+#include "KeyCompare.h"
 #include "VersionedMap.h"
 
 #include <assert.h>
 #include <map>
-#include <string>
 #include <vector>
 
-using String = std::basic_string<uint8_t>;
-
 struct Facade {
 
   explicit Facade(int64_t version)
@@ -58,11 +56,12 @@ struct Facade {
         do {
           --iter;
           auto m = *iter;
-          const auto mBegin = String(m.param1, m.param1Len);
+          const auto mBegin =
+              weaselab::VersionedMap::Key{m.param1, m.param1Len};
           const auto mEnd =
               m.type == weaselab::VersionedMap::Set || m.param2Len == 0
-                  ? String(m.param1, m.param1Len) + zero
-                  : String(m.param2, m.param2Len);
+                  ? weaselab::VersionedMap::Key{m.param1, m.param1Len + 1}
+                  : weaselab::VersionedMap::Key{m.param2, m.param2Len};
           if (unversionedIter != beginIter) {
             --unversionedIter;
             for (; unversionedIter->first >= mEnd && limit > 0;
@@ -79,7 +78,8 @@ struct Facade {
           }
           switch (m.type) {
           case weaselab::VersionedMap::Set: {
-            result.emplace_back(mBegin, String(m.param2, m.param2Len));
+            result.emplace_back(String(mBegin.p, mBegin.len),
+                                String(m.param2, m.param2Len));
             --limit;
             if (limit == 0) {
               return result;
@@ -93,9 +93,11 @@ struct Facade {
                 unversionedIter->first < mBegin) {
               ++unversionedIter;
             }
-            assert(unversionedIter == facade->unversioned.lower_bound(mBegin));
+            assert(unversionedIter == facade->unversioned.lower_bound(
+                                          String(mBegin.p, mBegin.len)));
           } else {
-            unversionedIter = facade->unversioned.lower_bound(mBegin);
+            unversionedIter =
+                facade->unversioned.lower_bound(String(mBegin.p, mBegin.len));
           }
         } while (iter != endIter);
 
@@ -115,11 +117,12 @@ struct Facade {
         auto unversionedIter = facade->unversioned.lower_bound(begin);
         for (auto iter = versionedIter[0]; iter != versionedIter[1]; ++iter) {
           auto m = *iter;
-          const auto mBegin = String(m.param1, m.param1Len);
+          const auto mBegin =
+              weaselab::VersionedMap::Key{m.param1, m.param1Len};
           const auto mEnd =
               m.type == weaselab::VersionedMap::Set || m.param2Len == 0
-                  ? String(m.param1, m.param1Len) + zero
-                  : String(m.param2, m.param2Len);
+                  ? weaselab::VersionedMap::Key{m.param1, m.param1Len + 1}
+                  : weaselab::VersionedMap::Key{m.param2, m.param2Len};
           auto c = unversionedIter->first <=> mBegin;
           for (; unversionedIter != facade->unversioned.end() && c < 0 &&
                  limit > 0;) {
@@ -133,7 +136,8 @@ struct Facade {
           }
           switch (m.type) {
           case weaselab::VersionedMap::Set: {
-            result.emplace_back(mBegin, String(m.param2, m.param2Len));
+            result.emplace_back(String(mBegin.p, mBegin.len),
+                                String(m.param2, m.param2Len));
             --limit;
             if (limit == 0) {
               return result;
@@ -146,9 +150,11 @@ struct Facade {
             if (unversionedIter != facade->unversioned.end() && c == 0) {
               ++unversionedIter;
             }
-            assert(unversionedIter == facade->unversioned.lower_bound(mEnd));
+            assert(unversionedIter ==
+                   facade->unversioned.lower_bound(String(mEnd.p, mEnd.len)));
           } else {
-            unversionedIter = facade->unversioned.lower_bound(mEnd);
+            unversionedIter =
+                facade->unversioned.lower_bound(String(mEnd.p, mEnd.len));
           }
         }
 
diff --git a/KeyCompare.h b/KeyCompare.h
index 74478ba..9bd374b 100644
--- a/KeyCompare.h
+++ b/KeyCompare.h
@@ -5,6 +5,9 @@
 #include <algorithm>
 #include <string.h>
 
+#include <string>
+using String = std::basic_string<uint8_t>;
+
 inline auto operator<=>(const weaselab::VersionedMap::Key &lhs,
                         const weaselab::VersionedMap::Key &rhs) {
   int cl = std::min(lhs.len, rhs.len);
@@ -16,3 +19,25 @@ inline auto operator<=>(const weaselab::VersionedMap::Key &lhs,
   }
   return lhs.len <=> rhs.len;
 }
+
+inline auto operator<=>(const String &lhs,
+                        const weaselab::VersionedMap::Key &rhs) {
+  int cl = std::min<int>(lhs.size(), rhs.len);
+  if (cl > 0) {
+    int c = memcmp(lhs.data(), rhs.p, cl);
+    if (c != 0) {
+      return c <=> 0;
+    }
+  }
+  return int(lhs.size()) <=> rhs.len;
+}
+
+inline auto operator==(const weaselab::VersionedMap::Key &lhs,
+                       const weaselab::VersionedMap::Key &rhs) {
+  return (lhs <=> rhs) == 0;
+}
+
+inline auto operator!=(const weaselab::VersionedMap::Key &lhs,
+                       const weaselab::VersionedMap::Key &rhs) {
+  return (lhs <=> rhs) != 0;
+}
diff --git a/include/VersionedMap.h b/include/VersionedMap.h
index d3eed4d..cba501f 100644
--- a/include/VersionedMap.h
+++ b/include/VersionedMap.h
@@ -107,6 +107,8 @@ struct __attribute__((__visibility__("default"))) VersionedMap {
     Iterator &operator=(Iterator &&) noexcept;
 
     struct VersionedMutation {
+      /** param1 is guaranteed to have an addressable null byte at
+       * param1[param1Len] */
       const uint8_t *param1;
       const uint8_t *param2;
       int param1Len;