From 790b68acacebb628f9552b1c20fbafa4962aebb7 Mon Sep 17 00:00:00 2001
From: Yedidya Feldblum <yfeldblum@fb.com>
Date: Tue, 23 May 2017 19:18:12 -0700
Subject: [PATCH] RFC: Embed exception_wrapper directly into Try

Summary:
[Folly] RFC: Embed `exception_wrapper` directly into `Try`.

Rather than storing it elsewhere on the heap. With `exception_wrapper` at 24 bytes on x64, it may now be small enough. However, it will expand the size of `Try<T>` for `sizeof(T) <= 16`, giving `Try` a new minimum size of 32 bytes on x64 instead of 16.

Reviewed By: ericniebler

Differential Revision: D5051436

fbshipit-source-id: 10d59686d64382c88d54340c97567eafb3e2f682
---
 folly/Try-inl.h | 19 +++++++----------
 folly/Try.h     | 55 ++++++++++++++++++++++++++++---------------------
 2 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/folly/Try-inl.h b/folly/Try-inl.h
index 92f8f59a..e6e4c415 100644
--- a/folly/Try-inl.h
+++ b/folly/Try-inl.h
@@ -26,7 +26,7 @@ Try<T>::Try(Try<T>&& t) noexcept : contains_(t.contains_) {
   if (contains_ == Contains::VALUE) {
     new (&value_)T(std::move(t.value_));
   } else if (contains_ == Contains::EXCEPTION) {
-    new (&e_)std::unique_ptr<exception_wrapper>(std::move(t.e_));
+    new (&e_) exception_wrapper(std::move(t.e_));
   }
 }
 
@@ -40,8 +40,7 @@ Try<T>::Try(typename std::enable_if<std::is_same<Unit, T2>::value,
     new (&value_) T();
   } else if (t.hasException()) {
     contains_ = Contains::EXCEPTION;
-    new (&e_) std::unique_ptr<exception_wrapper>(
-        std::make_unique<exception_wrapper>(t.exception()));
+    new (&e_) exception_wrapper(t.exception());
   }
 }
 
@@ -56,7 +55,7 @@ Try<T>& Try<T>::operator=(Try<T>&& t) noexcept {
   if (contains_ == Contains::VALUE) {
     new (&value_)T(std::move(t.value_));
   } else if (contains_ == Contains::EXCEPTION) {
-    new (&e_)std::unique_ptr<exception_wrapper>(std::move(t.e_));
+    new (&e_) exception_wrapper(std::move(t.e_));
   }
   return *this;
 }
@@ -70,8 +69,7 @@ Try<T>::Try(const Try<T>& t) {
   if (contains_ == Contains::VALUE) {
     new (&value_)T(t.value_);
   } else if (contains_ == Contains::EXCEPTION) {
-    new (&e_)std::unique_ptr<exception_wrapper>();
-    e_ = std::make_unique<exception_wrapper>(*(t.e_));
+    new (&e_) exception_wrapper(t.e_);
   }
 }
 
@@ -85,8 +83,7 @@ Try<T>& Try<T>::operator=(const Try<T>& t) {
   if (contains_ == Contains::VALUE) {
     new (&value_)T(t.value_);
   } else if (contains_ == Contains::EXCEPTION) {
-    new (&e_)std::unique_ptr<exception_wrapper>();
-    e_ = std::make_unique<exception_wrapper>(*(t.e_));
+    new (&e_) exception_wrapper(t.e_);
   }
   return *this;
 }
@@ -96,7 +93,7 @@ Try<T>::~Try() {
   if (LIKELY(contains_ == Contains::VALUE)) {
     value_.~T();
   } else if (UNLIKELY(contains_ == Contains::EXCEPTION)) {
-    e_.~unique_ptr<exception_wrapper>();
+    e_.~exception_wrapper();
   }
 }
 
@@ -122,7 +119,7 @@ template <class T>
 void Try<T>::throwIfFailed() const {
   if (contains_ != Contains::VALUE) {
     if (contains_ == Contains::EXCEPTION) {
-      e_->throw_exception();
+      e_.throw_exception();
     } else {
       throw UsingUninitializedTry();
     }
@@ -131,7 +128,7 @@ void Try<T>::throwIfFailed() const {
 
 void Try<void>::throwIfFailed() const {
   if (!hasValue_) {
-    e_->throw_exception();
+    e_.throw_exception();
   }
 }
 
diff --git a/folly/Try.h b/folly/Try.h
index 86a5dee0..fe963753 100644
--- a/folly/Try.h
+++ b/folly/Try.h
@@ -93,8 +93,7 @@ class Try {
    * @param e The exception_wrapper
    */
   explicit Try(exception_wrapper e)
-      : contains_(Contains::EXCEPTION),
-        e_(std::make_unique<exception_wrapper>(std::move(e))) {}
+      : contains_(Contains::EXCEPTION), e_(std::move(e)) {}
 
   /*
    * DEPRECATED
@@ -107,10 +106,10 @@ class Try {
     : contains_(Contains::EXCEPTION) {
     try {
       std::rethrow_exception(ep);
-    } catch (const std::exception& e) {
-      e_ = std::make_unique<exception_wrapper>(std::current_exception(), e);
+    } catch (std::exception& e) {
+      e_ = exception_wrapper(std::current_exception(), e);
     } catch (...) {
-      e_ = std::make_unique<exception_wrapper>(std::current_exception());
+      e_ = exception_wrapper(std::current_exception());
     }
   }
 
@@ -195,21 +194,21 @@ class Try {
    */
   template <class Ex>
   bool hasException() const {
-    return hasException() && e_->is_compatible_with<Ex>();
+    return hasException() && e_.is_compatible_with<Ex>();
   }
 
   exception_wrapper& exception() {
     if (UNLIKELY(!hasException())) {
       throw TryException("exception(): Try does not contain an exception");
     }
-    return *e_;
+    return e_;
   }
 
   const exception_wrapper& exception() const {
     if (UNLIKELY(!hasException())) {
       throw TryException("exception(): Try does not contain an exception");
     }
-    return *e_;
+    return e_;
   }
 
   /*
@@ -220,11 +219,18 @@ class Try {
    * @returns True if the Try held an Ex and func was executed, false otherwise
    */
   template <class Ex, class F>
+  bool withException(F func) {
+    if (!hasException()) {
+      return false;
+    }
+    return e_.with_exception(std::move(func));
+  }
+  template <class Ex, class F>
   bool withException(F func) const {
     if (!hasException()) {
       return false;
     }
-    return e_->with_exception(std::move(func));
+    return e_.with_exception(std::move(func));
   }
 
   template <bool isTry, typename R>
@@ -241,7 +247,7 @@ class Try {
   Contains contains_;
   union {
     T value_;
-    std::unique_ptr<exception_wrapper> e_;
+    exception_wrapper e_;
   };
 };
 
@@ -265,9 +271,7 @@ class Try<void> {
    *
    * @param e The exception_wrapper
    */
-  explicit Try(exception_wrapper e)
-      : hasValue_(false),
-        e_(std::make_unique<exception_wrapper>(std::move(e))) {}
+  explicit Try(exception_wrapper e) : hasValue_(false), e_(std::move(e)) {}
 
   /*
    * DEPRECATED
@@ -280,18 +284,16 @@ class Try<void> {
     try {
       std::rethrow_exception(ep);
     } catch (const std::exception& e) {
-      e_ = std::make_unique<exception_wrapper>(std::current_exception(), e);
+      e_ = exception_wrapper(std::current_exception(), e);
     } catch (...) {
-      e_ = std::make_unique<exception_wrapper>(std::current_exception());
+      e_ = exception_wrapper(std::current_exception());
     }
   }
 
   // Copy assigner
   Try& operator=(const Try<void>& t) {
     hasValue_ = t.hasValue_;
-    if (t.e_) {
-      e_ = std::make_unique<exception_wrapper>(*t.e_);
-    }
+    e_ = t.e_;
     return *this;
   }
   // Copy constructor
@@ -315,7 +317,7 @@ class Try<void> {
   // @returns True if the Try contains an exception of type Ex, false otherwise
   template <class Ex>
   bool hasException() const {
-    return hasException() && e_->is_compatible_with<Ex>();
+    return hasException() && e_.is_compatible_with<Ex>();
   }
 
   /*
@@ -327,14 +329,14 @@ class Try<void> {
     if (UNLIKELY(!hasException())) {
       throw TryException("exception(): Try does not contain an exception");
     }
-    return *e_;
+    return e_;
   }
 
   const exception_wrapper& exception() const {
     if (UNLIKELY(!hasException())) {
       throw TryException("exception(): Try does not contain an exception");
     }
-    return *e_;
+    return e_;
   }
 
   /*
@@ -345,11 +347,18 @@ class Try<void> {
    * @returns True if the Try held an Ex and func was executed, false otherwise
    */
   template <class Ex, class F>
+  bool withException(F func) {
+    if (!hasException()) {
+      return false;
+    }
+    return e_.with_exception(std::move(func));
+  }
+  template <class Ex, class F>
   bool withException(F func) const {
     if (!hasException()) {
       return false;
     }
-    return e_->with_exception(std::move(func));
+    return e_.with_exception(std::move(func));
   }
 
   template <bool, typename R>
@@ -359,7 +368,7 @@ class Try<void> {
 
  private:
   bool hasValue_;
-  std::unique_ptr<exception_wrapper> e_{nullptr};
+  exception_wrapper e_;
 };
 
 /*
-- 
2.34.1