Subject: Re: [boost] [thread] Alternate future implementation and future islands.
From: Jeremy Maitin-Shepard (jeremy_at_[hidden])
Date: 2015-03-18 00:08:42
Very nice work!
This is pretty much _the_ way to implement real composable futures, right?
A few comments:
1. Since shared_state_union<T> is basically expected<T>, it could be nice to
unify them explicitly. The atomic option would also be useful for
expected<T>, I would think.
2. I noticed a number of bugs in the implementation of future::then and
a. future::then throws if the state is ready and f throws, but instead
should just return a ready future with the exception. More generally,
calling a function with some arguments, and setting a
promise/shared_state_union<T>/expected<T> either with the result or the
exception is a useful operation to expose directly to users. This could
then be used by both future::then as well has async, for instance.
b. future::then and future::next should probably consistently release
ownership of their state even if the state is already ready. future::then
also has to be fixed to call f with an rvalue future representing this,
rather than an lvalue; currently, calling future::then with a standard
"then" function declared as taking a future by value wouldn't compile. To
ensure that the state is consistently released, you could use
f(future(std::move(*this))), or perhaps better yet use a local variable to
be robust to move elision potentially allowed by a future c++ standard.
Additionally the return type needs to be the appropriate future type based
on the return type of the function, rather than assuming it is the same as
the current future type.
3. Since invoking future::then and future::next release ownership, it is
worth considering making them only work on rvalues. Whether the extra
compile-time error checking this buys is worth the added verbosity of having
to add std::move in some cases is not clear, though.
4. In some contexts a thread-unsafe future/event that saves the cost of
atomic operations would be useful (unless it turns out that this cost is
negligible on modern CPUs, which I don't think is the case). However, it
would be important for the thread-safe and thread-unsafe futures/events to
interoperate in a reasonable way. (I'm not sure exactly what reasonable
semantics would be.) Obviously there would have to be some caveats, but we
would want to avoid another future-island situation.
I strongly encourage you to continue this work, and see about getting this
incorporated into boost::future and possibly std::future, as it seems to be
a cleaner and leaner abstraction than what is currently done.
Benchmarks (and unit tests) would also be particularly helpful.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk