#2027: ObjGroup: Use if constexpr to avoid multiple functions in ObjG…

…roup::invoke
DARMA-tasking · Feb 3, 2023 · 3ac6c5d · 3ac6c5d
1 parent 2fa7392
commit 3ac6c5d
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 85 deletions.
diff --git a/src/vt/objgroup/manager.h b/src/vt/objgroup/manager.h
@@ -235,27 +235,7 @@ struct ObjGroupManager : runtime::component::Component<ObjGroupManager> {
    * \param[in] args function arguments
    */
   template <typename ObjT, typename Type, Type f, typename... Args>
-  util::NotCopyable<Type> invoke(ProxyElmType<ObjT> 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 <typename ObjT, typename Type, Type f, typename... Args>
-  util::Copyable<Type> invoke(ProxyElmType<ObjT> 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 <typename ObjT, typename Type, Type f, typename... Args>
-  util::IsVoidReturn<Type> invoke(ProxyElmType<ObjT> proxy, Args&&... args);
+  auto invoke(ProxyElmType<ObjT> 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
@@ -60,7 +60,6 @@
 #include "vt/elm/elm_id_bits.h"
 
 #include <memory>
-#include <cstdlib>
 
 namespace vt { namespace objgroup {
 
@@ -212,7 +211,7 @@ void ObjGroupManager::invoke(
 }
 
 template <typename ObjT, typename Type, Type f, typename... Args>
-util::IsVoidReturn<Type>
+auto
 ObjGroupManager::invoke(ProxyElmType<ObjT> proxy, Args&&... args) {
   auto const dest_node = proxy.getNode();
   auto const this_node = theContext()->getNode();
@@ -223,62 +222,34 @@ ObjGroupManager::invoke(ProxyElmType<ObjT> 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<Type, f>(get(proxy), std::forward<Args>(args)...);
-    });
-}
-
-template <typename ObjT, typename Type, Type f, typename... Args>
-util::Copyable<Type>
-ObjGroupManager::invoke(ProxyElmType<ObjT> 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<Type> result;
-
-  runnable::makeRunnableVoid(false, uninitialized_handler, dest_node)
-    .withObjGroup(get(proxy))
-    .runLambda([&] {
-      result = runnable::invoke<Type, f>(get(proxy), std::forward<Args>(args)...);
-    });
-
-  return result;
-}
-
-template <typename ObjT, typename Type, Type f, typename... Args>
-util::NotCopyable<Type>
-ObjGroupManager::invoke(ProxyElmType<ObjT> 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<Type> result;
-
-  runnable::makeRunnableVoid(false, uninitialized_handler, dest_node)
-    .withObjGroup(get(proxy))
-    .runLambda([&] {
-      auto&& ret = runnable::invoke<Type, f>(get(proxy), std::forward<Args>(args)...);
-      result = std::move(ret);
-    });
-
-  return result;
+  using Ret = typename util::FunctionWrapper<Type>::ReturnType;
+  constexpr bool is_void = std::is_same<Ret, void>::value;
+  constexpr bool copyable = std::is_copy_constructible<Ret>::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<Type, f>(get(proxy), std::forward<Args>(args)...);
+        } else {
+          auto&& ret =
+            runnable::invoke<Type, f>(get(proxy), std::forward<Args>(args)...);
+          result = std::move(ret);
+        }
+      });
+
+    return result;
+  } else {
+    runnable::makeRunnableVoid(false, uninitialized_handler, this_node)
+      .withObjGroup(get(proxy))
+      .runLambda([&] {
+        runnable::invoke<Type, f>(get(proxy), std::forward<Args>(args)...);
+      });
+  }
 }
 
 

diff --git a/tests/unit/objgroup/test_objgroup.cc b/tests/unit/objgroup/test_objgroup.cc
@@ -45,6 +45,7 @@
 #include "test_helpers.h"
 #include "vt/objgroup/manager.h"
 
+#include <gtest/gtest.h>
 #include <typeinfo>
 
 namespace vt { namespace tests { namespace unit {
@@ -295,13 +296,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<int32_t> 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
@@ -97,11 +97,11 @@ struct MyObjA {
     return std::accumulate(std::begin(vec), std::end(vec), 0);
   }
 
-  MyObjA modifyNonCopyableStruct(MyObjA&& i) {
+  std::unique_ptr<int32_t> modifyNonCopyableStruct(std::unique_ptr<int32_t> i) {
     recv_++;
-    i.id_ = 10;
+    i = std::make_unique<int32_t>(10);
 
-    return std::move(i);
+    return i;
   }
 
   int id_ = -1;