From a6eb914b25eaa73a567eaf508c2b19b9b7a7079f Mon Sep 17 00:00:00 2001 From: Hans Fugal Date: Fri, 5 Dec 2014 09:33:25 -0800 Subject: [PATCH] Remove Later Summary: dalek-exterminate Test Plan: Moved `Later` tests to `via` tests. fbgs contbuild Reviewed By: jsedgwick@fb.com Subscribers: fbcode-common-diffs@, adityab, lins, trunkagent, fugalh, exa, njormrod, folly-diffs@, hannesr FB internal diff: D1714862 Tasks: 5409538 Signature: t1:1714862:1417621949:f63f49e1093a021170d2346e8e673db042d2bc56 --- folly/Makefile.am | 2 - folly/wangle/Future.h | 2 - folly/wangle/Later-inl.h | 207 ---------------- folly/wangle/Later.h | 234 ------------------ folly/wangle/OpaqueCallbackShunt.h | 35 +-- folly/wangle/README.md | 7 - folly/wangle/detail/Core.h | 9 - .../test/{LaterTest.cpp => ViaTest.cpp} | 124 +++------- 8 files changed, 37 insertions(+), 583 deletions(-) delete mode 100644 folly/wangle/Later-inl.h delete mode 100644 folly/wangle/Later.h rename folly/wangle/test/{LaterTest.cpp => ViaTest.cpp} (50%) diff --git a/folly/Makefile.am b/folly/Makefile.am index 59e45023..bd5cc48d 100644 --- a/folly/Makefile.am +++ b/folly/Makefile.am @@ -237,8 +237,6 @@ nobase_follyinclude_HEADERS = \ wangle/Future-inl.h \ wangle/Future.h \ wangle/InlineExecutor.h \ - wangle/Later-inl.h \ - wangle/Later.h \ wangle/ManualExecutor.h \ wangle/OpaqueCallbackShunt.h \ wangle/Promise-inl.h \ diff --git a/folly/wangle/Future.h b/folly/wangle/Future.h index 9c19d37f..57f8636b 100644 --- a/folly/wangle/Future.h +++ b/folly/wangle/Future.h @@ -91,8 +91,6 @@ class Future { /// /// f = f.via(e).then(a); /// f.then(b); - /// - /// If you need something like that, use a Later. template Future via(Executor* executor); diff --git a/folly/wangle/Later-inl.h b/folly/wangle/Later-inl.h deleted file mode 100644 index b054d6d5..00000000 --- a/folly/wangle/Later-inl.h +++ /dev/null @@ -1,207 +0,0 @@ -/* - * Copyright 2014 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. - */ - -#pragma once - -#include -#include -#include - -namespace folly { namespace wangle { - -template -struct isLater { - static const bool value = false; -}; - -template -struct isLater > { - static const bool value = true; -}; - -template -struct isLaterOrFuture { - static const bool value = false; -}; - -template -struct isLaterOrFuture> { - static const bool value = true; -}; - -template -struct isLaterOrFuture> { - static const bool value = true; -}; - -template -template -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)) { } - -template -template -Later::Later(U&& input) { - folly::MoveWrapper> promise; - folly::MoveWrapper inputm(std::forward(input)); - future_ = promise->getFuture(); - starter_.getFuture().then([=](Try&& t) mutable { - promise->setValue(std::move(*inputm)); - }); -} - -template -Later::Later(std::exception_ptr const& eptr) { - folly::MoveWrapper> promise; - future_ = promise->getFuture(); - starter_.getFuture().then([=](Try&& t) mutable { - promise->setException(eptr); - }); -} - -template -template -Later::Later(E const& e) : - Later::Later(std::make_exception_ptr(e)) { -} - -template -template -Later::Later(std::function&&)>&& fn) { - folly::MoveWrapper> promise; - future_ = promise->getFuture(); - starter_.getFuture().then([=](Try&& t) mutable { - fn([=](U&& output) mutable { - promise->setValue(std::move(output)); - }); - }); -} - -template -template -typename std::enable_if< - !isLaterOrFuture&&)>::type>::value, - Later&&)>::type> >::type -Later::then(F&& fn) { - typedef typename std::result_of&&)>::type B; - - Later later(std::move(starter_)); - later.future_ = future_->then(std::forward(fn)); - return later; -} - -template -template -typename std::enable_if< - isFuture&&)>::type>::value, - Later&&)>::type::value_type> >::type -Later::then(F&& fn) { - typedef typename std::result_of&&)>::type::value_type B; - - Later later(std::move(starter_)); - later.future_ = future_->then(std::move(fn)); - return later; -} - -template -template -typename std::enable_if< - isLater&&)>::type>::value, - Later&&)>::type::value_type> >::type -Later::then(F&& fn) { - typedef typename std::result_of&&)>::type::value_type B; - - folly::MoveWrapper> promise; - folly::MoveWrapper fnm(std::move(fn)); - Later later(std::move(starter_)); - later.future_ = promise->getFuture(); - future_->then([=](Try&& t) mutable { - (*fnm)(std::move(t)) - .then([=](Try&& t2) mutable { - promise->fulfilTry(std::move(t2)); - }) - .launch(); - }); - return later; -} - -template -Later Later::via(Executor* executor) { - folly::MoveWrapper> promise; - Later later(std::move(starter_)); - later.future_ = promise->getFuture(); - - future_->setCallback_([executor, promise](Try&& t) mutable { - folly::MoveWrapper> tt(std::move(t)); - executor->add([promise, tt]() mutable { - promise->fulfilTry(std::move(*tt)); - }); - }); - - return later; -} - -template -Future Later::launch() { - starter_.setValue(); - return std::move(*future_); -} - -template -Later>> whenAllLater(std::vector>&& laters) { - if (laters.size() == 0) { - return Later>>(std::vector>()); - } - - auto ctx = new detail::WhenAllLaterContext(); - ctx->total = laters.size(); - ctx->results.resize(ctx->total); - - MoveWrapper>> mlaters{std::move(laters)}; - - std::function>&&)>&&)> wrapper = - [ctx, mlaters](std::function>&&)>&& fn) mutable { - ctx->fn = std::move(fn); - size_t i = 0; - for (auto& l : *mlaters) { - l.then([ctx, i](Try&& t) { - ctx->results[i] = std::move(t); - if (++ctx->count == ctx->total) { - ctx->fn(std::move(ctx->results)); - delete ctx; - } - }).launch(); - ++i; - } - }; - return Later>>(std::move(wrapper)); -} - -}} diff --git a/folly/wangle/Later.h b/folly/wangle/Later.h deleted file mode 100644 index 329748e1..00000000 --- a/folly/wangle/Later.h +++ /dev/null @@ -1,234 +0,0 @@ -/* - * Copyright 2014 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. - */ - -#pragma once - -#include -#include -#include -#include - -namespace folly { namespace wangle { - -template struct isLaterOrFuture; -template struct isLater; - -/* - * Later is like a cold Future, but makes it easier to avoid triggering until - * later, because it must be triggered explicitly. An equivalence example will - * help differentiate: - * - * Later later = - * Later(std::move(foo)) - * .then(cb1) - * .via(ex1) - * .then(cb2) - * .then(cb3) - * .via(ex2) - * .then(cb4) - * .then(cb5); - * ... - * later.launch(); - * - * Future coldFuture = makeFuture(std::move(foo)); - * coldFuture.deactivate(); - * coldFuture - * .then(cb1) - * .via(ex1) - * .then(cb2) - * .then(cb3) - * .via(ex2) - * .then(cb4) - * .then(cb5); - * ... - * coldFuture.activate(); - * - * Using a Later means you don't have to grab a handle to the first Future and - * deactivate it. - * - * Later used to be a workaround to the thread-unsafe nature of Future - * chaining, but that has changed and there is no need to use Later if your - * only goal is to traverse thread boundaries with executors. In that case, - * just use Future::via(). - * - * Here is an example of a workflow: - * - * Later later(std::move(request)); - * - * auto future = later. - * .via(cpuExecutor) - * .then([=](Try&& t) { return doCpuWork(t.value()); }) - * .via(diskExecutor) - * .then([=](Try&& t) { return doDiskWork(t.value()); }) - * .via(serverExecutor) - * .then([=]Try&& t) { return sendClientResponse(t.value()); }) - * .launch(); - */ -// DEPRECATED. Just use Future::via() to accomplish the same thing. If it's -// not obvious how, feel free to reach out. -template -class DEPRECATED Later { - public: - typedef T value_type; - - /* - * This default constructor is used to build an asynchronous workflow that - * takes no input. - */ - template ::value>::type, - 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. - */ - template ::value>::type, - class = typename std::enable_if::value>::type> - explicit Later(U&& input); - - /* - * This constructor is used to build an asynchronous workflow that takes an - * exception_ptr as input, and throws it on completion. - */ - explicit Later(std::exception_ptr const&); - - /* - * This constructor is used to build an asynchronous workflow that takes an - * exception as input, and throws it on completion. - */ - template ::value>::type> - explicit Later(E const& e); - - /* - * This constructor is used to wrap a pre-existing cob-style asynchronous api - * so that it can be used in wangle. wangle provides the callback to this - * pre-existing api, and this callback will fulfill a promise so as to - * incorporate this api into the workflow. - * - * Example usage: - * - * // This adds two ints asynchronously. cob is called in another thread. - * void addAsync(int a, int b, std::function&& cob); - * - * Later asyncWrapper([=](std::function&& fn) { - * addAsync(1, 2, std::move(fn)); - * }); - */ - // TODO we should implement a makeFuture-ish with this pattern too, now. - template ::value>::type, - class = typename std::enable_if::value>::type> - explicit Later(std::function&&)>&& fn); - - /* - * then() adds additional work to the end of the workflow. If the lambda - * provided to then() returns a future, that future must be fulfilled in the - * same thread of the last set executor (either at constructor or from a call - * to via()). - */ - template - typename std::enable_if< - !isLaterOrFuture&&)>::type>::value, - Later&&)>::type> >::type - then(F&& fn); - - template - typename std::enable_if< - isFuture&&)>::type>::value, - Later&&)>::type::value_type> >::type - then(F&& fn); - - /* - * If the function passed to then() returns a Later, calls to then() will - * be chained to the new Later before launching the new Later. - * - * This can be used to build asynchronous modules that can be called from a - * user thread and completed in a callback thread. - * - * Using the Later(std::function)>&& fn) - * constructor, you can wrap existing asynchronous modules with a Later and - * can chain it to wangle asynchronous workflows via this call. - */ - template - typename std::enable_if< - isLater&&)>::type>::value, - Later&&)>::type::value_type> >::type - then(F&& fn); - - - /// Variant where func is an ordinary function (static method, method) - /// Must return a Later - template - typename std::enable_if::value, R>::type - inline then(R(*func)(Try&&)) { - return then([func](Try&& t) { - return (*func)(std::move(t)); - }); - } - - /// Variant where func is an member function - /// Must return a Later - template - typename std::enable_if::value, R>::type - inline then(Caller *instance, R(Caller::*func)(Try&&)) { - return then([instance, func](Try&& t) { - return (instance->*func)(std::move(t)); - }); - } - - /* - * Resets the executor - all then() calls made after the call to via() will be - * made in the new executor. The Executor must outlive. - */ - Later via(Executor* executor); - - /* - * Starts the workflow. The function provided in the constructor will be - * called in the executor provided in the constructor. Subsequent then() - * calls will be made, potentially changing threads if a via() call is made. - * The future returned will be fulfilled in the last executor. - */ - Future launch(); - - private: - Promise starter_; - folly::Optional> future_; - - struct hide { }; - - explicit Later(Promise&& starter); - - template - friend class Later; -}; - -// See Future.whenAll -template -Later>> whenAllLater(std::vector>&& laters); - -}} - -#include diff --git a/folly/wangle/OpaqueCallbackShunt.h b/folly/wangle/OpaqueCallbackShunt.h index a5aebf8e..6cd325ec 100644 --- a/folly/wangle/OpaqueCallbackShunt.h +++ b/folly/wangle/OpaqueCallbackShunt.h @@ -21,7 +21,7 @@ namespace folly { namespace wangle { /// These classes help you wrap an existing C style callback function -/// into a Future/Later. +/// into a Future. /// /// void legacy_send_async(..., void (*cb)(void*), void*); /// @@ -54,37 +54,4 @@ private: T obj_; }; -/// Variant that returns a Later instead of a Future -/// -/// Later wrappedSendAsyncLater(int i) { -/// folly::MoveWrapper wrapped(std::move(i)); -/// return Later( -/// [..., wrapped](std::function&& fn) mutable { -/// auto handle = new OpaqueCallbackLaterShunt( -/// std::move(*wrapped), std::move(fn)); -/// legacy_send_async(..., -/// OpaqueCallbackLaterShunt::callback, handle); -/// }); -/// } -/// -/// Depending on your compiler's kung-fu knowledge, you might need to assign -/// the lambda to a std::function&&)> temporary -/// variable before std::moving into it into the later. - -template -class OpaqueCallbackLaterShunt { -public: - explicit - OpaqueCallbackLaterShunt(T&& obj, std::function&& fn) - : fn_(std::move(fn)), obj_(std::move(obj)) { } - static void callback(void* arg) { - std::unique_ptr> handle( - static_cast*>(arg)); - handle->fn_(std::move(handle->obj_)); - } -private: - std::function fn_; - T obj_; -}; - }} // folly::wangle diff --git a/folly/wangle/README.md b/folly/wangle/README.md index 57f1230a..cb88fe36 100644 --- a/folly/wangle/README.md +++ b/folly/wangle/README.md @@ -207,13 +207,6 @@ You can still have a race after `via` if you break it into multiple statements, f = f.via(e1).then(y1).then(y2); // nothing racy here f2.then(y3); // racy ``` -If you want more control over the delayed execution, check out `Later`. -```C++ -Later later; -later = later.via(e1).then(y1).then(y2); // nothing racy here -later = later.then(y3); // nor here -later.launch(); // explicit launch -``` ## You make me Promises, Promises diff --git a/folly/wangle/detail/Core.h b/folly/wangle/detail/Core.h index 1113e0b2..03d09b74 100644 --- a/folly/wangle/detail/Core.h +++ b/folly/wangle/detail/Core.h @@ -289,13 +289,4 @@ struct WhenAnyContext { } }; -template -struct WhenAllLaterContext { - explicit WhenAllLaterContext() : count(0), total(0) {} - std::function>&&)> fn; - std::vector > results; - std::atomic count; - size_t total; -}; - }}} // namespace diff --git a/folly/wangle/test/LaterTest.cpp b/folly/wangle/test/ViaTest.cpp similarity index 50% rename from folly/wangle/test/LaterTest.cpp rename to folly/wangle/test/ViaTest.cpp index 97dfd9fc..9a3758c2 100644 --- a/folly/wangle/test/LaterTest.cpp +++ b/folly/wangle/test/ViaTest.cpp @@ -17,9 +17,9 @@ #include #include -#include +#include #include -#include +#include using namespace folly::wangle; @@ -34,8 +34,9 @@ struct ManualWaiter { std::shared_ptr ex; }; -struct LaterFixture : public testing::Test { - LaterFixture() : +struct ViaFixture : public testing::Test { + ViaFixture() : + future_(makeFuture().deactivate()), westExecutor(new ManualExecutor), eastExecutor(new ManualExecutor), waiter(new ManualWaiter(westExecutor)), @@ -48,7 +49,7 @@ struct LaterFixture : public testing::Test { }); } - ~LaterFixture() { + ~ViaFixture() { done = true; eastExecutor->add([=]() { }); t.join(); @@ -60,7 +61,7 @@ struct LaterFixture : public testing::Test { }); } - Later later; + Future future_; std::shared_ptr westExecutor; std::shared_ptr eastExecutor; std::shared_ptr waiter; @@ -69,110 +70,77 @@ struct LaterFixture : public testing::Test { std::thread t; }; -TEST(Later, construct_and_launch) { - bool fulfilled = false; - auto later = Later().then([&](Try&& t) { - fulfilled = true; - return makeFuture(1); - }); - - // has not started yet. - EXPECT_FALSE(fulfilled); - - EXPECT_EQ(later.launch().value(), 1); - EXPECT_TRUE(fulfilled); -} - -TEST(Later, exception_on_launch) { - auto later = Later(std::runtime_error("E")); - EXPECT_THROW(later.launch().value(), std::runtime_error); +TEST(Via, exception_on_launch) { + auto future = makeFuture(std::runtime_error("E")); + EXPECT_THROW(future.value(), std::runtime_error); } -TEST(Later, then_value) { - auto future = Later(std::move(1)) +TEST(Via, then_value) { + auto future = makeFuture(std::move(1)) .then([](Try&& t) { return t.value() == 1; }) - .launch(); + ; EXPECT_TRUE(future.value()); } -TEST(Later, then_future) { - auto future = Later(1) +TEST(Via, then_future) { + auto future = makeFuture(1) .then([](Try&& t) { return makeFuture(t.value() == 1); }) - .launch(); + ; EXPECT_TRUE(future.value()); } -static Later doWorkStatic(Try&& t) { - return Later(t.value() + ";static"); +static Future doWorkStatic(Try&& t) { + return makeFuture(t.value() + ";static"); } -TEST(Later, then_function) { +TEST(Via, then_function) { struct Worker { - Later doWork(Try&& t) { - return Later(t.value() + ";class"); + Future doWork(Try&& t) { + return makeFuture(t.value() + ";class"); } - static Later doWorkStatic(Try&& t) { - return Later(t.value() + ";class-static"); + static Future doWorkStatic(Try&& t) { + return makeFuture(t.value() + ";class-static"); } } w; - auto f = Later(std::string("start")) + auto f = makeFuture(std::string("start")) .then(doWorkStatic) .then(Worker::doWorkStatic) .then(&w, &Worker::doWork) - .launch(); + ; EXPECT_EQ(f.value(), "start;static;class-static;class"); } -TEST_F(LaterFixture, thread_hops) { +TEST_F(ViaFixture, thread_hops) { auto westThreadId = std::this_thread::get_id(); - auto future = later.via(eastExecutor.get()).then([=](Try&& t) { + auto f = future_.via(eastExecutor.get()).then([=](Try&& t) { EXPECT_NE(std::this_thread::get_id(), westThreadId); return makeFuture(1); }).via(westExecutor.get() ).then([=](Try&& t) { EXPECT_EQ(std::this_thread::get_id(), westThreadId); return t.value(); - }).launch(); - while (!future.isReady()) { - waiter->makeProgress(); - } - EXPECT_EQ(future.value(), 1); -} - -TEST_F(LaterFixture, wrapping_preexisting_async_modules) { - auto westThreadId = std::this_thread::get_id(); - std::function&&)> wrapper = - [=](std::function&& fn) { - addAsync(2, 2, std::move(fn)); - }; - auto future = Later(std::move(wrapper)) - .via(westExecutor.get()) - .then([=](Try&& t) { - EXPECT_EQ(std::this_thread::get_id(), westThreadId); - return t.value(); - }) - .launch(); - while (!future.isReady()) { + }); + while (!f.isReady()) { waiter->makeProgress(); } - EXPECT_EQ(future.value(), 4); + EXPECT_EQ(f.value(), 1); } -TEST_F(LaterFixture, chain_laters) { +TEST_F(ViaFixture, chain_vias) { auto westThreadId = std::this_thread::get_id(); - auto future = later.via(eastExecutor.get()).then([=](Try&& t) { + auto f = future_.via(eastExecutor.get()).then([=](Try&& t) { EXPECT_NE(std::this_thread::get_id(), westThreadId); return makeFuture(1); }).then([=](Try&& t) { int val = t.value(); - return Later(std::move(val)).via(westExecutor.get()) + return makeFuture(std::move(val)).via(westExecutor.get()) .then([=](Try&& t) mutable { EXPECT_EQ(std::this_thread::get_id(), westThreadId); return t.value(); @@ -180,31 +148,11 @@ TEST_F(LaterFixture, chain_laters) { }).then([=](Try&& t) { EXPECT_EQ(std::this_thread::get_id(), westThreadId); return t.value(); - }).launch(); + }); - while (!future.isReady()) { + while (!f.isReady()) { waiter->makeProgress(); } - EXPECT_EQ(future.value(), 1); + EXPECT_EQ(f.value(), 1); } -TEST(Later, when_all_later) { - size_t done = 0; - std::vector> laters; - laters.emplace_back(Later(1).then([&](Try&& i) mutable { - done += i.value(); return 8; - })); - laters.emplace_back(Later(2).then([&](Try&& i) mutable { - done += i.value(); return 16; - })); - laters.emplace_back(Later(4).then([&](Try&& i) mutable { - done += i.value(); return 32; - })); - whenAllLater(std::move(laters)) - .then([&](Try>>&& v) mutable { - for (const auto& i : v.value()) { - done += i.value(); - } - }).launch(); - EXPECT_EQ(done, 63); -} -- 2.34.1