Restore size limitation for copying whole inline buffer in small_vector

Summary:
D43401609, derived from [PR #1934](#1934), allows the move constructor to use `memcpy` to relocate the inline storage for relocatable types, but it copies the whole storage disregarding the previous limitation on size for trivial types (which are a subset of relocatable types). Furthermore, the same optimization is not applied to the move-assignment operator.

This diff restores the size limitation (using a precise copy when relocating inline buffers that exceed it) and applies the same logic to move-assignment.

Also, since now we can assume C++17 and thus `if constexpr`, I removed the unnecessary overload of `copyInlineTrivial()`, and I made the names less ambiguous.

We might also benefit from special-casing relocatable types on reallocation in `makeSizeInternal()`, but that can be done separately.

Reviewed By: Gownta

Differential Revision: D53965673

fbshipit-source-id: 2939c03ada3b19d4fadfa11621a3e2f3afc5ccb7

folly/small_vector.h

@@ -28,6 +28,7 @@
 #include <cstdlib>
 #include <cstring>
 #include <iterator>
+#include <memory>
 #include <stdexcept>
 #include <type_traits>
 #include <utility>
@@ -526,9 +527,11 @@ class small_vector
   small_vector(const std::allocator<Value>&) {}
 
   small_vector(small_vector const& o) {
-    if (kShouldCopyInlineTrivial && !o.isExtern()) {
-      copyInlineTrivial<Value>(o);
-      return;
+    if constexpr (kShouldCopyWholeInlineStorageTrivial) {
+      if (!o.isExtern()) {
+        copyWholeInlineStorageTrivial(o);
+        return;
+      }
     }
 
     auto n = o.size();
@@ -551,12 +554,11 @@ class small_vector
         this->u.setCapacity(o.u.getCapacity());
       }
     } else {
-      if constexpr (IsRelocatable<Value>::value) {
-        // Copy the entire inline storage, instead of just size() values, to
-        // make the loop fixed-size and unrollable.
-        std::memcpy(u.buffer(), o.u.buffer(), MaxInline * kSizeOfValue);
-        this->setSize(o.size());
+      if constexpr (kShouldCopyWholeInlineStorageTrivial) {
+        copyWholeInlineStorageTrivial(o);
         o.resetSizePolicy();
+      } else if constexpr (IsRelocatable<Value>::value) {
+        moveInlineStorageRelocatable(std::move(o));
       } else {
         auto n = o.size();
         std::uninitialized_copy(
@@ -593,9 +595,13 @@ class small_vector
 
   small_vector& operator=(small_vector const& o) {
     if (FOLLY_LIKELY(this != &o)) {
-      if (kShouldCopyInlineTrivial && !this->isExtern() && !o.isExtern()) {
-        copyInlineTrivial<Value>(o);
-      } else if (o.size() < capacity()) {
+      if constexpr (kShouldCopyWholeInlineStorageTrivial) {
+        if (!this->isExtern() && !o.isExtern()) {
+          copyWholeInlineStorageTrivial(o);
+          return *this;
+        }
+      }
+      if (o.size() < capacity()) {
         const size_t oSize = o.size();
         detail::partiallyUninitializedCopy(o.begin(), oSize, begin(), size());
         this->setSize(oSize);
@@ -616,9 +622,12 @@ class small_vector
       }
 
       if (!o.isExtern()) {
-        if (kShouldCopyInlineTrivial) {
-          copyInlineTrivial<Value>(o);
+        if constexpr (kShouldCopyWholeInlineStorageTrivial) {
+          copyWholeInlineStorageTrivial(o);
           o.resetSizePolicy();
+        } else if constexpr (IsRelocatable<Value>::value) {
+          std::destroy_n(u.buffer(), size());
+          moveInlineStorageRelocatable(std::move(o));
         } else {
           const size_t oSize = o.size();
           detail::partiallyUninitializedCopy(
@@ -1049,19 +1058,22 @@ class small_vector
     this->setSize(sz);
   }
 
-  template <class T>
-  typename std::enable_if<is_trivially_copyable_v<T>>::type copyInlineTrivial(
-      small_vector const& o) {
-    // Copy the entire inline storage, instead of just size() values, to make
-    // the loop fixed-size and unrollable.
+  void copyWholeInlineStorageTrivial(small_vector const& o) {
+    static_assert(is_trivially_copyable_v<Value>);
     std::copy(o.u.buffer(), o.u.buffer() + MaxInline, u.buffer());
     this->setSize(o.size());
   }
 
-  template <class T>
-  typename std::enable_if<!is_trivially_copyable_v<T>>::type copyInlineTrivial(
-      small_vector const&) {
-    assume_unreachable();
+  void moveInlineStorageRelocatable(small_vector&& o) {
+    static_assert(IsRelocatable<Value>::value);
+    const auto n = o.size();
+    if constexpr (kMayCopyWholeInlineStorage) {
+      std::memcpy(u.buffer(), o.u.buffer(), MaxInline * kSizeOfValue);
+    } else {
+      std::memcpy(u.buffer(), o.u.buffer(), n * kSizeOfValue);
+    }
+    this->setSize(n);
+    o.resetSizePolicy();
   }
 
   void reset() {
@@ -1340,13 +1352,16 @@ class small_vector
       InlineStorageDataType,
       char>::type InlineStorageType;
 
-  // If the values are trivially copyable and the storage is small enough, copy
-  // it entirely. Limit is half of a cache line, to minimize probability of
-  // introducing a cache miss.
-  static constexpr bool kShouldCopyInlineTrivial =
-      is_trivially_copyable_v<Value> &&
+  // If the storage is small enough, it is usually faster to copy it entirely,
+  // instead of just size() values, to make the loop fixed-size and
+  // unrollable. Limit is half of a cache line, to minimize probability of
+  // crossing a cache line and thus introducing an unnecessary cache miss.
+  static constexpr bool kMayCopyWholeInlineStorage =
       sizeof(InlineStorageType) <= hardware_constructive_interference_size / 2;
 
+  static constexpr bool kShouldCopyWholeInlineStorageTrivial =
+      is_trivially_copyable_v<Value> && kMayCopyWholeInlineStorage;
+
   static bool constexpr kHasInlineCapacity = !BaseType::kAlwaysUseHeap &&
       sizeof(HeapPtrWithCapacity) < sizeof(InlineStorageType);