From d903adc27c190c28cc349a7f1567b9f21007a306 Mon Sep 17 00:00:00 2001 From: Jacob Domagala Date: Fri, 3 Feb 2023 10:07:35 +0100 Subject: [PATCH] #2027: ObjGroup: Use if constexpr to avoid multiple functions in ObjGroup::invoke --- src/vt/objgroup/manager.h | 22 +----- src/vt/objgroup/manager.impl.h | 87 ++++++++-------------- tests/unit/objgroup/test_objgroup.cc | 7 +- tests/unit/objgroup/test_objgroup_common.h | 6 +- 4 files changed, 37 insertions(+), 85 deletions(-) diff --git a/src/vt/objgroup/manager.h b/src/vt/objgroup/manager.h index 492cb51c07..692fa32982 100644 --- a/src/vt/objgroup/manager.h +++ b/src/vt/objgroup/manager.h @@ -235,27 +235,7 @@ struct ObjGroupManager : runtime::component::Component { * \param[in] args function arguments */ template - util::NotCopyable invoke(ProxyElmType proxy, Args&&... args); - - /** - * \internal \brief Invoke function 'f' on an element of the object group - * The function will be invoked inline without going through scheduler - * - * \param[in] proxy proxy to the object group - * \param[in] args function arguments - */ - template - util::Copyable invoke(ProxyElmType proxy, Args&&... args); - - /** - * \internal \brief Invoke function 'f' on an element of the object group - * The function will be invoked inline without going through scheduler - * - * \param[in] proxy proxy to the object group - * \param[in] args function arguments - */ - template - util::IsVoidReturn invoke(ProxyElmType proxy, Args&&... args); + auto invoke(ProxyElmType proxy, Args&&... args); /** * \internal \brief Broadcast a message to all nodes in object group diff --git a/src/vt/objgroup/manager.impl.h b/src/vt/objgroup/manager.impl.h index 2a61d052ac..ca84372dcd 100644 --- a/src/vt/objgroup/manager.impl.h +++ b/src/vt/objgroup/manager.impl.h @@ -60,7 +60,6 @@ #include "vt/elm/elm_id_bits.h" #include -#include namespace vt { namespace objgroup { @@ -212,7 +211,7 @@ void ObjGroupManager::invoke( } template -util::IsVoidReturn +auto ObjGroupManager::invoke(ProxyElmType proxy, Args&&... args) { auto const dest_node = proxy.getNode(); auto const this_node = theContext()->getNode(); @@ -223,62 +222,34 @@ ObjGroupManager::invoke(ProxyElmType proxy, Args&&... args) { "Attempting to invoke handler on node:{} instead of node:{}!\n", this_node, dest_node)); - runnable::makeRunnableVoid(false, uninitialized_handler, this_node) - .withObjGroup(get(proxy)) - .runLambda([&] { - runnable::invoke(get(proxy), std::forward(args)...); - }); -} - -template -util::Copyable -ObjGroupManager::invoke(ProxyElmType proxy, Args&&... args) { - auto const dest_node = proxy.getNode(); - auto const this_node = theContext()->getNode(); - - vtAssert( - dest_node == this_node, - fmt::format( - "Attempting to invoke handler on node:{} instead of node:{}!\n", this_node, - dest_node - ) - ); - - util::Copyable result; - - runnable::makeRunnableVoid(false, uninitialized_handler, dest_node) - .withObjGroup(get(proxy)) - .runLambda([&] { - result = runnable::invoke(get(proxy), std::forward(args)...); - }); - - return result; -} - -template -util::NotCopyable -ObjGroupManager::invoke(ProxyElmType proxy, Args&&... args) { - auto const dest_node = proxy.getNode(); - auto const this_node = theContext()->getNode(); - - vtAssert( - dest_node == this_node, - fmt::format( - "Attempting to invoke handler on node:{} instead of node:{}!\n", this_node, - dest_node - ) - ); - - util::NotCopyable result; - - runnable::makeRunnableVoid(false, uninitialized_handler, dest_node) - .withObjGroup(get(proxy)) - .runLambda([&] { - auto&& ret = runnable::invoke(get(proxy), std::forward(args)...); - result = std::move(ret); - }); - - return result; + using Ret = typename util::FunctionWrapper::ReturnType; + constexpr bool is_void = std::is_same::value; + constexpr bool copyable = std::is_copy_constructible::value; + + if constexpr (not is_void) { + Ret result; + + runnable::makeRunnableVoid(false, uninitialized_handler, this_node) + .withObjGroup(get(proxy)) + .runLambda([&] { + if constexpr (copyable) { + result = + runnable::invoke(get(proxy), std::forward(args)...); + } else { + auto&& ret = + runnable::invoke(get(proxy), std::forward(args)...); + result = std::move(ret); + } + }); + + return result; + } else { + runnable::makeRunnableVoid(false, uninitialized_handler, this_node) + .withObjGroup(get(proxy)) + .runLambda([&] { + runnable::invoke(get(proxy), std::forward(args)...); + }); + } } diff --git a/tests/unit/objgroup/test_objgroup.cc b/tests/unit/objgroup/test_objgroup.cc index 23dd3440a9..d9f1626710 100644 --- a/tests/unit/objgroup/test_objgroup.cc +++ b/tests/unit/objgroup/test_objgroup.cc @@ -295,13 +295,14 @@ TEST_F(TestObjGroup, test_proxy_invoke) { EXPECT_EQ(proxy.get()->recv_, 2); // Non-copyable - MyObjA s{}; - auto const result = proxy[this_node] + std::unique_ptr s{}; + auto result = proxy[this_node] .invoke< decltype(&MyObjA::modifyNonCopyableStruct), &MyObjA::modifyNonCopyableStruct>(std::move(s)); - EXPECT_EQ(result.id_, 10); + EXPECT_TRUE(result); + EXPECT_EQ(*result, 10); EXPECT_EQ(proxy.get()->recv_, 3); } diff --git a/tests/unit/objgroup/test_objgroup_common.h b/tests/unit/objgroup/test_objgroup_common.h index 26bdbd83d3..eafc12d033 100644 --- a/tests/unit/objgroup/test_objgroup_common.h +++ b/tests/unit/objgroup/test_objgroup_common.h @@ -97,11 +97,11 @@ struct MyObjA { return std::accumulate(std::begin(vec), std::end(vec), 0); } - MyObjA modifyNonCopyableStruct(MyObjA&& i) { + std::unique_ptr modifyNonCopyableStruct(std::unique_ptr i) { recv_++; - i.id_ = 10; + i = std::make_unique(10); - return std::move(i); + return i; } int id_ = -1;