Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
lewissbaker opened this issue Aug 14, 2017 · 2 comments
Assignees

Comments

@lewissbaker
Copy link
Owner

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
Copy link
Owner Author

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 lewissbaker self-assigned this Aug 17, 2017
@lewissbaker
Copy link
Owner Author

Commit 316ce6c 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 ;)

Garcia6l20 pushed a commit to Garcia6l20/cppcoro that referenced this issue Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant