-
-
Notifications
You must be signed in to change notification settings - Fork 427
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
Fix a problem about asynchronous execution of parallel::merge and parallel::partition. #3041
Fix a problem about asynchronous execution of parallel::merge and parallel::partition. #3041
Conversation
hpx/parallel/algorithms/merge.hpp
Outdated
dest + (last1 - first1) + (last2 - first2)); | ||
// Not reachable. | ||
HPX_ASSERT(false); | ||
}); |
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.
Could we prefect forward comp
, proj1
and proj2
into the lambda?
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.
@sithhell I did. There was 3 key points.
- The lambda must have own variables including
comp
,proj1
, andproj2
because the variables will be used in asynchronous manner. - So, the lambda must construct own variables from original parameters. In this point, the parameters must be perfect forwarded into constructors of the own variables.
- Support C++11. (This is why I use
std::bind()
.)
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.
@sithhell How do you think about my modification for perfect forwarding?
There are some places that this way can be applied to. (For example,
hpx/hpx/parallel/util/scan_partitioner.hpp
Lines 342 to 350 in a5ac8fc
hpx::future<R> f = execution::async_execute( | |
policy.executor(), | |
[=]() mutable -> R | |
{ | |
return static_scan_partitioner_helper< | |
R, Result1, Result2, ScanPartTag | |
>::call(policy, first, count, init, | |
f1, f2, f3, f4); | |
}); |
If this way is appropriate enough, I will apply it to some places, too.
7304e05
to
89bbf2c
Compare
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.
LGTM, thanks a lot!
No description provided.