Frozen: fix performance of FixedSizeString view comparison

swolchok · facebook-github-bot · commit 80e61b384868 · 2024-02-12T09:40:22.000-08:00
Summary:
I observed clang generating a byte-by-byte comparison for
FixedSizeString, despite the length being known to be 8 and therefore
comparable with a simple integer comparison.

Reviewed By: iahs

Differential Revision: D53557089

fbshipit-source-id: 9b6a5bfd06a3130567f1ed22e7532a2178f44e07
diff --git a/thrift/lib/cpp2/frozen/FrozenFixedSizeString-inl.h b/thrift/lib/cpp2/frozen/FrozenFixedSizeString-inl.h
@@ -65,10 +65,20 @@ struct FixedSizeStringLayout : public LayoutBase {
     os << folly::demangle(type.name());
   }
 
-  using View = folly::ByteRange;
-  View view(ViewPosition self) const {
-    return folly::ByteRange{self.start, T::kFixedSize};
-  }
+  struct View : public folly::ByteRange {
+   public:
+    using folly::ByteRange::ByteRange;
+
+    View(folly::ByteRange bytes) : folly::ByteRange(bytes) {}
+
+    bool operator==(View rhs) {
+      return memcmp(this->data(), rhs.data(), T::kFixedSize) == 0;
+    }
+
+    using folly::ByteRange::toString;
+  };
+
+  View view(ViewPosition self) const { return View(self.start, T::kFixedSize); }
 
   static size_t hash(const T& value) {
     return FixedSizeStringHash<T::kFixedSize, T>::hash(value);
diff --git a/thrift/lib/cpp2/frozen/test/FrozenBench.cpp b/thrift/lib/cpp2/frozen/test/FrozenBench.cpp
@@ -190,11 +190,19 @@ struct OwnedKey<folly::ByteRange> {
   using type = FixedSizeString<8>;
 };
 
+template <>
+struct OwnedKey<
+    typename detail::FixedSizeStringLayout<FixedSizeString<8>>::View> {
+  using type = FixedSizeString<8>;
+};
+
 template <
     typename M,
     typename T,
     std::enable_if_t<
-        !std::is_same<typename M::key_type, folly::ByteRange>::value,
+        !std::is_same<
+            typename M::key_type,
+            detail::FixedSizeStringLayout<FixedSizeString<8>>::View>::value,
         bool> = true>
 typename M::const_iterator mapFind(const M& map, const T& key) {
   return map.find(key);
@@ -206,7 +214,9 @@ template <
     typename M,
     typename T,
     std::enable_if_t<
-        std::is_same<typename M::key_type, folly::ByteRange>::value,
+        std::is_same<
+            typename M::key_type,
+            detail::FixedSizeStringLayout<FixedSizeString<8>>::View>::value,
         bool> = true>
 typename M::const_iterator mapFind(const M& map, const T& key) {
   static_assert(std::is_same<T, FixedSizeString<8>>::value);
