Improve performance of fbstring copy and move ctors on small strings

Summary: The move constructor of `fbstring` generates a large amount of code, preventing it to be inlined, and it is inefficient for small strings. This diff fixes both problems.

Reviewed By: @yfeldblum

Differential Revision: D2505762

diff --git a/folly/FBString.h b/folly/FBString.h
index 3af49b672f8272cf643a642553ea570e4775a81a..a9618aa4938b83ac4314fbb230de1a870e4118f5 100644 (file)
--- a/folly/FBString.h
+++ b/folly/FBString.h
@@ -290,16 +290,7 @@ protected:
   static_assert(
       kIsLittleEndian || kIsBigEndian, "unable to identify endianness");
 public:
-  fbstring_core() noexcept {
-    // Only initialize the tag, will set the MSBs (i.e. the small
-    // string size) to zero too
-    ml_.capacity_ = kIsLittleEndian
-      ? maxSmallSize << (8 * (sizeof(size_t) - sizeof(Char)))
-      : ml_.capacity_ = maxSmallSize << 2;
-    // or: setSmallSize(0);
-    writeTerminator();
-    assert(category() == Category::isSmall && size() == 0);
-  }
+  fbstring_core() noexcept { reset(); }
 
   fbstring_core(const fbstring_core & rhs) {
     assert(&rhs != this);
@@ -311,18 +302,12 @@ public:
           "fbstring layout failure");
       static_assert(offsetof(MediumLarge, capacity_) == 2 * sizeof(ml_.data_),
           "fbstring layout failure");
-      const size_t size = rhs.smallSize();
-      if (size == 0) {
-        ml_.capacity_ = rhs.ml_.capacity_;
-        writeTerminator();
-      } else {
-        // Just write the whole thing, don't look at details. In
-        // particular we need to copy capacity anyway because we want
-        // to set the size (don't forget that the last character,
-        // which stores a short string's length, is shared with the
-        // ml_.capacity field).
-        ml_ = rhs.ml_;
-      }
+      // Just write the whole thing, don't look at details. In
+      // particular we need to copy capacity anyway because we want
+      // to set the size (don't forget that the last character,
+      // which stores a short string's length, is shared with the
+      // ml_.capacity field).
+      ml_ = rhs.ml_;
       assert(category() == Category::isSmall && this->size() == rhs.size());
     } else if (rhs.category() == Category::isLarge) {
       // Large strings are just refcounted
@@ -350,14 +335,11 @@ public:
   }
 
   fbstring_core(fbstring_core&& goner) noexcept {
-    if (goner.category() == Category::isSmall) {
-      // Just copy, leave the goner in peace
-      new(this) fbstring_core(goner.small_, goner.smallSize());
-    } else {
-      // Take goner's guts
-      ml_ = goner.ml_;
+    // Take goner's guts
+    ml_ = goner.ml_;
+    if (goner.category() != Category::isSmall) {
       // Clean goner's carcass
-      goner.setSmallSize(0);
+      goner.reset();
     }
   }
 
@@ -463,7 +445,7 @@ public:
     } else {
       // No need for the memory
       free(data);
-      setSmallSize(0);
+      reset();
     }
   }
 
@@ -723,6 +705,19 @@ private:
   // Disabled
   fbstring_core & operator=(const fbstring_core & rhs);
 
+  // Equivalent to setSmallSize(0), but with specialized
+  // writeTerminator which doesn't re-check the category after
+  // capacity_ is overwritten.
+  void reset() {
+    // Only initialize the tag, will set the MSBs (i.e. the small
+    // string size) to zero too.
+    ml_.capacity_ = kIsLittleEndian
+      ? maxSmallSize << (8 * (sizeof(size_t) - sizeof(Char)))
+      : maxSmallSize << 2;
+    small_[0] = '\0';
+    assert(category() == Category::isSmall && size() == 0);
+  }
+
   struct RefCounted {
     std::atomic<size_t> refCount_;
     Char data_[1];