replace the use of boost::variant with a simple union

Summary:
`boost::variant` is an expensive template, and `boost/variant.hpp` is an expensive header. In the one place it is used in futures/detail/Core.h (a commonly-included header), it can be trivially replaced with a `union`, so do so.

As a drive-by, I mark as `noexcept` those members of `KeepAliveOrDeferred` that can be done so unconditionally.

Reviewed By: yfeldblum, kirkshoop, ot

Differential Revision: D22460453

fbshipit-source-id: 5a01f873058273d1a20265507d87796450cc008b

folly/futures/detail/Core.cpp

@@ -14,7 +14,10 @@
  * limitations under the License.
  */
 
+#include <new>
+
 #include <folly/futures/detail/Core.h>
+#include <folly/lang/Assume.h>
 
 namespace folly {
 namespace futures {
@@ -26,96 +29,110 @@ void UniqueDeleter::operator()(DeferredExecutor* ptr) {
   }
 }
 
-KeepAliveOrDeferred::KeepAliveOrDeferred() = default;
+KeepAliveOrDeferred::KeepAliveOrDeferred() noexcept : state_(State::Deferred) {
+  ::new (&deferred_) DW{};
+}
 
-KeepAliveOrDeferred::KeepAliveOrDeferred(Executor::KeepAlive<> ka)
-    : storage_{std::move(ka)} {
-  DCHECK(!isDeferred());
+KeepAliveOrDeferred::KeepAliveOrDeferred(KA ka) noexcept
+    : state_(State::KeepAlive) {
+  ::new (&keepAlive_) KA{std::move(ka)};
 }
 
-KeepAliveOrDeferred::KeepAliveOrDeferred(DeferredWrapper deferred)
-    : storage_{std::move(deferred)} {}
+KeepAliveOrDeferred::KeepAliveOrDeferred(DW deferred) noexcept
+    : state_(State::Deferred) {
+  ::new (&deferred_) DW{std::move(deferred)};
+}
 
 KeepAliveOrDeferred::KeepAliveOrDeferred(KeepAliveOrDeferred&& other) noexcept
-    : storage_{std::move(other.storage_)} {}
+    : state_(other.state_) {
+  switch (state_) {
+    case State::Deferred:
+      ::new (&deferred_) DW{std::move(other.deferred_)};
+      break;
+    case State::KeepAlive:
+      ::new (&keepAlive_) KA{std::move(other.keepAlive_)};
+      break;
+  }
+}
 
-KeepAliveOrDeferred::~KeepAliveOrDeferred() = default;
+KeepAliveOrDeferred::~KeepAliveOrDeferred() {
+  switch (state_) {
+    case State::Deferred:
+      deferred_.~DW();
+      break;
+    case State::KeepAlive:
+      keepAlive_.~KA();
+      break;
+  }
+}
 
 KeepAliveOrDeferred& KeepAliveOrDeferred::operator=(
-    KeepAliveOrDeferred&& other) {
-  storage_ = std::move(other.storage_);
+    KeepAliveOrDeferred&& other) noexcept {
+  // This is safe to do because KeepAliveOrDeferred is nothrow
+  // move-constructible.
+  this->~KeepAliveOrDeferred();
+  ::new (this) KeepAliveOrDeferred{std::move(other)};
   return *this;
 }
 
-DeferredExecutor* KeepAliveOrDeferred::getDeferredExecutor() const {
-  if (!isDeferred()) {
-    return nullptr;
+DeferredExecutor* KeepAliveOrDeferred::getDeferredExecutor() const noexcept {
+  switch (state_) {
+    case State::Deferred:
+      return deferred_.get();
+    case State::KeepAlive:
+      return nullptr;
   }
-  return asDeferred().get();
+  assume_unreachable();
 }
 
-Executor* KeepAliveOrDeferred::getKeepAliveExecutor() const {
-  if (isDeferred()) {
-    return nullptr;
+Executor* KeepAliveOrDeferred::getKeepAliveExecutor() const noexcept {
+  switch (state_) {
+    case State::Deferred:
+      return nullptr;
+    case State::KeepAlive:
+      return keepAlive_.get();
   }
-  return asKeepAlive().get();
+  assume_unreachable();
 }
 
-Executor::KeepAlive<> KeepAliveOrDeferred::stealKeepAlive() && {
-  if (isDeferred()) {
-    return Executor::KeepAlive<>{};
+KeepAliveOrDeferred::KA KeepAliveOrDeferred::stealKeepAlive() && noexcept {
+  switch (state_) {
+    case State::Deferred:
+      return KA{};
+    case State::KeepAlive:
+      return std::move(keepAlive_);
   }
-  return std::move(asKeepAlive());
+  assume_unreachable();
 }
 
-std::unique_ptr<DeferredExecutor, UniqueDeleter>
-KeepAliveOrDeferred::stealDeferred() && {
-  if (!isDeferred()) {
-    return std::unique_ptr<DeferredExecutor, UniqueDeleter>{};
+KeepAliveOrDeferred::DW KeepAliveOrDeferred::stealDeferred() && noexcept {
+  switch (state_) {
+    case State::Deferred:
+      return std::move(deferred_);
+    case State::KeepAlive:
+      return DW{};
   }
-  return std::move(asDeferred());
-}
-
-bool KeepAliveOrDeferred::isDeferred() const {
-  return boost::get<DeferredWrapper>(&storage_) != nullptr;
-}
-
-bool KeepAliveOrDeferred::isKeepAlive() const {
-  return !isDeferred();
+  assume_unreachable();
 }
 
 KeepAliveOrDeferred KeepAliveOrDeferred::copy() const {
-  if (isDeferred()) {
-    if (auto def = getDeferredExecutor()) {
-      return KeepAliveOrDeferred{def->copy()};
-    } else {
-      return KeepAliveOrDeferred{};
-    }
-  } else {
-    return KeepAliveOrDeferred{asKeepAlive()};
+  switch (state_) {
+    case State::Deferred:
+      if (auto def = getDeferredExecutor()) {
+        return KeepAliveOrDeferred{def->copy()};
+      } else {
+        return KeepAliveOrDeferred{};
+      }
+    case State::KeepAlive:
+      return KeepAliveOrDeferred{keepAlive_};
   }
+  assume_unreachable();
 }
 
-/* explicit */ KeepAliveOrDeferred::operator bool() const {
+/* explicit */ KeepAliveOrDeferred::operator bool() const noexcept {
   return getDeferredExecutor() || getKeepAliveExecutor();
 }
 
-Executor::KeepAlive<>& KeepAliveOrDeferred::asKeepAlive() {
-  return boost::get<Executor::KeepAlive<>>(storage_);
-}
-
-const Executor::KeepAlive<>& KeepAliveOrDeferred::asKeepAlive() const {
-  return boost::get<Executor::KeepAlive<>>(storage_);
-}
-
-DeferredWrapper& KeepAliveOrDeferred::asDeferred() {
-  return boost::get<DeferredWrapper>(storage_);
-}
-
-const DeferredWrapper& KeepAliveOrDeferred::asDeferred() const {
-  return boost::get<DeferredWrapper>(storage_);
-}
-
 void DeferredExecutor::addFrom(
     Executor::KeepAlive<>&& completingKA,
     Executor::KeepAlive<>::KeepAliveFunc func) {

folly/futures/detail/Core.h

@@ -22,8 +22,6 @@
 #include <utility>
 #include <vector>
 
-#include <boost/variant.hpp>
-
 #include <folly/Executor.h>
 #include <folly/Function.h>
 #include <folly/Optional.h>
@@ -90,43 +88,53 @@ using DeferredWrapper = std::unique_ptr<DeferredExecutor, UniqueDeleter>;
  * can safely be distinguished.
  */
 class KeepAliveOrDeferred {
+ private:
+  using KA = Executor::KeepAlive<>;
+  using DW = DeferredWrapper;
+
  public:
-  KeepAliveOrDeferred();
-  /* implicit */ KeepAliveOrDeferred(Executor::KeepAlive<> ka);
-  /* implicit */ KeepAliveOrDeferred(DeferredWrapper deferred);
+  KeepAliveOrDeferred() noexcept;
+  /* implicit */ KeepAliveOrDeferred(KA ka) noexcept;
+  /* implicit */ KeepAliveOrDeferred(DW deferred) noexcept;
   KeepAliveOrDeferred(KeepAliveOrDeferred&& other) noexcept;
 
   ~KeepAliveOrDeferred();
 
-  KeepAliveOrDeferred& operator=(KeepAliveOrDeferred&& other);
+  KeepAliveOrDeferred& operator=(KeepAliveOrDeferred&& other) noexcept;
 
-  DeferredExecutor* getDeferredExecutor() const;
+  DeferredExecutor* getDeferredExecutor() const noexcept;
 
-  Executor* getKeepAliveExecutor() const;
+  Executor* getKeepAliveExecutor() const noexcept;
 
-  Executor::KeepAlive<> stealKeepAlive() &&;
+  KA stealKeepAlive() && noexcept;
 
-  std::unique_ptr<DeferredExecutor, UniqueDeleter> stealDeferred() &&;
+  DW stealDeferred() && noexcept;
 
-  bool isDeferred() const;
+  bool isDeferred() const noexcept;
 
-  bool isKeepAlive() const;
+  bool isKeepAlive() const noexcept;
 
   KeepAliveOrDeferred copy() const;
 
-  explicit operator bool() const;
+  explicit operator bool() const noexcept;
 
  private:
   friend class DeferredExecutor;
 
-  Executor::KeepAlive<>& asKeepAlive();
-  const Executor::KeepAlive<>& asKeepAlive() const;
+  enum class State { Deferred, KeepAlive } state_;
+  union {
+    DW deferred_;
+    KA keepAlive_;
+  };
+};
 
-  DeferredWrapper& asDeferred();
-  const DeferredWrapper& asDeferred() const;
+inline bool KeepAliveOrDeferred::isDeferred() const noexcept {
+  return state_ == State::Deferred;
+}
 
-  boost::variant<DeferredWrapper, Executor::KeepAlive<>> storage_;
-};
+inline bool KeepAliveOrDeferred::isKeepAlive() const noexcept {
+  return state_ == State::KeepAlive;
+}
 
 /**
  * Defer work until executor is actively boosted.