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

Compile time launch policies #2276

Merged
merged 8 commits into from Aug 8, 2016
Merged

Compile time launch policies #2276

merged 8 commits into from Aug 8, 2016

Conversation

hkaiser
Copy link
Member

@hkaiser hkaiser commented Aug 2, 2016

This changes the implementation of launch policies such that they can be used for tag dispaching.

@hkaiser hkaiser force-pushed the launch_policy branch 8 times, most recently from 1eff09e to a8eb093 Compare August 3, 2016 18:02
{
HPX_ASSERT(this->get_id());

typedef typename server::template_accumulator<T>::add_action
action_type;
hpx::apply<action_type>(this->get_id(), arg);
hpx::apply<action_type>(hpx::launch::sync, this->get_id(), arg);
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean? apply(sync, ...)?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a typo, thanks - will fix.

- free function API
- client side objects
- AGAS interface
- flyby: removing object_semaphore stubs
}
else
{
id_ = id;
Copy link
Member

Choose a reason for hiding this comment

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

do we really need to keep the id around if it is unmanaged?

Copy link
Member

Choose a reason for hiding this comment

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

an alternative would be to move it into handle_managed_target.

Copy link
Member Author

Choose a reason for hiding this comment

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

We usually pass id_types by const&, I don't see a way to move away from it in this case.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sithhell Better now?

Copy link
Member

Choose a reason for hiding this comment

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

A little, yes. But you are right ... the fact that we take the id_type by const reference is certainly a bit unfortunate here.

@hkaiser
Copy link
Member Author

hkaiser commented Aug 5, 2016

I think this PR is ready to be merged now. Some of the other pending PRs could be adjusted based on this work, thus merging this PR first would be preferable. Any objections?

@sithhell
Copy link
Member

sithhell commented Aug 5, 2016

One general remark that strikes me a bit is that async(hpx::launch::sync, ...) returns a future, while almost all other functions taking a the sync policy return an immediate value. This leads to a bit of an API inconsistency and still a lot of (more or less) code duplication.

@hkaiser
Copy link
Member Author

hkaiser commented Aug 5, 2016

One general remark that strikes me a bit is that async(hpx::launch::sync, ...) returns a future, while almost all other functions taking a the sync policy return an immediate value. This leads to a bit of an API inconsistency and still a lot of (more or less) code duplication.

Yes, you're certainly right. I'd rather see it as a way of supporting generic programming, where async can be used by generic code without having to deal with different return types. async is kind of a unique API as it is generic to begin with, so making it special might not be a problem.

If you need a 'truly' synchronous-looking syntax, use act(id, ...).

@biddisco
Copy link
Contributor

biddisco commented Aug 8, 2016

Looks good. Merging soon?

@sithhell
Copy link
Member

sithhell commented Aug 8, 2016

On 08/08/2016 02:11 PM, John Biddiscombe wrote:

Looks good. Merging soon?

Yes, feel free to merge.

@biddisco
Copy link
Contributor

biddisco commented Aug 8, 2016

I'm not able to merge stuff that isn't 100% up-to-date so I'll leave it to one of the admins.

@hkaiser hkaiser merged commit 624acb8 into master Aug 8, 2016
@hkaiser hkaiser deleted the launch_policy branch August 8, 2016 13:54
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.

None yet

3 participants