search

openshmem-org / specification

OpenSHMEM Application Programming Interface
http://www.openshmem.org
51 stars 40 forks source link

Per PE Quiet interface #316

Closed manjugv closed 2 years ago

manjugv commented 4 years ago

Closes #317

agrippa commented 4 years ago

As a non-implementer, I may need some help with this.

Is there a performance benefit to this routine over maintaining 1 context per PE I communicate with? At the application level that might undesirably lead to memory consumption scaling with # of PEs, so are there tricks in the runtime layer that mean it won't?

Could the options parameter to shmem_ctx_create be used to set a property on contexts (e.g. SHMEM_ONLY_USED_WITH_ONE_PE) that could have a similar effect rather than introducing a new API?

manjugv commented 4 years ago

@agrippa That is definitely one way to do it. As a matter of fact, with that option, one can do more optimizations. However, it is much stronger semantic than the proposal here. The context is creating a communication stream. With your proposed change, you are creating a stream per PE and you have an ability to couple that stream with a thread.

shamisp commented 4 years ago

@manjugv UCX support this, but if you look at the code it comes with the cost. It is not huge but you have to track outstanding messages on the PE. If this semantics is useful for OpenSHMEM users, I'm all for it.

nspark commented 2 years ago

WG feedback: Clarify that the target_pes pointer isn't accessed if npes is zero.

manjugv commented 2 years ago

@jdinan Do you need anything else on this before merging?

jdinan commented 2 years ago

@manjugv I added a few doc editor updates for you to review and apply. I also see that there are unresolved comments on this PR. Please resolve these, e.g. by adding doc edit changes to this PR, filing new issues, new PRs, etc.

manjugv commented 2 years ago

@jdinan I accepted the commit. Thanks.

jdinan commented 2 years ago

@manjugv Could you please resolve the open comments on this ticket before merging? It looks like some of these should have issues created for follow-up before the next spec release.

manjugv commented 2 years ago

@jdinan I have resolved all comments. Thanks.