Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contains and contains_subrange parallel algorithm implementation GSOC 2024 #6497

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
#include <cstddef>
#include <iterator>
#include <type_traits>
#include "hpx/config/forward.hpp"
#include "hpx/iterator_support/traits/is_sentinel_for.hpp"

namespace hpx::parallel { namespace detail {
struct contains : public algorithm<contains, bool>
Expand Down Expand Up @@ -88,29 +86,20 @@ namespace hpx::parallel { namespace detail {
struct contains_subrange_helper<ExPolicy, T,
std::enable_if_t<!hpx::is_async_execution_policy_v<ExPolicy>>>
{
static constexpr T get(T& t)
static bool get(T& itr, T& last)
{
return HPX_FORWARD(T, t);
}

static constexpr T get(hpx::future<T>& t)
{
return t.get();
return itr != last;
}
};

template <typename ExPolicy, typename T>
struct contains_subrange_helper<ExPolicy, T,
std::enable_if_t<hpx::is_async_execution_policy_v<ExPolicy>>>
{
static constexpr T get(T& t)
{
return HPX_FORWARD(T, t);
}

static constexpr T get(hpx::future<T>& t)
static hpx::future<bool> get(hpx::future<T>& itr, T& last)
{
return t.get();
return itr.then(
[&last](hpx::future<T> it) { return it.get() != last; });
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pansysk75 Hi Panos, I'm not sure if this is correct, could you check this line of code I changed? I changed the helper function to return hpx::future, a continuation to the previous future, by using .then().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks right, i think you might want to capture last by value and not by reference, as it is a local variable of the parallel function. Otherwise looks good!

}
};

Expand Down Expand Up @@ -148,8 +137,7 @@ namespace hpx::parallel { namespace detail {
HPX_FORWARD(Proj2, proj2));

return util::detail::algorithm_result<ExPolicy, bool>::get(
contains_subrange_helper<ExPolicy, FwdIter1>().get(itr) !=
last1);
contains_subrange_helper<ExPolicy, FwdIter1>().get(itr, last1));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think returning contains_subrange_helper<ExPolicy, FwdIter1>().get(itr, last1) would suffice here.
The return types of your helper function already match with algorithm_result_t. Calling that additional ::get will call .get() on your future before returning it, which undermines your latest change on your helper.

}
};

Expand Down