Clarify behavior of non-empty promises with rput/rget

Issue #41 resolved
Dan Bonachea created an issue

From current chapter 5.6:

\upcxx operations that operate on promises make an important distinction between empty and non-empty promises: applying a \upcxx operation to an empty promise \emph{does} increment its dependency count, calling a \upcxx operation on a non-empty promise \emph{does not} increment its dependency count. The rationale for this is that an operation on a non-empty promise can only fulfill its initial, value-representing dependency, while an operation on an empty promise always fulfills an anonymous dependency.

However the spec for rput(..,promise) says simply:

Increments the dependency count of the given promise. The dependency will be fulfilled after the transfer completes

or for VIS:

In the promise variant, an anonymous dependency is added to the promise during the call and is fulfilled upon completion.

making no distinction in behavior for empty vs non-empty promises.

These sections seem to be behaviorally in conflict when the user passes a non-empty promise. I don't understand how the first statement in 5.6 can possibly be correct for UPC++ operations on non-empty promises that perform memory accesses. In particular, it's definitely unsafe/incorrect to issue an rput or rget that has no way to notify the some thread of final completion (as it effectively renders the contents of all destination memory permanently undefined).

I suspect the fix is to remove the relevant sentences from 5.6, but I don't understand why they were there in the first place.

Comments (9)

  1. Dan Bonachea reporter

    If the issue is that any given UPC++ operation is restricted to take only an empty promise or only a non-empty promise (but not either), then perhaps it should be clarified something like:

    • UPC++ operations that do not produce a value accept only empty promises for promise-based synchronization. They increment the anonymous dependency count of the promise at initiation, and decrement the anonymous dependency count upon completion.
    • UPC++ operations that produce a value generally require a non-empty promise for promise-based synchronization. They do NOT increment any dependency count on initiation, but DO fulfill the non-anonymous dependency on completion.

    As a corollary, the UPC++ operations that accept a non-empty promise should probably include a precondition that the dependency count of the input promise must be greater than zero and that the value-representing dependency on this promise has not been fulfilled before. (Note it's not sufficient to rely upon the preconditions of promise methods here, because the semantics of promise fulfillment for RMA is not specified in terms of user-level calls to promise methods - although perhaps it should be).

  2. Amir Kamil

    Most operations will take a promise of a specific type, so that it is always empty or non-empty. rpc is an exception, where the promise type depends on the return type of the remote function. For both rpc and rput/rget, I've added preconditions concerning the dependency status of promises. These remain to be added to VIS.

  3. Dan Bonachea reporter

    Thanks Amir, your commit helps.

    I still think section 5.6 needs to be reworded to avoid giving the impression that the user has a choice about which flavor of promise can be passed to any given UPC++ function.

    I think there's also a subtle problem with some of the promise preconditions, because they are stated in terms of method calls. For example, promise::fulfill_result:

    Precondition: fulfill_result has not been called on this promise before, and the dependency count of this promise is greater than zero.

    However I think we also want to disallow conflicting actions performed by the runtime in promise fulfillment, which are not specified in terms of method calls. Eg:

    promise <int> pro; // initial dependency count is 1
    pro.require_anonymous (1); // dependency count is now 2
    rget(globPtr, pro);
    pro.fulfill_result(47);
    

    Where do we prohibit this sort of misusage?

  4. Former user Account Deleted

    rget should spec itself as calling fulfill_result on the promise. Then contracts concerning method call history work for us.

  5. Dan Bonachea reporter
    • changed status to open

    John's new text for rget:

    1327 The second variant notifies completion and the value by fulfilling the promise
    1328 via fulfill_result(value).

    Unfortunately I think there's still a problem here (this is the reason I said "perhaps"). In particular, the spec for fulfill_result(value) includes:

    If the dependency counter reaches zero as a
    1121 result of this call, the associated future is set to ready, and callbacks that are
    1122 waiting on the future are executed on the calling thread before this function
    1123 returns.

    The combination of these seems to imply that rget might run callbacks before it returns, if for example the rget src was local() and thus completed synchronously. However rget was labeled as progress level internal.

    Perhaps we need to clarify somehow that the invocation of fulfill_result(value) is deferred to the next user-level progress?

  6. Former user Account Deleted

    In the progress section text we say that all completion notifications are deferred until user progress, and here we say the notification is invoking fulfill. This chapter should probably have a helpful paragraph introducing completion with a forward ref to progress.

  7. Dan Bonachea reporter

    John - I think this issue may be resolved by your recent commit for a related issue, unless you had in mind to add something additional?

  8. Log in to comment