lewissbaker / cppcoro

A library of C++ coroutine abstractions for the coroutines TS
MIT License
3.43k stars 472 forks source link

Make lazy_task safe to use in a loop when the task completes synchronously #31

Closed lewissbaker closed 7 years ago

lewissbaker commented 7 years ago

The lazy_task implementation currently unconditionally suspends the awaiter before starting execution of the task and then unconditionally resumes the awaiter when it reaches the final_suspend point.

This approach means that we don't need any synchronisation (ie. std::atomic usages) to coordinate awaiter and task.

However, it has the downside that if the lazy_task completes synchronously then the awaiter is recursively resumed. This can potentially consume a little bit of stack space every time a coroutine awaits the a lazy_task that completes synchronously if the compiler is not able to perform tail-call optimisation on the calls to void await_suspend() and void coroutine_handle<>::resume() (note that MSVC is not currently able to perform this tail-call optimisation and Clang only does this under optimised builds).

If the coroutine is awaiting lazy_task values in a loop and can possibly have a large number of these tasks complete synchronously then this could lead to stack-overflow.

eg.

lazy_task<int> async_one() { co_return 1; }

lazy_task<int> sum()
{
  int result = 0;
  for (int i = 0; i < 1'000'000; ++i)
  {
    result += co_await async_one();
  }
  co_return result;
}

The lazy_task implementation needs to be modified to not recursively resume the awaiter in the case that it completes synchronously. This, unfortunately means it's going to need std::atomic to decide the race between the awaiter suspending and the task completing.

lewissbaker commented 7 years ago

Note that @GorNishanov's https://github.com/GorNishanov/clang/tree/tailcall branch contains an experimental implementation of Clang that supports a new kind of await_suspend() overload that returns a coroutine_handle<> which can potentially allow guaranteed tail-call recursion.

See https://github.com/lewissbaker/cppcoro/blob/tailcall/include/cppcoro/lazy_task.hpp for an implementation of lazy_task that uses this new await_suspend() overload.

lewissbaker commented 7 years ago

Commit 316ce6c7dd40c1e78f65f60d886db1eb8cc570dd has made the change to use a synchronous-completion-safe implementation that uses atomics.

I'll reinstate a more efficient implementation once we have coroutine_handle-returning await_suspend() available in both MSVC and Clang (assuming it passes muster at the C++ standards meetings ;)