Verify and specify copy/move requirements on callbacks

[Dan Bonachea]

Currently section 6.5 talks about in very abstract terms about what we require from FunctionObjects passed to RPC, LPC and future::then callbacks.

We currently fail to specify that the FunctionObject must be MoveConstructible, but IIUC the implementation is often relying upon that property. Hopefully it is not also relying on CopyConstruction, but it would be nice to have a test verifying that.

We should determine what the copy/move requirements are for FunctionObjects in our implementation and update the spec accordingly.

[Dan Bonachea]

Additional notes:

1. The LPC semantics explicitly say it moves the function object, but does not include MoveConstruction in the preconditions.

2. The future::then case is a bit worse because there we never serialize Func (and Serializable is not a requirement) so I think we semantically must rely on move/copying the function object in order to schedule it for later execution. The spec declaration mandates a by-value call, but this doesn't match the implementation of future1::then() which is using a universal reference and appears to be moving the FunctionObject

[Amir Kamil]

as_lpc and as_rpc require their arguments to be CopyConstructible.

The implementation of as_rpc does not match the spec, however – it takes the FunctionObject by universal reference rather than by value. My opinion is that the implementation is correct and that the spec should be changed.

For as_lpc, both the implementation and spec take the FunctionObject by value.

[Amir Kamil]

future::then passes the argument along by universal reference until it gets down to the depths of future_impl_then_lazy, where it is moved (if its an rvalue) or copied (if its an lvalue) to internal storage.

as_lpc and lpc take the argument by value and then move it.

@john bachan will probably have to remind us why the pattern of taking an argument by value (incurring a copy or move) and then moving it into internal storage (as is done for as_lpc and lpc) is preferred over taking it as a universal reference and incurring just a single copy/move at the end.

[john bachan]

I no longer can find the "reply to" button under comments... what the?

Anyway, in reply to @Amir Kamil shoutout to me just above, passing by value is only advantageous for cases like make_future<T> where the user may want to explicitly pass a reference within T. I'm pretty sure any other occurrence of pass by value is lazyness on my part. In the case of lpc I could see this lazyness saving me from typing out a std::decay<Fn>::type to get at the underlying value type which is used to instantiate lpc_impl class.

[Amir Kamil]

Setting aside the signatures, here’s what I think the right semantics are:

• as_lpc and as_rpc require their arguments to be CopyConstructible, so that the resulting completion object is CopyConstructible. This is what is currently in the spec.
• persona::lpc and future::then require argument objects passed as lvalues to be CopyConstructible, those passed as rvalues to be MoveConstructible. (The spec will have to be careful to talk about the constructibility requirement on the referent rather than on the reference for arguments that are passed by universal reference.)

• rpc and rpc_ff require argument objects passed as lvalues to be CopyConstructible, those passed as rvalues to be MoveConstructible. The same requirement applies to the object returned by the rpc callback.

  • This is stricter than the implementation (pending impl PR 270), but I don’t think we should make the optimizations we do a requirement for a conformant implementation.

‌

[Dan Bonachea]

rpc, rpc_ff

I don't like the idea of requiring CopyConstructible for rpc(_ff) lvalue input arguments. Amir has worked hard to eliminate all the copies along that path, and I think users should be permitted to rely on that. Otherwise we'd either be static_asserting specified properties the implementation doesn't actually need (making user's lives more difficult for no reason), or not static_asserting properties required by the spec (silently accepting non-compliant programs and reserving the right to break them later); neither of these options seems to be in our user's best interest.

I don't see copy-free RPC injection solely as an "optimization", but more importantly as correcting a semantic defect - we've replaced aberrant/unnecessary use of C++ copy constructors along the RPC path with direct Serialization, which is our own special form of copy that is designed exactly for the case of data passed across the RPC interface.

Similarly on the RPC return path, IMO Copyability of the return type T should not be a requirement unless the RPC callback also takes some action that forces a copy; for example, invoking make_future<T>() to copy a T by-value into a future, or returning by-value where RVO copy-elision is not guaranteed (such as an returning by-value an argument that was accepted by reference). That's the user code imposing the Copyability requirement on itself, and not something we need to specify.

lpc, as_lpc and future::then

lpc, as_lpc and future::then are a different semantic category - there Serialization does not apply and we really do need to copy lvalue inputs in order to preserve the argument value for later. AFAIK there's no way around that requirement without imposing klunky/error-prone lifetime requirements. I'm less sure about the lpc and future::then return paths - I think we could possibly handle non-copyable types there (although probably currently do not), except again for cases where the user forces a copy (such as the callback returning a future<T>).

as_rpc

as_rpc seems debatable - the API reference requires CopyConstruction of the Func and Args.. (and I'm assuming our implementation currently relies upon this), but I don't see any fundamental reason to require this. 7.2.1-8 requires lvalue inputs to remain valid until source completion, so nothing prevents us from stashing an lvalue reference in the completion object (instead of copying the value) and serializing it later. We also have the freedom to serialize the arguments immediately when the as_rpc completion is constructed and hold a serialized buffer (possibly as a ref-counted heap object) for later use. Either approach could still yield a copyable completion object

[Amir Kamil]

Agree that as_rpc and rpc[_ff] do not require CopyConstructibility on lvalue arguments. I think they should continue to impose MoveConstructibility on rvalue arguments, regardless of whether the implementation requires it.

[Amir Kamil]

@Dan Bonachea pointed out that as_lpc and as_rpc require arguments that are not stored by reference to be CopyConstructible, so that the resulting completion object is CopyConstructible. This means that as_lpc arguments must be CopyConstructible, and non-lvalue arguments to as_rpc must also be CopyConstructible.

[Amir Kamil]

Specify constructibility requirements in then, lpc[ff], rpc[_ff], and as. Fixes #168.

→ <<cset 2929fe61dcfe>>

[Amir Kamil]

Merged in akamil/upcxx-spec/issue168 (pull request #57)

Specify constructibility requirements in then, lpc[ff], rpc[_ff], and as. Fixes #168.

Approved-by: Dan Bonachea [e-mail]

→ <<cset b47630de8c62>>