Re: [Async] Second attempt at review of proposed Boost.Async begins

From: Mohammad Nejati [ashtum] (ashtumashtum_at_[hidden])
Date: 2023-10-01 08:06:32

Hi everyone,

This is my review of the proposed Boost.Async:

> Does this library bring real benefit to C++ developers for real-world use cases?

Considering the current support for C++ coroutines in the standard and
the absence of networking support in the foreseeable future, this
library can serve real-world users for many years. I find the author's
choice to build on top of Asio to be a smart decision, as it enables
the effortless utilization of multiple other high-quality networking
libraries in the field.

The author has chosen a single-threaded design, and I must say I agree
with this approach for several reasons:

1. It leads to a simpler and more efficient implementation,
eliminating the need for mutexes and atomic operations for
synchronization.
2. Writing multithreaded asynchronous code is a complex task, and it
becomes even more challenging in coroutine functions due to their
ability to suspend in the middle.
3. Most of the time, the best way to leverage multithreading is by
offloading heavy computations to other threads, a possibility offered
by this design through concurrent channels (share memory by
communicating, not communicate by sharing memory).
4. Users can be confident that they won't encounter race conditions
during development while still being able to write concurrent code.

The library capitalizes on two fundamental optimization opportunities
in C++ coroutines that cannot be achieved in Asio:

1. It prevents unnecessary suspension and rescheduling through the use
of the `await_ready` customization point. This is particularly
valuable when developing high-level coroutine functions that
occasionally need to suspend but can often return results without
performing async operations. For example:
async::promise<Result> online_users() {
if (is_cached_)
co_return online_users_;
// asynchronously fetch online users
}
By utilizing `await_ready`, we can eliminate unnecessary rescheduling.
2. It utilizes symmetric-transfer to efficiently resume the next
coroutine without involving a scheduler. Asio requires resuming the
next coroutine handle on a scheduler to prevent stack exhaustion,
which results in significant indirection and branches. However, with
symmetric-transfer, this can be accomplished with a fraction of that
overhead.

> Do you have an application for this library?

I've used C++ coroutines in every Asio-based application I've worked
on in the last three years. The main issue I've encountered is the
lack of asynchronous synchronization primitives such as join, select,
wait_group, and condition variables, which I believe Boost.Async has
addressed to some extent.

> Does the API match current best practices?

I couldn't find a complete set of asynchronous synchronization
primitives, such as semaphores, barriers, condition variables,
mutexes, and futures/promises. While some of these primitives can be
simulated using channels, I couldn't find any examples of this in the
documentation.

> Is the documentation helpful and clear?

I believe the documentation is rather clear for those who already know
how to use Asio. I would recommend the author to start by explaining
how this library is related to Asio and what gaps it tries to fill.
Another issue I have encountered is the lack of inline documentation
in header files, which means users need to switch to their web browser
for even basic information that could have been accessible directly
within their IDE. Furthermore, the documentation lacks details
explaining how `interrupt_await` works and snippet examples for
`wait_group`, `with`, `select`, `gather`, `join`, and `thread`.

> Did you try to use it? What problems or surprises did you encounter?

I have only attempted to experiment with the provided examples and
didn't encounter any issues in the process.

> What is your evaluation of the implementation?

I have skimmed over the implementation, and I find the use of
`await_ready` and `symmetric-transfer` in coroutines to be effective.
I also like the idea of introducing the `interrupt_await` concept to
enable a lossless select.
I found the use of std::mt19937 in select and wait_group a bit
excessive; it might be possible to be replaced with a more lightweight
option like PCG or something similar with a smaller state.

> Are you knowledgeable about the subject?

I have some experience in working on asynchronous networking libraries
that conform to Asio's universal asynchronous model (psql and smpp)
and scheduler-aware synchronization primitives in Asio (future/promise
pairs and oneshot channel). I have some experience with using C++
coroutines in real-life applications.

> How much time did you spend evaluating the library?

I spent approximately 12 hours reading documentation and assessing the
implementation, and an additional 4 hours experimenting with the
provided examples.

> Please explicitly state that you either accept or reject the inclusion of this library into Boost.

I believe this is a useful library that can cover 90% of ASIO-based
applications with a much simpler and more intuitive interface. I
recommend conditional acceptance of Boost.Async into Boost, provided
the following issues are addressed:

1. Add the mentioned missing synchronization primitives or demonstrate
in the documentation how the same can be achieved using a channel.
2. Revise the documentation to cater to users who may have little
knowledge of ASIO.

I would like to mention that both Klemens and I are associated with
the C++ Alliance.

Thanks, Niall, for managing this review, and Klemens for submitting it.

Regards,
Mohammad

On Fri, Sep 22, 2023 at 4:03 PM Niall Douglas via Boost
<boost_at_[hidden]> wrote:
>
> Dear Boost,
>
> Due to too few reviews last time, we are going to try again the review
> of this library. This review shall run between Friday 22nd September and
> Monday 2nd October. Please do consider contributing a review!
>
> My thanks to Marcelo Zimbres Silva for the only formal review last time,
> and to the others including those on Reddit /r/cpp and by private email
> who submitted feedback. All your reviews and feedback have been retained
> and will be reused for this review.
>
> To remind you all, Boost.Async is a C++ coroutine programming library
> based around Boost.ASIO as the executor, with a unique set of properties
> which make it incomparable to most existing C++ coroutine programming
> libraries. To be specific, it lets you compose arbitrary third party
> awaitables without any additional effort, which lets you tie together
> third party libraries with ease and convenience. Changes to proposed
> Boost.Async since the last review:
>
> 1. Major doc improvements & bug fixes
> 2. PMR is optional
> 3. optimized associated allocator for use_op
> 4. lazy generator support
> 5. public unique_handle & more examples with custom awaitables
> 6. as_result/as_tuple support (akin to the asio stuff, but takes an
> awaitables, i.e. co_await as_result(foo())) - reduces unnecessary
> exceptions.
> 7. with supports return values.
>
>
> You can read the documentation at https://klemens.dev/async/ and study
> or try out the code at https://github.com/klemens-morgenstern/async.
>
> Anyone with experience using C++ coroutines is welcome to contribute a
> review at the Boost mailing list
> (https://lists.boost.org/mailman/listinfo.cgi/boost), at /r/cpp on
> Reddit, via email to me personally, or any other mechanism where I the
> review manager will see it. In your review please state at the end
> whether you recommend acceptance, acceptance with conditions, or
> rejection. Please state your experience with C++ coroutines and ASIO in
> your review, and how many hours you spent on the review.
>
> Thanks in advance for your time and reviews!
>
> Niall
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost