From a181fa047a616222a4826516c00d7dc36d8a3deb Mon Sep 17 00:00:00 2001 From: Hans Fugal Date: Thu, 26 Jun 2014 16:23:11 -0700 Subject: [PATCH] (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 --- folly/wangle/Future-inl.h | 15 ++------------- folly/wangle/Future.h | 14 ++++++-------- folly/wangle/Later-inl.h | 9 +++++++++ folly/wangle/Later.h | 5 +++++ folly/wangle/README.md | 4 +--- folly/wangle/test/LaterTest.cpp | 12 ++++++++++++ 6 files changed, 35 insertions(+), 24 deletions(-) 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& Future::getTry() { template template -inline Future Future::via(Executor* executor) { +inline Later Future::via(Executor* executor) { throwIfInvalid(); - - folly::MoveWrapper> p; - auto f = p->getFuture(); - - setCallback_([executor, p](Try&& t) mutable { - folly::MoveWrapper> tt(std::move(t)); - executor->add([p, tt]() mutable { - p->fulfilTry(std::move(*tt)); - }); - }); - - return f; + return Later(std::move(*this)).via(executor); } template 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 struct isFuture; +template class Later; template class Future { @@ -59,17 +60,13 @@ class Future { typename std::add_lvalue_reference::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 - Future via(Executor* executor); + Later via(Executor* executor); /** True when the result (or exception) is ready. */ bool isReady() const; @@ -316,3 +313,4 @@ Future waitWithSemaphore(Future&& 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::Later() { future_ = starter_.getFuture(); } +template +Later::Later(Future&& f) { + MoveWrapper> fw(std::move(f)); + *this = Later() + .then([fw](Try&&) mutable { + return std::move(*fw); + }); +} + template Later::Later(Promise&& starter) : starter_(std::forward>(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::value>::type> Later(); + /* + * Lift a Future into a Later + */ + /* implicit */ Later(Future&& 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() ``` `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 f = makeFuture(); + Later l = f.via(&x); + } + { + Future f = makeFuture(42); + Later l = f.via(&x); + } +} -- 2.34.1