Questions / comments about p0057r2 coroutines

[alexander.v...@gmail.com]

Hi,

I have a few questions regarding the coroutine proposal p0057r2. I tried to find the rationale behind those things in the paper's predecessors but that is quite a bit of material to read so it's not impossible I missed the reasoning there.

- AFAICS the proposal does not define a life time for the result of coroutine_handle<P>::address(). I agree that address() and from_address() are necessary for interfacing with C functions but it should be well-defined until which point reconstructing a coroutine_handle<P> is still valid.

- The from_promise() machinery seems very easy to misuse (as constructing a coroutine promise yourself and passing it to from_promise() is undefined behaviour) but I understand that it is necessary to implement generators. Would passing a coroutine_handle<P> to get_return_object() not be a much cleaner alternative / was such an approach considered yet?

- There are valid reasons for co_yield (mainly that other programming languages have it) and it unlikely to be changed at this stage but an alternative to co_yield would be a co_this expression that evaluates to a reference (or pointer, for consistency with regular this) to the promise object of the containing coroutine. That way co_yield could be written as co_await co_this->yield() which is only marginally more code and uses a more minimal primitive.

Aside from those minor points I think that p0057r2 is a major step forward towards scaleable stackless coroutines and quite a well-designed proposal that avoids most pitfalls of earlier papers.

Thanks,

Alexander van der Grinten

[Gor Nishanov]

Hi Alexander:

>> functions but it should be well-defined until which point reconstructing a coroutine_handle<P> is still valid.

Library Group has not looked at that part of the wording. They may very well add the wording to that extent. Currently in P0057 whether coroutine handle is usable or not is derived from magical "Requires: *this refers to a suspended coroutine.", does not matter how you obtained the coroutine_handle, as long as it refers to a suspended coroutine you can call resume, done or destroy on it.

>> Would passing a coroutine_handle<P> to get_return_object() not be a much cleaner alternative / was such an approach considered yet?

Yes. This was considered. It was a close call which way to go. Given that having from_promise is allowing for symmetry with untyped promise, potentially less parameters to pass to get_return_object, and that all of these machinery are sharp objects for library writers and not for use by most of the C++ users, there was a slight preference for the current version. Either approach is fine. (With one caveat: current approach was in the hand of customers for several years, another one was never tried, so "fine" is purely a conjecture).

>> expression that evaluates to a reference (or pointer, for consistency with regular this) to the promise object of the containing coroutine. That way co_yield could be written as co_await co_this->yield() which is only marginally more code and uses a more minimal primitive.

We added await_transform in P0057R1, it can be used to do what you are suggesting. You can remove co_yield completely and implement it via await_transform with some yield_t tag type.

Something like:

co_await yield_t(5)

You would specialize await_transform for yield_t to do what currently yield_value does.

You can also have an await_transform for all other types that are not yield_t to result in static_assert, so as to ban any other use of await, apart from with yield_t type.

Cheers,

Gor

[Alexander van der Grinten]

Hello Gor,

thank you for taking the time to answer my comments.

>> Given that having from_promise is allowing for symmetry with untyped promise, [...]

Well, address() and from_address() is not strictly necessary either, right? It can always be replaced by the following pattern:

    void do_something_async(void (*callback) (void *), void *argument);

    /* ... */

    auto operator await(/* ... */) {

        struct awaitable {

            void await_suspend(coroutine_handle<> handle) {

                _handle = handle;

                do_something_async([] (void *argument) {

                    auto self = static_cast<awaitable *>(argument);

                    self->_handle.resume();

                }, this);

                // The awaitable object is alive until the

                // await-expression completes, so this is fine.

            }

            /* ... */

            coroutine_handle<> _handle;

        }

        return awaitable(/* ... */);

    }

This pattern is more general than address() / from_address(): It allows to store additional parameters the callback receives in the awaitable object can return them from await_resume. This enables users to implement non-void await regardless of the underlying promise type; something which is not possible with address() / from_address()! Why does the proposal include an explicit address() / from_address() instead of promoting the use of this store-handle-in-awaitable pattern?

>> We added await_transform in P0057R1, it can be used to do what you are suggesting.

I see, this is indeed a much better idea than what I was suggesting.

Let me ask you another question: Why does the proposal _only_ provide type-erased coroutine_handles? A call to resume() / done() / destroy() results in an indirect jump. I agree that the compiler should be able to devirtualize this jump in many cases; I also agree that having the coroutine_handle of the prosal is important. But why don't we do something like the following:

Define a `suspend-point handle` as a type that has the same interface as coroutine_handle but has weaker guarantees: resume() / done() and destroy() must only be called while the underlying coroutine is suspended in exactly the same execution of the await-expression it was constructed for. Add an implicit conversion from each `suspend-point handle` type to coroutine_handle. Pass a `suspend-point handle` instead of a coroutine_handle to await_suspend().

This way we can write await_suspend like this:

    template<typename H>

    void await_suspend(H handle);

    // This is specialized for each await-expression, so that the compiler can

    // avoid using indirect calls for resume() / done() / destory() here.

    void await_suspend(coroutine_handle<> handle);

    // Still works the same as before. Uses the implicit conversion to construct

    // a coroutine_handle<>.

I feel like doing this has zero downsides. Sure, it is a code size for speed trade-off but it is opt-in so that await operators for commonly used types like std::future would still be able to use the more conservative coroutine_handle. It has a niche for code where the failure to devirtualize an indirect jump would result in a noticeable performance overhead: Consider using await to read from a fast in-memory cache. The fast-path of such a operation might be a single atomic load-acquire (aka a usual non-atomic load on x86_64) and a indirect call would dominate the run time of the whole operation. Also note that devirtualization is a relatively complex optimization and might not be possible in some cases (e.g. when code in a different translation unit is invoked and the compiler is unable to prove that this code does not change the target of the jump (e.g. by resuming the coroutine and suspending again); or just when compiling in debug mode). Using this technique resume() would be so fast (it would always be inlined and often be completely removed by an optimizing compiler) that one could decide to abolish await_ready.

Note than in order to enable the store-handle-in-awaitable pattern from above together with this technique the handle would need to be passed to operator await (and not to await_suspend). This would allow us to convert existing C-style callbacks to await in a very natural way and without using any indirect jumps or address() magic:

    template<typename H>

    auto operator await(std::chrono::seconds t, H handle) {

        struct awaitable {

            awaitable(std::chrono::seconds t, H handle)

            : _t(t), _handle(handle) { }

            void await_suspend() {

                call_in_n_seconds(_t.count(), [] (void *argument) {

                    auto self = static_cast<awaitable *>(argument);

                    self->_handle.resume();

                }, this);

            }

            std::chrono::seconds _t;

            H _handle;

        };

        return awaitable(t, handle);

    }

Many thanks,

Alexander

[Gor Nishanov]

Hi Alexander:

You can go even bolder and design a coroutine model that does not need
coroutine_handle at all.
Imagine that at every suspend point, you are given an object:

h

for which you can call:

h.on_value(T) or h(T)
h.on_error(E)

where T is the result type for that await (may be different for every
await) and E is the error (probably the same E for entire coroutine).

Then you are free to do whatever you want is it.

(Combined with get_return_object getting the same object that you can
store in the promise yourself).

Say, you opt-into this model on top of p0057 by adding
get_coroutine_handle to your promise.

That is a nice model. It needs to be designed and prototyped, but,
otherwise, I can see how C++ coroutines can evolve in that direction.

> --
> You received this message because you are subscribed to a topic in the
> Google Groups "ISO C++ Standard - Future Proposals" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/a/isocpp.org/d/topic/std-proposals/sAzQqQvY6BI/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> std-proposal...@isocpp.org.
> To post to this group, send email to std-pr...@isocpp.org.
> To view this discussion on the web visit
> https://groups.google.com/a/isocpp.org/d/msgid/std-proposals/c2ccd580-126c-42f0-967c-bdb9fc72f890%40isocpp.org.

[Alexander van der Grinten]

Hi Gor,

for now I'm not that interested on what can be built on top of P0057: I really think that P0057 enables powerful abstraction and I think that it is the correct foundation to build on.

At the moment I'm more concerned about how to build abstractions that are as efficient as possible on top of P0057. There are two points that bug me about P0057 in its current form but I think that both can be fixed without changing both the wording or existing prototype implementations too much. I do not want P0057 to change much, after all it has working implementations.

- I think we're missing an optimization opportunity by passing a type-erased type to await_suspend.

- I want to be able to wrap C functions of the form

  void do_something_async(void (*callback) (void *context, int result), void *context)

  as efficiently as possible. The problem here is that I can use from_address() to reconstruct the coroutine_handle but I cannot access the awaiter that needs to hold the int result.

What do you think about the following non-intrusive, hypothetical changes to P0057?

- Change (3.5) in 5.3.8 of the paper to:

  h is an object of an implementation-defined type that is implicitly convertible to and has the same interface as std::coroutine_handle<P> except that resume(), done() and destroy() must only be called until this await-expression is complete.

  

  This does allow (but not force) implementations to pass more specialized types to await_suspend. It does not break existing implementations as passing a coroutine_handle is still allowed.

- Allow access to the awaiter object of a suspended coroutine; maybe overload std::get<T> for coroutine_handle<> so that it returns a reference to the awaiter (and throws if T does not equal its type).

  

  This allows users to store the return value of an async operation in the awaiter object after using from_address(). It does not break code that uses existing prototypes of P0057.

Kind regards,

Alexander

[Gor Nishanov]

Hi Alexander:

The first one, absolutely. I just did not get cycles to try it out and
tweaking the wording to make it legal.

For the second one, "getting an awaiter", can you give me an example
what it allows you to do that you cannot do without it.

> --
> You received this message because you are subscribed to a topic in the
> Google Groups "ISO C++ Standard - Future Proposals" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/a/isocpp.org/d/topic/std-proposals/sAzQqQvY6BI/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> std-proposal...@isocpp.org.
> To post to this group, send email to std-pr...@isocpp.org.
> To view this discussion on the web visit

> https://groups.google.com/a/isocpp.org/d/msgid/std-proposals/c0467b3e-bbdd-4eda-b6d3-b2a7f57417f4%40isocpp.org.

[Alexander van der Grinten]

Hi Gor,

> The first one, absolutely.

Perfect :)

> For the second one, "getting an awaiter", can you give me an example what it allows you to do that you cannot do without it.

There is a large number of C libraries (e.g. libuv, Windows IOCP, Qt signals/slots, the Linux epoll interface) that perform async computations and signal completion either by invoking a callback or by providing some get_completed_events() function. Usually these libraries allow you to pass an arbitrary word (i.e. void *) to the completion code.

Wrapping those functions to an awaitable type currently requires workarounds (like storing the coroutine_handler in the awaiter) because you need *two* words of state: The pointer returned from coroutine_handle<>::address() and a pointer to the awaiter where you want to store the result of the operation.

Consider

    void do_write_async(const void *buffer, size_t size,

        void (*callback) (void *context, int status), void *context);

where 'context' is passed to 'callback' as the first argument.

Lets say I want to wrap it via operator await:

    auto operator await (write_async w) {

        struct awaiter {

            awaiter(write_async w) : _w(w) { }

            

            void await_ready() { return false; }

            int await_resume() { return _status; }

            

            void await_suspend(std::coroutine_handle<> h) {

                do_write_async(_w.buffer, _w.size, [] (void *p, int status) {

                    auto h = std::coroutine_handle<>::from_address(p);

                    

                    // We want to to store status in awaiter::_status, but there is

                    // no way to get a reference to the awaiter!

                    // The suggestion is to allow

                    // std::get<awaiter>(handle)._status = status;

                    

                    h.resume();

                }, h.address());

            }

            

            write_async _w;

            int _status;

        };

        

        return awaiter(w);

    }

[glu...@gmail.com]

Hi Alexander,

Will the following work for your example ?

   auto operator await (write_async w) {

        struct awaiter {

            awaiter(write_async w) : _w(w) { }

            

            void await_ready() { return false; }

            int await_resume() { return _status; }

            

            void await_suspend(std::coroutine_handle<> h) {

                _awaitingHandle = std::move(h);

                do_write_async(_w.buffer, _w.size, resumeCb, this);

            }

            static void resumeCb(void* ptr, int status) {

                reinterpret_cast<awaiter*>(ptr)->resume(status);
            }

            void resume(int status) {

                _status = status;

                _awaitingHandle.resume();

            }

            

            write_async _w;

            int _status;

            std::coroutine_handle<> _awaitingHandle;

        };

        

        return awaiter(w);

    }

Thanks,

Andrii

середа, 20 липня 2016 р. 00:18:03 UTC-7 користувач Alexander van der Grinten написав:

[Alexander van der Grinten]

> Will the following work for your example ?

That does work but I consider it a clunky workaround.

* It defeats the purpose of from_address(). The *only* use of from_address() is wrapping C functions. The from_address() mechanism is of questionale usefulness if it can only wrap C functions do not return async results.

* If storing a coroutine_handle<> in the awaiter is common practice then coroutine_handle<> should be passed to the constructor of the awaiter and not to its member function. That however is a more invasive change and would render current prototypes non-conforming; providing a way to access the awaiter does not.

* If non-type-erased handles were implemented this workaround would yield suboptimal performance as it requires two indirect jumps instead of one in that situation.

[Alexander van der Grinten]

>If non-type-erased handles were implemented this workaround would yield suboptimal performance as it requires two indirect jumps instead of one in that situation.

Thinking about it the pattern works even in this case if one does not store the handle in the awaiter but the address(). So basically all boils down to:

Does a more straightforward way to wrap C functions justify the implementation of an awaiter accessor function?

[Gor Nishanov]

I am not sure I understand your point here. You don't need two indirect jumps.

You wanted a non-type erased coroutine_handle, so that you can
synthesize a callback function that will perform a direct call to
resume(). Well, just pass an address of an awaiter as a context and
store coroutine_handle<> or void* in your awaiter.

Modifying slightly Andrii's example, gets you what you want:

auto operator await (write_async w) {
struct awaiter {
awaiter(write_async w) : _w(w) { }

void await_ready() { return false; }
int await_resume() { return _status; }

template <typename Promise>
void await_suspend(std::coroutine_handle<Promise> h) {
_awaitingHandle = h.address();
do_write_async(_w.buffer, _w.size, [](void* ptr, int status) {
auto me = reinterpret_cast<awaiter*>(ptr);
me->_status = status;

std::coroutine_handle<Promise>::from_address(me->_awaitingHandle);
},
this);
}

write_async _w;
int _status;
void* _awaitingHandle;
};

return awaiter(w);

> --
> You received this message because you are subscribed to a topic in the
> Google Groups "ISO C++ Standard - Future Proposals" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/a/isocpp.org/d/topic/std-proposals/sAzQqQvY6BI/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> std-proposal...@isocpp.org.
> To post to this group, send email to std-pr...@isocpp.org.
> To view this discussion on the web visit

> https://groups.google.com/a/isocpp.org/d/msgid/std-proposals/dcc846ed-46cf-44bf-95e0-7d48576bcdf4%40isocpp.org.

[Alexander van der Grinten]

> Modifying slightly Andrii's example, gets you what you want

Yes, you're right. I overlooked that possibility. That makes "getting an awaiter" a luxury feature that arguably should not be added to the proposal.

Thanks again for your comments. I'm looking forward to see if non-type-erased handles make it to a future revision of the paper :)