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

[Niall Douglas via Boost]

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

[Ruben Perez via Boost]

On Fri, 22 Sept 2023 at 14:33, Niall Douglas via Boost

<bo...@lists.boost.org> 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!
>

Since my last review (see
https://lists.boost.org/Archives/boost/2023/08/254921.php
and https://lists.boost.org/Archives/boost/2023/08/254929.php), the
author has addressed
the issues I raised, so my recommendation would be to ACCEPT the library
into Boost.

As I said last time, both Klemens and I are affiliated with the C++ alliance, so
feel free to not consider the vote if deemed necessary.

Regards,
Ruben.

[Robert Ramey via Boost]

The question of not enough reviews to proposed Boost Library submissions
was addressed by the boost library incubator. Sadly, I wasn't able to
get enough traction for the idea. (One of many failed endeavors in my
life!). More information here:

https://akrzemi1.wordpress.com/2014/11/19/boost-library-incubator/

As regards the problem of not enough reviews, it permitted anyone to
make a review of any library at any time before the oficial review
period. That is when the user was actually evaluating the library for
possible usage. Hopefully by the time the official review period came
around, there would be enough reviews already accumulated - thus making
the review process easier and more complete.

Also see: https://github.com/robertramey/Blincubator

Robert Ramey

[Mohammad Nejati [ashtum] via Boost]

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

[Niall Douglas via Boost]

On 01/10/2023 09:06, Mohammad Nejati [ashtum] wrote:
> Hi everyone,
>
> This is my review of the proposed Boost.Async:

Thanks very much for this thorough review.

A reminder to everybody that the review period ends tomorrow (Monday).