Allowing remote direct actions to be executed without spawning a task #2336
Conversation
|
Note : This partially addresses #2284 |
| // non HPX thread. | ||
| bool recurse_asynchronously = hpx::threads::get_self_ptr() == nullptr; | ||
| #if defined(HPX_HAVE_THREADS_GET_STACK_POINTER) | ||
| recurse_asynchronously = | ||
| !this_thread::has_sufficient_stack_space(); |
There was a problem hiding this comment.
Does this_thread::has_sufficient_stack_space() fail if not executed on a HPX thread?
There was a problem hiding this comment.
No. If not executed on an HPX thread, this_thread::has_sufficient_stack_space() returns false.
| @@ -82,6 +82,8 @@ namespace hpx { namespace lcos { namespace server | |||
| } | |||
| } | |||
|
|
|||
| HPX_DEFINE_COMPONENT_ACTION(barrier, set_event, set_event_non_direct_action); | |||
|
|
|||
There was a problem hiding this comment.
We might want to move this new action to the base_lco base class as other LCO's might require this functionality as well.
There was a problem hiding this comment.
Good thinking, will fix that.
| // Direct actions should be able to be executed from a non-HPX thread | ||
| // as well | ||
| if (this_thread::has_sufficient_stack_space() || | ||
| hpx::threads::get_self_ptr() == nullptr) |
There was a problem hiding this comment.
Here you check both, the stack space and whether its running on an HPX thread. You might want to revisit my comment above related to this.
There was a problem hiding this comment.
We discussed this when introducing the semantics of this_thread::has_sufficient_stack_space(), which returns false when not in an HPX thread. I never liked this since a regular OS thread should have sufficient stack space in any case.
There was a problem hiding this comment.
Either way is fine, let's make it consistent, though.
There was a problem hiding this comment.
What we have now is consistent, I guess. It is just the special case of when we try to run a direct action inside of one of the parcel pool threads where I want it to be run directly as well (as in not on a HPX thread, but still execute it directly if it is a direct action).
There was a problem hiding this comment.
Ok, I see it now. Thanks for the explanations.
| {}; | ||
| }} | ||
|
|
||
| #define HPX_TRAITS_ACTION_DIRECT_EXECUTION(Action, DirectExecution) \ |
There was a problem hiding this comment.
Is this new traits class used anywhere? Shouldn't all existing direct actions now use this?
There was a problem hiding this comment.
This trait class doesn't have any specialization so far. It is used where we check for direct actions, which returns std::true_type when we have a nested direct_execution typedef.
There was a problem hiding this comment.
I understand that. But the trait not only has no specializations, it is not used anywhere. Do we need it at all?
There was a problem hiding this comment.
The idea for that was to add a kind of tag type to base_lco and base_lco_with_value to allow direct or non direct action execution based on the actual implementation of the LCO. Implementing that turned out to be a little challenging, so I decided to drop that for now. I am fine with removing this trait.
sithhell
force-pushed
the
direct_actions
branch
from
f23f2e8
to
2ae4a76
Compare
With this patch, direct actions are now always executed directly on the receiving side. This means that they might get executed in a non-HPX thread.
sithhell
force-pushed
the
direct_actions
branch
from
2ae4a76
to
98af5b8
Compare
|
The review comments have been addressed. |
With this patch, direct actions are now always executed directly on the
receiving side. This means that they might get executed in a non-HPX thread.
This is a breaking change in behavior as it doesn't require direct_actions to run on HPX threads.