Navigation Menu

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

Fixing return type calculation for bulk_then_execute. #3205

Merged
merged 2 commits into from Mar 2, 2018
Merged

Conversation

hkaiser
Copy link
Member

@hkaiser hkaiser commented Feb 28, 2018

  • flyby: added util::functional::unwrap[_n|_all]
  • some restructuring required by circular #include dependencies

Fixes #3182

Unfortunately this PR contains a lot of unrelated changes that were necessary in order to disentangle a couple of circular #include dependencies that were introduced by the initial fix to the original problem.

@msimberg
Copy link
Contributor

msimberg commented Mar 1, 2018

Thanks @hkaiser, this works as intended (below is likely unrelated).

Out of curiousity, HPX futures don't seem to wait in the destructors if get/wait was not called earlier. This seems to be a contentious topic for the standards, but is there a reason for HPX choosing to go this way? (I'm asking because I wanted to see what the thread pool executors do now if one does not get the future before destroying the executor -- currently it hangs, but I have not investigated further).

@hkaiser
Copy link
Member Author

hkaiser commented Mar 1, 2018

Out of curiousity, HPX futures don't seem to wait in the destructors if get/wait was not called earlier. This seems to be a contentious topic for the standards, but is there a reason for HPX choosing to go this way? (I'm asking because I wanted to see what the thread pool executors do now if one does not get the future before destroying the executor -- currently it hangs, but I have not investigated further).

Yes, HPX futures don't wait on destruction if the value was not extracted. Please note that even the standard prescribes this only for futures that have been returned from std::async. The rationale for the standard prescribing this is to avoid for the user to easily being able to create 'runaway' threads, i.e. threads that the user has no way of controlling as there is no 'handle' to them available anymore once the future instance has gone away.

We have deliberately diverged from this for two reasons: a) in HPX we can't have runways threads as the runtime waits for all threads to terminate eventually before exiting, also there is always a way for us to access still running threads, if needed, and b) (probably more important) we wanted for all futures to be equal in semantics, no matter how they were created (through async or any other means).

- flyby: added util::functional::unwrap[_n|_all]
- some restructuring required by circular #include dependencies
@msimberg
Copy link
Contributor

msimberg commented Mar 2, 2018

@hkaiser I added some missing includes which for some reason pycicle (and my local build) caught but circleci did not (here). Will check if pycicle complains about something else after this.

@hkaiser
Copy link
Member Author

hkaiser commented Mar 2, 2018

@msimberg Thanks a lot!

@hkaiser hkaiser merged commit 022d849 into master Mar 2, 2018
@hkaiser hkaiser deleted the fixing_3182 branch March 2, 2018 16:24
@msimberg
Copy link
Contributor

msimberg commented Mar 5, 2018

So this seems to have broken the sequenced_executor test (link).

I think the test itself may not be correct though. From the docs: "hpx::parallel::execution::sequenced_executor: creates groups of sequential execution agents which execute in the calling thread." It looks to me like the test should check that the thread ids are equal, not different. Am I interpreting the docs correctly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bulk_then_execute has unexpected return type/does not compile
2 participants