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
Consistently use executors to schedule work #3039
Conversation
- customization points for thread-executors now don't rely on async/apply anymore - async(exec, ...)/apply(exec, ...) now always dispatch to executor customization points - future::then(thread_exec, ...) now dispatches to executor customization points - flyby: more proper forwarding for predecessor in executor API # Conflicts: # hpx/parallel/executors/execution.hpp # hpx/parallel/executors/thread_execution.hpp
- generalizing parallel_executor to be statically parameterized by a launch policy (introducing parallel_policy_executor<Policy>, parallel_executor is now just a alias for parallel_policy_executor<hpx::launch>) - adding test for parallel_policy_executor - parallel_policy_executor and sequenced_executor now don't use async/apply anymore # Conflicts: # hpx/parallel/executors/execution_fwd.hpp
Cleanup of #includes to avoid cyclic include dependencies - created forwarding header for types related to performance counters - refactored some code that does not have to be exposed in header files - changed executor_traits (V1) to not use apply/async anymore Also: - dataflow now schedules all work through the executor customization points
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, the PR looks good to go. Please fix the one comment.
There are lots of necessary, but unrelated fixes in this changeset, would it make sense to put those in a seperate PR with meaningful commit messages? Especially regarding the parcel handling, big boot barrier and performance counter changes.
return detail::then_execute_helper(exec, std::forward<F>(f), | ||
std::move(fut)); | ||
return detail::then_execute_helper(std::forward<Executor_>(exec), | ||
std::forward<F>(f), std::move(fut)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fut
should be forwarded here instead of unconditionally moved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. Future && fut
is an rvalue reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D'oh, you are right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I missed the commit with the include dis-entangling.
This fixes #3027