Skip to content

Commit

Permalink
use when_all | then instead of dataflow in rotate.hpp
Browse files Browse the repository at this point in the history
  • Loading branch information
isidorostsa committed May 17, 2024
1 parent 53ce2ec commit 20094d3
Showing 1 changed file with 30 additions and 6 deletions.
36 changes: 30 additions & 6 deletions libs/core/algorithms/include/hpx/parallel/algorithms/rotate.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,7 @@ namespace hpx::parallel {

detail::reverse<FwdIter> r;

return hpx::dataflow(
hpx::launch::sync,
[=](auto&& f1, auto&& f2) mutable {
auto&& func = [=](auto&& f1, auto&& f2) mutable {
// propagate exceptions, if appropriate
constexpr bool handle_futures =
hpx::traits::is_future_v<decltype((f1))> &&
Expand All @@ -284,9 +282,35 @@ namespace hpx::parallel {

std::advance(first, size_right);
return util::in_out_result<FwdIter, Sent>{first, last};
},
r.call(left_policy, first, new_first),
r.call(right_policy, new_first, last));
};

using rcall_left_t = decltype(r.call(left_policy, first, new_first));
using rcall_right_t = decltype(r.call(left_policy, new_first, last));

constexpr bool rcall_are_senders = (
hpx::execution::experimental::sender<rcall_left_t> &&
hpx::execution::experimental::sender<rcall_right_t>);

This comment has been minimized.

Copy link
@hkaiser

hkaiser May 17, 2024

Member

Shouldn't this be is_sender_v?

This comment has been minimized.

Copy link
@isidorostsa

isidorostsa May 17, 2024

Author Contributor

this is using the concept instead. I am planning to always use either the metafunction or the concept, though I am not sure which. The main benefit that I have experienced from using the concept is way better error messages. Do you think I should default to the metafunction for now?

This comment has been minimized.

Copy link
@hkaiser

hkaiser May 17, 2024

Member

If it should work for our implementation as well we need to stick with the metafunctions.

This comment has been minimized.

Copy link
@isidorostsa

isidorostsa May 17, 2024

Author Contributor

Right, forgot about that. Would it make sense for is_sender_v to be a concept if we are using stdexec?

This comment has been minimized.

Copy link
@hkaiser

hkaiser May 17, 2024

Member

Right, forgot about that. Would it make sense for is_sender_v to be a concept if we are using stdexec?

Sure, same for all the other related metafunctions.


constexpr bool rcall_are_futures = (
hpx::traits::is_future_v<rcall_left_t> &&
hpx::traits::is_future_v<rcall_right_t>);

static_assert(rcall_are_senders || rcall_are_futures,
"the reverse operation must return either a sender or a future");

if constexpr (rcall_are_senders) {
return hpx::execution::experimental::when_all(
r.call(left_policy, first, new_first),
r.call(right_policy, new_first, last))
| hpx::execution::experimental::then(std::move(func));

This comment has been minimized.

Copy link
@hkaiser

hkaiser May 17, 2024

Member

May I ask not to use the pipeline syntax for the internal use of senders? Not using it is a bit lighter on the compiler.

This comment has been minimized.

Copy link
@isidorostsa

isidorostsa May 17, 2024

Author Contributor

On it, thanks for the hint!

} else if constexpr (rcall_are_futures) {
return hpx::dataflow(
hpx::launch::sync,
std::move(func),
r.call(left_policy, first, new_first),
r.call(right_policy, new_first, last)
);
}
}

template <typename IterPair>
Expand Down

0 comments on commit 20094d3

Please sign in to comment.