folly/futures: use folly::Function to store callback

Summary:This diff makes it possible to pass callables (such as lambdas) to
folly::Future::then that are not copy-constructible. As a consequence, move-only
types (such as folly::Promise or std::unique_ptr) can now be captured in lambdas
passed to folly::Future::then.

Using C++14 notation, the following is now possible:
  Future<Unit>().then([promise = std::move(promise)]() mutable {
    promise.setValue(123);
  });
See folly/futures/test/NonCopyableLambdaTest.cpp for more examples.

folly::Future uses std::function to store callback functions. (More precisely,
the callback function is stored in a std::function in folly::detail::Core.)
std::function is a copy-constructible type and it requires that the callable
that it stores is copy constructible as well.

This diff changes the implementation of folly::detail::Core to use
folly::Function instead of std::function. It also simplifies the code in
folly::detail::Core a little bit: Core had a reserved space of size
8*sizeof(void*) to store callbacks in-place. Only larger callbacks (capturing
more data) would be stored in the std::function, which puts those on the heap.
folly::Function has a template parameter to set the size of the in-place
callable storage. In Core, it is set to 8*sizeof(void*), so all callbacks that
used to be stored in-place inside Core, are now stored in-place inside
folly::Function. This even reduces the size of a Core object: the
folly::Function object occupies 80 bytes, which is 16 bytes less than what was
used before for a std::function (32 bytes) and the callback storage (64
bytes). The resulting size of a Core<Unit> goes down from 192 to 176 bytes (on
x86_64).

Reviewed By: fugalh

Differential Revision: D2884868

fb-gh-sync-id: 35548890c392f80e6c680676b0f98e711bc03ca3
fbshipit-source-id: 35548890c392f80e6c680676b0f98e711bc03ca3

diff --git a/folly/futures/detail/Core.h b/folly/futures/detail/Core.h
index 70f887d2365ccba8de93116ddd1531f9f16c5537..20bb78b1bc88ef1868418595ee11cbf3c50f30eb 100644 (file)
--- a/folly/futures/detail/Core.h
+++ b/folly/futures/detail/Core.h
@@ -21,13 +21,13 @@
 #include <stdexcept>
 #include <vector>
 
-#include <folly/Optional.h>
+#include <folly/Executor.h>
+#include <folly/Function.h>
 #include <folly/MicroSpinLock.h>
-
-#include <folly/futures/Try.h>
-#include <folly/futures/Promise.h>
+#include <folly/Optional.h>
 #include <folly/futures/Future.h>
-#include <folly/Executor.h>
+#include <folly/futures/Promise.h>
+#include <folly/futures/Try.h>
 #include <folly/futures/detail/FSM.h>
 
 #include <folly/io/async/Request.h>
@@ -147,34 +147,13 @@ class Core {
     }
   }
 
-  template <typename F>
-  class LambdaBufHelper {
-   public:
-    template <typename FF>
-    explicit LambdaBufHelper(FF&& func) : func_(std::forward<FF>(func)) {}
-    void operator()(Try<T>&& t) {
-      SCOPE_EXIT { this->~LambdaBufHelper(); };
-      func_(std::move(t));
-    }
-   private:
-    F func_;
-  };
-
   /// Call only from Future thread.
   template <typename F>
   void setCallback(F func) {
     bool transitionToArmed = false;
     auto setCallback_ = [&]{
       context_ = RequestContext::saveContext();
-
-      // Move the lambda into the Core if it fits
-      if (sizeof(LambdaBufHelper<F>) <= lambdaBufSize) {
-        auto funcLoc = reinterpret_cast<LambdaBufHelper<F>*>(&lambdaBuf_);
-        new (funcLoc) LambdaBufHelper<F>(std::forward<F>(func));
-        callback_ = std::ref(*funcLoc);
-      } else {
-        callback_ = std::move(func);
-      }
+      callback_ = std::move(func);
     };
 
     FSM_START(fsm_)
@@ -395,13 +374,14 @@ class Core {
   // sizeof(Core<T>) == size(Core<U>).
   // See Core::convert for details.
 
-  // lambdaBuf occupies exactly one cache line
-  static constexpr size_t lambdaBufSize = 8 * sizeof(void*);
-  typename std::aligned_storage<lambdaBufSize>::type lambdaBuf_;
+  folly::Function<
+      void(Try<T>&&),
+      folly::FunctionMoveCtor::MAY_THROW,
+      8 * sizeof(void*)>
+      callback_;
   // place result_ next to increase the likelihood that the value will be
   // contained entirely in one cache line
   folly::Optional<Try<T>> result_;
-  std::function<void(Try<T>&&)> callback_ {nullptr};
   FSM<State> fsm_;
   std::atomic<unsigned char> attached_;
   std::atomic<bool> active_ {true};

diff --git a/folly/futures/test/NonCopyableLambdaTest.cpp b/folly/futures/test/NonCopyableLambdaTest.cpp
new file mode 100644 (file)
index 0000000..73bc6dc
--- /dev/null
+++ b/folly/futures/test/NonCopyableLambdaTest.cpp
@@ -0,0 +1,92 @@
+/*
+ * Copyright 2016 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+
+#include <folly/futures/Future.h>
+
+using namespace folly;
+
+TEST(NonCopyableLambda, basic) {
+  Promise<int> promise;
+  Future<int> future = promise.getFuture();
+
+  Future<Unit>().then(std::bind(
+      [](Promise<int>& promise) mutable { promise.setValue(123); },
+      std::move(promise)));
+
+  // The previous statement can be simplified in C++14:
+  //  Future<Unit>().then([promise = std::move(promise)]() mutable {
+  //    promise.setValue(123);
+  //  });
+
+  EXPECT_TRUE(future.isReady());
+  EXPECT_EQ(future.get(), 123);
+}
+
+TEST(NonCopyableLambda, unique_ptr) {
+  Promise<Unit> promise;
+  auto int_ptr = folly::make_unique<int>(1);
+
+  EXPECT_EQ(*int_ptr, 1);
+
+  auto future = promise.getFuture().then(std::bind(
+      [](std::unique_ptr<int>& int_ptr) mutable {
+        ++*int_ptr;
+        return std::move(int_ptr);
+      },
+      std::move(int_ptr)));
+
+  // The previous statement can be simplified in C++14:
+  //  auto future =
+  //      promise.getFuture().then([int_ptr = std::move(int_ptr)]() mutable {
+  //        ++*int_ptr;
+  //        return std::move(int_ptr);
+  //      });
+
+  EXPECT_FALSE(future.isReady());
+  promise.setValue();
+  EXPECT_TRUE(future.isReady());
+  EXPECT_EQ(*future.get(), 2);
+}
+
+TEST(NonCopyableLambda, Function) {
+  Promise<int> promise;
+
+  Function<int(int)> callback = [](int x) { return x + 1; };
+
+  auto future = promise.getFuture().then(std::move(callback));
+  EXPECT_THROW(callback(0), std::bad_function_call);
+
+  EXPECT_FALSE(future.isReady());
+  promise.setValue(100);
+  EXPECT_TRUE(future.isReady());
+  EXPECT_EQ(future.get(), 101);
+}
+
+TEST(NonCopyableLambda, FunctionConst) {
+  Promise<int> promise;
+
+  Function<int(int) const> callback = [](int x) { return x + 1; };
+
+  auto future = promise.getFuture().then(std::move(callback));
+  EXPECT_THROW(callback(0), std::bad_function_call);
+
+  EXPECT_FALSE(future.isReady());
+  promise.setValue(100);
+  EXPECT_TRUE(future.isReady());
+  EXPECT_EQ(future.get(), 101);
+}

diff --git a/folly/test/Makefile.am b/folly/test/Makefile.am
index 7d037b9613598d6c2bdef1353806f021016db2e7..71fe7a80a99126b1cb5c065fe938caa0e08211cf 100644 (file)
--- a/folly/test/Makefile.am
+++ b/folly/test/Makefile.am
@@ -226,6 +226,7 @@ futures_test_SOURCES = \
     ../futures/test/HeaderCompileTest.cpp \
     ../futures/test/InterruptTest.cpp \
     ../futures/test/MapTest.cpp \
+    ../futures/test/NonCopyableLambdaTest.cpp \
     ../futures/test/PollTest.cpp \
     ../futures/test/PromiseTest.cpp \
     ../futures/test/ReduceTest.cpp \