From: Hans Fugal <fugalh@fb.com>
Date: Thu, 26 Jun 2014 23:23:11 +0000 (-0700)
Subject: (wangle) Return a Later from Future::via
X-Git-Tag: v0.22.0~490
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=a181fa047a616222a4826516c00d7dc36d8a3deb;p=folly.git

(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 97be14c4..aa45cb26 100644
--- 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 b0c32081..1acf0973 100644
--- 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 d1e1ef88..fa9b86b1 100644
--- 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 c37e698a..c01f293e 100644
--- 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 b97a7272..662a1ea0 100644
--- 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 a8243f5d..a1ad05e1 100644
--- 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);
+  }
+}