(wangle) Return a Later from Future::via

Summary: Stroke of brilliance, Hannes.

Test Plan:
unit tests, including a new one
Looked through `fbgs 'via('` and all the extant `via`s are attached to `Later`s already so it shouldn't break anything. But check contbuild before commit.

Reviewed By: hannesr@fb.com

Subscribers: net-systems@, fugalh, exa

FB internal diff: D1406976

Tasks: 4480567

diff --git a/folly/wangle/Future-inl.h b/folly/wangle/Future-inl.h
index 97be14c416604c62a720866bc77d06ff1de5052a..aa45cb26d6a3ec8f5e7c2fe98aea49d1ee3dd801 100644 (file)
--- a/folly/wangle/Future-inl.h
+++ b/folly/wangle/Future-inl.h
@@ -183,20 +183,9 @@ Try<T>& Future<T>::getTry() {
 
 template <class T>
 template <typename Executor>
-inline Future<T> Future<T>::via(Executor* executor) {
+inline Later<T> Future<T>::via(Executor* executor) {
   throwIfInvalid();
-
-  folly::MoveWrapper<Promise<T>> p;
-  auto f = p->getFuture();
-
-  setCallback_([executor, p](Try<T>&& t) mutable {
-      folly::MoveWrapper<Try<T>> tt(std::move(t));
-      executor->add([p, tt]() mutable {
-          p->fulfilTry(std::move(*tt));
-        });
-    });
-
-  return f;
+  return Later<T>(std::move(*this)).via(executor);
 }
 
 template <class T>

diff --git a/folly/wangle/Future.h b/folly/wangle/Future.h
index b0c32081253b06622df8d0ad0efe9e726dca8dfc..1acf09736293d5d037ed0faeb1337b6e0d22506b 100644 (file)
--- a/folly/wangle/Future.h
+++ b/folly/wangle/Future.h
@@ -30,6 +30,7 @@
 namespace folly { namespace wangle {
 
 template <typename T> struct isFuture;
+template <class> class Later;
 
 template <class T>
 class Future {
@@ -59,17 +60,13 @@ class Future {
   typename std::add_lvalue_reference<const T>::type
   value() const;
 
-  /// Returns a future which will call back on the other side of executor.
+  /// Returns a Later which will call back on the other side of executor.
   ///
-  ///   f.via(e).then(a); // safe
+  ///   f.via(e).then(a).then(b).launch();
   ///
-  ///   f.via(e).then(a).then(b); // faux pas
-  ///
-  /// a will definitely execute in the intended thread, but b may execute
-  /// either in that thread, or in the current thread. If you need to
-  /// guarantee where b executes, use a Later.
+  /// a and b will execute in the same context (the far side of e)
   template <typename Executor>
-  Future<T> via(Executor* executor);
+  Later<T> via(Executor* executor);
 
   /** True when the result (or exception) is ready. */
   bool isReady() const;
@@ -316,3 +313,4 @@ Future<T> waitWithSemaphore(Future<T>&& f, Duration timeout);
 }} // folly::wangle
 
 #include "Future-inl.h"
+#include "Later.h"

diff --git a/folly/wangle/Later-inl.h b/folly/wangle/Later-inl.h
index d1e1ef88c2e9239c0a6261f690a420c3249fabce..fa9b86b199ee76e0a53d746809365cb6e78a7488 100644 (file)
--- a/folly/wangle/Later-inl.h
+++ b/folly/wangle/Later-inl.h
@@ -53,6 +53,15 @@ Later<T>::Later() {
   future_ = starter_.getFuture();
 }
 
+template <class T>
+Later<T>::Later(Future<T>&& f) {
+  MoveWrapper<Future<T>> fw(std::move(f));
+  *this = Later<void>()
+    .then([fw](Try<void>&&) mutable {
+      return std::move(*fw);
+    });
+}
+
 template <typename T>
 Later<T>::Later(Promise<void>&& starter)
   : starter_(std::forward<Promise<void>>(starter)) { }

diff --git a/folly/wangle/Later.h b/folly/wangle/Later.h
index c37e698a61acab19190f569e03d89021db2f4657..c01f293e44972f3a31ad70a6bdfef830dd1d609c 100644 (file)
--- a/folly/wangle/Later.h
+++ b/folly/wangle/Later.h
@@ -72,6 +72,11 @@ class Later {
             class = typename std::enable_if<std::is_same<T, U>::value>::type>
   Later();
 
+  /*
+   * Lift a Future into a Later
+   */
+  /* implicit */ Later(Future<T>&& f);
+
   /*
    * This constructor is used to build an asynchronous workflow that takes a
    * value as input, and that value is passed in.

diff --git a/folly/wangle/README.md b/folly/wangle/README.md
index b97a7272559fbe72f650a366e3eec76458239676..662a1ea07e500cf3d5c91c30f0bf5bdd9c81a778 100644 (file)
--- a/folly/wangle/README.md
+++ b/folly/wangle/README.md
@@ -201,9 +201,7 @@ Later<void>()
 ```
 `x` will execute in the current thread (the one calling `launch`). `y1` and `y2` will execute in the thread on the other side of `e1`, and `z` will execute in the thread on the other side of `e2`. `y1` and `y2` will execute on the same thread, whichever thread that is. If `e1` and `e2` execute in different threads than the current thread, then the final callback does not happen in the current thread. If you want to get back to the current thread, you need to get there via an executor.
 
-The second and most basic is `Future::via(Executor*)`, which creates a future which will execute its callback via the given executor. i.e. given `f.via(e).then(x)`, `x` will always execute via executor `e`. NB given `f.via(e).then(x).then(y)`, `y` is *not* guaranteed to execute via `e` or in the same thread as `x` (use a Later).
-
-TODO implement `Future::then(callback, executor)` so we can do the above with a single Future.
+`Future::via(Executor*)` will return a Later, too.
 
 The third and least flexible (but sometimes very useful) method assumes only two threads and that you want to do something in the far thread, then come back to the current thread. `ThreadGate` is an interface for a bidirectional gateway between two threads. It's usually easier to use a Later, but ThreadGate can be more efficient, and if the pattern is used often in your code it can be more convenient.
 ```C++

diff --git a/folly/wangle/test/LaterTest.cpp b/folly/wangle/test/LaterTest.cpp
index a8243f5d095ed7d232b84c4f413165405925cd5f..a1ad05e10d4ba2eba3898d11552c66d69194db34 100644 (file)
--- a/folly/wangle/test/LaterTest.cpp
+++ b/folly/wangle/test/LaterTest.cpp
@@ -167,3 +167,15 @@ TEST_F(LaterFixture, fire_and_forget) {
   }).fireAndForget();
   waiter->makeProgress();
 }
+
+TEST(Later, FutureViaReturnsLater) {
+  ManualExecutor x;
+  {
+    Future<void> f = makeFuture();
+    Later<void> l = f.via(&x);
+  }
+  {
+    Future<int> f = makeFuture(42);
+    Later<int> l = f.via(&x);
+  }
+}