executors / futures

A proposal for a futures programming model for ISO C++
22 stars 7 forks source link

Support passing executors to future continuations #45

Open yfeldblum opened 6 years ago

yfeldblum commented 6 years ago

We may have cases where, from the outside, we want to schedule work to be done on any thread in a thread pool but not necessarily on one that can be chosen immediately; but from inside that work, we want to schedule additional work to be done on the same thread in that thread pool.

Some reasons we may want this are

// boilerplate
class A; // not thread-safe
class B; // not thread-safe
ThreadPoolExecutor tpe = /* ... */;
SomeOtherExecutor soe = /* ... */;
size_t iters = /* ... */;

SemiFuture<std::shared_ptr<A>> f = /* ... */;
thread_local B tlobj;

// we don't care which thread eventually picks up the task
// but as a constraint, we cannot select the thread yet
// thread selection (internally in tpe) must wait until f becomes ready
f.via(tpe).then([soe, iters](shared_ptr<A> value, ThreadExecutor te) {
  std::vector<Future<size_t, ThreadExecutor>> fs;
  for (size_t i = 0; i < iters; ++i) {
    fs.push_back(
      make_ready_future()
        .via(soe).then([] {
          // intermediate step executes anywhere
        })

        // having te be made available lets us do this:
        .via(te).then([&tlobj, value, i] {
          // back to the same thread
          value->manipulate(tlobj); // not thread-safe!!!
          return i;
        }));

  }
  return std::when_all(fs.begin(), fs.end());
});
dhollman commented 6 years ago

That's totally reasonable, but that's not currently how executors work; you can't schedule work on one executor and have it actually run on another one under the current model for P0443, so it's not really a motivation to pass the executor as an argument. You'd have to do something like:

f.via(tpe).then([soe, iters, tpe](shared_ptr<A> value) {
  ThreadExecutor te = execution::query(tpe, current_thread_executor);
  /* ... */
});

where current_thread_executor is a property that you've defined to work with the type of tpe any executor you want to do this on in a generic context.

Given this, I'm pretty sure that what you're suggesting can be done completely additively with the helper functions approach (e.g., #48, #49), so it seems pretty reasonable IMHO to leave it out of version 1 of the paper.

LeeHowes commented 6 years ago

I think Jay's suggestion is the cleanest approach here because it exposes more precise information directly where it is needed. The point is not to capture an executor, it's for a task to be able to query information about the executor it is running on. Which means that if the parent executor chooses a subexecutor to run work on, which strikes me as a reasonable implementation decision and I don't see where P0443 blocks me from doing that, then it would pass that to the continuation, and we can construct a task to run on that executor directly.

On the other hand, it probably isn't urgent for V1.

hkaiser commented 6 years ago

I think this is another case where one might want to directly pass an executor to .then() (as discussed before):

f..then(tpe, [soe, iters](shared_ptr<A> value) {
  /* ... */
});

Compared to .via() (which makes the executor 'sticky'), this would associate an executor for just this continuation.

brycelelbach commented 6 years ago

Note that being able to have a future passed to the continuation instead of a value or exception doesn't solve this; you can get the executor from that future, but that's not necessarily "YOUR* executor, it's your predecessors.

LeeHowes commented 6 years ago

That is one reason why we were in favour of the ready future design, yes. It allows you to encapsulate this kind of information in the parameter package sent to the continuation and do it in a very consistent way.

Sending any future to the continuation means you need to think about getability etc, so that isn't viable unless we are willing to take the risk on blocking operations on all futures again.

dhollman commented 6 years ago

@hkaiser Can your concern be addressed by just capturing the executor?

f.then(tpe, [tpe, soe, iters](shared_ptr<A> value) {
  /* ... */
});
hkaiser commented 6 years ago

Passing the executor directly to .then (or equivalent) makes implementing it trivial:

template <typename T>
template <typename Exec, typename F>
auto future<T>::then(Exec && exec, F && f)
{
    return exec.then_execute(std::move(*this), std::forward<F>(f));
}

(or similar). How can something equivalent be implemented using your suggestion?

dhollman commented 6 years ago

Because in our current system, exec must be a member of future before you can call then. Essentially:

template <typename T, typename Exec>
template <typename F>
auto continuable_future<T, Exec>::then(F && f)
{
    return exec_.then_execute(std::move(*this), std::forward<F>(f));
}

But I don't see how this has anything to do with passing the executor as an argument to the continuation function itself, which is what this issue is about AFAICT. What am I missing?

hkaiser commented 6 years ago

Correct, I understand. However, in 'your system' (as you put it) the executor is 'sticky' to the future which might not always be what you want. Sometimes you just need to use a specific executor for one continuation only, in which case passing it directly to .then might be the most convenient thing to do.

Sticking the executor into the capture of a lambda does not help as you have no way of accessing it from inside your implementation of .then such that you can use it to schedule the continuation.

Also, using .via for this would require an additional allocation (you create a new shared state after all) which might not be desirable. It would also be unnecessarily inconvenient to have to go through .via just for attaching a single continuation to be run on a given executor.

I personally almost find it to be 'over-designed' to have to go through .via in the first place, but this is a separate discussion.

BTW, your code above would expose UB as you're moving *this in the same context as you're using one of the members (exec_) of the moved object. Working around this might require even more allocations...

jaredhoberock commented 6 years ago

However, in 'your system' (as you put it) the executor is 'sticky' to the future which might not always be what you want. Sometimes you just need to use a specific executor for one continuation only, in which case passing it directly to .then might be the most convenient thing to do.

This concern has occurred to me as well. Sometimes it would be convenient to be able to insert an executor "inline" to .then().

What's tricky about this is that .then() is a convenience interface that it is intended to be user facing and pleasant to use. On the other hand .then() is also a "requirements" interface that future authors have to implement to conform to Future requirements. In this setting, convenience features are a nuisance to authors who just want to implement the basic requirements for conformance.

dhollman commented 6 years ago

This is the reason we introduced FutureContinuation helper functions. .then() itself isn't really a user-facing abstraction; rather the helpers plus .then() make up the user-facing abstraction (with the exception that the value-only callable version can be shortcut since it's directly compatible with FutureContinuation). To handle Hartmut's case, we'd need to attach a trait or some other indicator to the FutureContinuation concept that says "call me with the executor" and then provide a helper function that adapts a callable and sets this trait, but the beauty of this is that it can be done entirely additively if and when we want it.

dhollman commented 6 years ago

Also, using .via for this would require an additional allocation (you create a new shared state after all)

I don't see how this follows. Either the two executors have compatible shared state, and no copy is made, or they don't, and there's nothing you can do about it no matter how you spell it.

It would also be unnecessarily inconvenient to have to go through .via just for attaching a single continuation to be run on a given executor.

But that's exactly the point: if the two executors have some special knowledge of each other that allows them to interoperate without copies or extra allocation, they can provide an overload of .then() on their futures that takes the other executor. But if you have to generic executors, the general hand-off process can be quite expensive, no matter how you phrase it. This "unnecessary inconvenience" is actually a good thing, in keeping with the design principle of make expensive things look expensive. In other words, an executor overload to .then() is fine in code where you know the two executors are compatible, but in generic code it should be obvious that the transition is potentially expensive.

dhollman commented 6 years ago

BTW, your code above would expose UB as you're moving *this in the same context as you're using one of the members (exec_) of the moved object. Working around this might require even more allocations...

Yeah, it should probably be something like:

template <typename T, typename Exec>
template <typename F>
auto continuable_future<T, Exec>::then(F && f)
{
    return Exec{exec_}.then_execute(std::move(*this), std::forward<F>(f));
}

which then allows the compiler to elide the copy to a temporary or something like that. Remember, executors are lightweight handles in the P0443 model, and a copy should amount to little more than a reference count increment or something similar, which a compiler should be able to elide (at least under certain memory models) if it can see the corresponding decrement.