From 9c5d19bc19d78f6068cccf73a28e757b6ce7d768 Mon Sep 17 00:00:00 2001 From: Tyler Neely Date: Thu, 21 Jul 2022 11:00:11 +0000 Subject: [PATCH] Start to adapt the transport-related code to use namespaces --- src/io/v3/future.hpp | 86 +++++++++++++++++-------------- src/io/v3/simulator_handle.hpp | 4 -- src/io/v3/simulator_transport.hpp | 3 +- src/io/v3/transport.hpp | 18 +++---- tests/simulation/future.cpp | 6 ++- 5 files changed, 59 insertions(+), 58 deletions(-) diff --git a/src/io/v3/future.hpp b/src/io/v3/future.hpp index 5ea5b8edf..26ef55326 100644 --- a/src/io/v3/future.hpp +++ b/src/io/v3/future.hpp @@ -22,6 +22,12 @@ #include "io/v3/errors.hpp" +namespace memgraph::io { + +// Shared is in an anonymous namespace, and the only way to +// construct a Promise or Future is to pass a Shared in. This +// ensures that Promises and Futures can only be constructed +// in this translation unit. namespace { template class Shared { @@ -71,7 +77,7 @@ class Shared { if (!simulator_progressed) { cv_.wait(lock); } - MG_ASSERT(!consumed_, "MgFuture consumed twice!"); + MG_ASSERT(!consumed_, "Future consumed twice!"); } T ret = std::move(item_).value(); @@ -108,8 +114,8 @@ class Shared { { std::unique_lock lock(mu_); - MG_ASSERT(!consumed_, "MgPromise filled after it was already consumed!"); - MG_ASSERT(!item_, "MgPromise filled twice!"); + MG_ASSERT(!consumed_, "Promise filled after it was already consumed!"); + MG_ASSERT(!item_, "Promise filled twice!"); item_ = item; } // lock released before condition variable notification @@ -125,35 +131,35 @@ class Shared { } // namespace template -class MgFuture { +class Future { bool consumed_or_moved_ = false; std::shared_ptr> shared_; public: - explicit MgFuture(std::shared_ptr> shared) : shared_(shared) {} + explicit Future(std::shared_ptr> shared) : shared_(shared) {} - MgFuture() = delete; - MgFuture(MgFuture &&old) { + Future() = delete; + Future(Future &&old) { shared_ = std::move(old.shared_); consumed_or_moved_ = old.consumed_or_moved_; - MG_ASSERT(!old.consumed_or_moved_, "MgFuture moved from after already being moved from or consumed."); + MG_ASSERT(!old.consumed_or_moved_, "Future moved from after already being moved from or consumed."); old.consumed_or_moved_ = true; } - MgFuture &operator=(MgFuture &&old) { + Future &operator=(Future &&old) { shared_ = std::move(old.shared_); - MG_ASSERT(!old.consumed_or_moved_, "MgFuture moved from after already being moved from or consumed."); + MG_ASSERT(!old.consumed_or_moved_, "Future moved from after already being moved from or consumed."); old.consumed_or_moved_ = true; } - MgFuture(const MgFuture &) = delete; - MgFuture &operator=(const MgFuture &) = delete; - ~MgFuture() = default; + Future(const Future &) = delete; + Future &operator=(const Future &) = delete; + ~Future() = default; - /// Returns true if the MgFuture is ready to + /// Returns true if the Future is ready to /// be consumed using TryGet or Wait (prefer Wait /// if you know it's ready, because it doesn't /// return an optional. bool IsReady() { - MG_ASSERT(!consumed_or_moved_, "Called IsReady after MgFuture already consumed!"); + MG_ASSERT(!consumed_or_moved_, "Called IsReady after Future already consumed!"); return shared_->IsReady(); } @@ -161,7 +167,7 @@ class MgFuture { /// item if it's already ready, or std::nullopt /// if it is not ready yet. std::optional TryGet() { - MG_ASSERT(!consumed_or_moved_, "Called TryGet after MgFuture already consumed!"); + MG_ASSERT(!consumed_or_moved_, "Called TryGet after Future already consumed!"); std::optional ret = shared_->TryGet(); if (ret) { consumed_or_moved_ = true; @@ -172,55 +178,55 @@ class MgFuture { /// Block on the corresponding promise to be filled, /// returning the inner item when ready. T Wait() { - MG_ASSERT(!consumed_or_moved_, "MgFuture should only be consumed with Wait once!"); + MG_ASSERT(!consumed_or_moved_, "Future should only be consumed with Wait once!"); T ret = shared_->Wait(); consumed_or_moved_ = true; return ret; } - /// Marks this MgFuture as canceled. + /// Marks this Future as canceled. void Cancel() { - MG_ASSERT(!consumed_or_moved_, "MgFuture::Cancel called on a future that was already moved or consumed!"); + MG_ASSERT(!consumed_or_moved_, "Future::Cancel called on a future that was already moved or consumed!"); consumed_or_moved_ = true; } }; template -class MgPromise { +class Promise { std::shared_ptr> shared_; bool filled_or_moved_ = false; public: - explicit MgPromise(std::shared_ptr> shared) : shared_(shared) {} + explicit Promise(std::shared_ptr> shared) : shared_(shared) {} - MgPromise() = delete; - MgPromise(MgPromise &&old) { + Promise() = delete; + Promise(Promise &&old) { shared_ = std::move(old.shared_); - MG_ASSERT(!old.filled_or_moved_, "MgPromise moved from after already being moved from or filled."); + MG_ASSERT(!old.filled_or_moved_, "Promise moved from after already being moved from or filled."); old.filled_or_moved_ = true; } - MgPromise &operator=(MgPromise &&old) { + Promise &operator=(Promise &&old) { shared_ = std::move(old.shared_); - MG_ASSERT(!old.filled_or_moved_, "MgPromise moved from after already being moved from or filled."); + MG_ASSERT(!old.filled_or_moved_, "Promise moved from after already being moved from or filled."); old.filled_or_moved_ = true; } - MgPromise(const MgPromise &) = delete; - MgPromise &operator=(const MgPromise &) = delete; + Promise(const Promise &) = delete; + Promise &operator=(const Promise &) = delete; - ~MgPromise() { MG_ASSERT(filled_or_moved_, "MgPromise destroyed before its associated MgFuture was filled!"); } + ~Promise() { MG_ASSERT(filled_or_moved_, "Promise destroyed before its associated Future was filled!"); } // Fill the expected item into the Future. void Fill(T item) { - MG_ASSERT(!filled_or_moved_, "MgPromise::Fill called on a promise that is already filled or moved!"); + MG_ASSERT(!filled_or_moved_, "Promise::Fill called on a promise that is already filled or moved!"); shared_->Fill(item); filled_or_moved_ = true; } bool IsAwaited() { return shared_->IsAwaited(); } - /// Moves this MgPromise into a unique_ptr. - std::unique_ptr> ToUnique() && { - std::unique_ptr> up = std::make_unique>(std::move(shared_)); + /// Moves this Promise into a unique_ptr. + std::unique_ptr> ToUnique() && { + std::unique_ptr> up = std::make_unique>(std::move(shared_)); filled_or_moved_ = true; @@ -229,21 +235,23 @@ class MgPromise { }; template -std::pair, MgPromise> FuturePromisePair() { +std::pair, Promise> FuturePromisePair() { std::shared_ptr> shared = std::make_shared>(); - MgFuture future = MgFuture(shared); - MgPromise promise = MgPromise(shared); + Future future = Future(shared); + Promise promise = Promise(shared); return std::make_pair(std::move(future), std::move(promise)); } template -std::pair, MgPromise> FuturePromisePairWithNotifier(std::function simulator_notifier) { +std::pair, Promise> FuturePromisePairWithNotifier(std::function simulator_notifier) { std::shared_ptr> shared = std::make_shared>(simulator_notifier); - MgFuture future = MgFuture(shared); - MgPromise promise = MgPromise(shared); + Future future = Future(shared); + Promise promise = Promise(shared); return std::make_pair(std::move(future), std::move(promise)); } + +}; // namespace memgraph::io diff --git a/src/io/v3/simulator_handle.hpp b/src/io/v3/simulator_handle.hpp index 9b7c4dcf0..65603e3b1 100644 --- a/src/io/v3/simulator_handle.hpp +++ b/src/io/v3/simulator_handle.hpp @@ -29,10 +29,6 @@ #include "io/v3/simulator_stats.hpp" #include "io/v3/transport.hpp" -// TODO(tyler) enforce this around std::any usage -template -concept SameAsDecayed = std::same_as>; - struct OpaqueMessage { Address from_address; uint64_t request_id; diff --git a/src/io/v3/simulator_transport.hpp b/src/io/v3/simulator_transport.hpp index 29db90b63..ac21ccaeb 100644 --- a/src/io/v3/simulator_transport.hpp +++ b/src/io/v3/simulator_transport.hpp @@ -30,7 +30,8 @@ class SimulatorTransport { ResponseFuture Request(Address address, uint64_t request_id, Request request, uint64_t timeout_microseconds) { std::function maybe_tick_simulator = [=] { return simulator_handle_->MaybeTickSimulator(); }; - auto [future, promise] = FuturePromisePairWithNotifier>(maybe_tick_simulator); + auto [future, promise] = + memgraph::io::FuturePromisePairWithNotifier>(maybe_tick_simulator); simulator_handle_->SubmitRequest(address, address_, request_id, std::move(request), timeout_microseconds, std::move(promise)); diff --git a/src/io/v3/transport.hpp b/src/io/v3/transport.hpp index f5937ba9e..e077f3479 100644 --- a/src/io/v3/transport.hpp +++ b/src/io/v3/transport.hpp @@ -28,17 +28,11 @@ class SimulatorHandle; template class Io; +// TODO(tyler) ensure that Message continues to represent +// reasonable constraints around message types over time, +// as we adapt things to use Thrift-generated message types. template -concept Message = true; - -/*requires(T a, uint8_t *ptr, size_t len) { - // These are placeholders and will be replaced - // by some concept that identifies Thrift-generated - // messages. - { a.Serialize() } -> std::same_as>; - { T::Deserialize(ptr, len) } -> std::same_as; -}; -*/ +concept Message = std::same_as>; template struct ResponseEnvelope { @@ -51,10 +45,10 @@ template using ResponseResult = BasicResult>; template -using ResponseFuture = MgFuture>; +using ResponseFuture = memgraph::io::Future>; template -using ResponsePromise = MgPromise>; +using ResponsePromise = memgraph::io::Promise>; template struct RequestEnvelope { diff --git a/tests/simulation/future.cpp b/tests/simulation/future.cpp index a0e53c243..76a2e61b1 100644 --- a/tests/simulation/future.cpp +++ b/tests/simulation/future.cpp @@ -15,9 +15,11 @@ #include "io/v3/future.hpp" #include "utils/logging.hpp" -void Fill(MgPromise promise_1) { promise_1.Fill("success"); } +using namespace memgraph::io; -void Wait(MgFuture future_1, MgPromise promise_2) { +void Fill(Promise promise_1) { promise_1.Fill("success"); } + +void Wait(Future future_1, Promise promise_2) { std::string result_1 = future_1.Wait(); MG_ASSERT(result_1 == "success"); promise_2.Fill("it worked");