Re: [Async] Review from Janko Dedic

From: Peter Turcan (peterturcan_at_[hidden])
Date: 2023-10-02 19:13:32

Here is my review of the Boost.Async documentation. I have not tested the
library, but just reviewed the docs from a technical writing perspective:

*Boost.async Documentation review*
*=============================*

*Overview*

The overview should be a narrative, answering the WHY question: "why should
a C++ developer be interested in this library?". The why answer should
cover the scenarios that this library addresses - from a scenario rather
than technology point of view. In other words, what problems do I have as a
developer that this library is the answer to. It is OK to add short
definitions of key terms - such as coroutine - as this is an opportunity to
explain any nuances in the interpretation of these terms that are unique or
particular to this library. Don't worry about insulting the intelligence of
the reader - the overview needs to paint the big picture and be relevant
for all levels of expertise that might use the library.

Good to see lots of linking, and the name of the author and version number.

Be consistent with capitalizations - table headings of "Coroutine types"
and "Synchronization Functions" - either start all with caps or all just
first word cap.

Code examples are good, but there is not a "copy" button for each sample
that will copy to the clipboard? Such a feature adds usability.

*Motivation*

The English is a little clunky "It based on boost.asio" - should for
example be "It is based on boost.asio". Or perhaps a rewrite such as "This
library, based on boost.asio, provides single threaded asynchronicity that
works well with existing libraries such as boost.beast, boost.mysql and
boost.redis."  Words like "simple", "easy" and "very" add almost no meaning
and should be avoided. Not sure it is worth repeating similarities to
node.js or python - just mention once perhaps.

Do not use "&" instead of "and" in documentation. English throughout the
docs could use a gentle edit pass, for grammar and consistency.

Use *second *person in docs, to engage the reader more, for example: not
"this allows to write applications....." but "this allows *you *to write
applications....".

*Coroutine Primer*

Don't run two headings together, this is an issue throughout the
documentation. The first heading "Coroutine Primer" should have an
introductory paragraph (just a single sentence might suffice) that explains
what the section is about, and in particular what the section provides to
the reader.

*Async programming*
I like that this section is trying to explain the technology. No need for
"This library(boost.async) provides ...." just say "Boost.async
provides..." as this removes unnecessary words as is direct.

It is a matter of style, but I much prefer "For example..." over "E.g." it
is just easier to read smoothly, and similarly avoid "i.e." - replace this
with "that is" or appropriate wording.

The Stackless definition is a little weak - but like the link to the full
definition. Consider not attempting partial definitions, but instead add a
note such as:
Note:
Coroutines are *Stackless*.  [with Stackless as a link to a full definition]

Awaitables
It is best to define technology with a purpose, not as part of other
technology. So introduce Awaitables with their purpose. For example:

*An "awaitable" function is designed to work with the co-routine mechanism,
allowing for asynchronous execution. When called, instead of running to
completion, it returns control back to the caller until a specific point,
marked by the `co_await` keyword, is reached. The function then pauses its
execution, allowing other operations to run, and resumes only when the
awaited condition is satisfied, making it a powerful tool for non-blocking
and concurrent programming scenarios, especially in contexts like I/O
operations or task scheduling.*

Similarly for Event Loops and any other technology term particular to async

Like the Previous, Up, Next links - aids navigation

*Tour*

Needs an intro paragraph.

Perhaps an intro paragraph to Promises, Tasks, Generator etc. explaining
their purposes - within the context of the library.

*Tutorial*

Intro paragraph that explains what the student will learn from the
tutorial.  "By completing this tutorial, you will understand what coding
steps are necessary to  ....".

Perhaps the tutorial should have numbered Steps to follow (1, 2, 3....)
where the steps are active - do this, do that etc.

Like the numbered comments though - though they could be added as comments
to the code - either way they are helpful.

Again, be consistent with headings - use either sentence caps ("Advanced
examples") or Title caps ("Advanced Examples") consistently. I prefer Title
caps.

Good to see many more examples, listed as "Advanced examples". Could
perhaps do with some general usage information - such as Create a Project,
copy and paste the code into which file of the project, etc.

*Design*

Needs an intro para.

Title capitalization consistency

*Reference*

Intro paragraph

Not sure why some code is in bold red?

Would have liked to see more tables - tables of properties, parameters,
options, class methods etc. - adds structure.

*Technical Background*

Intro para - what does this section contain?

Adding diagrams helps - perhaps a little more explanation of the flow of
control?

*Benchmarks*

Intro Para

=======================

Hope this is all clear

- Peter Turcan

On Mon, Oct 2, 2023 at 10:28 AM Niall Douglas via Boost <
boost_at_[hidden]> wrote:

> This was sent to me privately with an express wish it be posted here. I
> don't know if the author is subscribed here, so I don't know if replies
> will be seen.
>
> This is the first REJECT review.
>
> Niall
>
> --- cut ---
>
> What follows is my review of the proposed Boost.Async library.
>
>  >Please state your experience with C++ coroutines and ASIO in your
> review, and how many hours you spent on the review.
>
> I have considerable experience with C++ coroutines as I've been
> implementing my own coroutine types in the past. I have used Asio before
> in a professional setting, although I wouldn't call myself an Asio
> expert. My experience involved using Beast/Asio to perform HTTP requests
> and sending fan-out notifications to external subscribers. Notably, I
> have a lot of experience with asynchronous programming in other
> programming languages, mainly Java, Kotlin and JavaScript, but I'm also
> very familiar with how it works in Go and Rust. I would say I've spent
> around 10 hours evaluating the proposed library.
>
> When I first saw this library being proposed, I was hoping it would
> provide a more modern, capable and ergonomic version of asynchronous
> programming, like other modern programming languagues and unlike Asio.
> The goal of this library is seemingly not to provide such an
> out-of-the-box seamless solution, but very specifically to provide
> certain coroutine-related utilities and enable composition of different
> coroutine types, including user-defined coroutine types (unlike
> `asio::awaitable`).
>
> However, I don't think either characterization is entirely correct, and
> Boost.Async finds itself in an awkward middle. On one hand, it does not
> build on top of Asio to provide a modern and seamless experience which
> would completely replace the direct usage of Asio (which is exposed
> front-and-center in the interface and code using this library has to
> also use Asio a lot to get the job done). On the other hand, it's also
> not a completely generic "coroutine library" that only provides
> abstractions based on C++ coroutines that could be used by any library
> (because it depends on Asio specifically).
>
> In my view, the correct way to characterize this library would be to
> regard it as a bundle of coroutine-related utilities on top of Asio.
> However, the library is evidently not entirely compatible with any kind
> of Asio usage as it's focused on "simple single threaded
> asynchronicity." It can certainly be used in conjunction with Asio, but
> it definitely brings some additional nuance and complexity (which I
> don't think is good, considering that Asio has a lot of complexity of
> its own).
>
> When it comes to added functionality, there are definitely some welcome
> additions in the form of `async::with` for asynchronous clean-up,
> `async::select`, generators. What mainly stands out to me is channels,
> which are a very important bread-and-butter feature for many languages.
> However, we should keep in mind that Asio has some experimental support
> for channels as well, and users of this library would probably be faced
> with an unnecessary choice sometime in the future.
>
> One of the most advertised features is the ability to "compose arbitrary
> third party awaitables without any additional effort, which lets you tie
> together third party libraries with ease and convenience". Firstly, I
> don't think this is a goal worth pursuing. As a user, I would definitely
> want as few coroutine types as possible in my codebase (ideally one).
> `asio::awaitable` already nicely fits the bill. I very well understand
> that we need at least a few more for sync/async generators, but that's
> where I'd draw the line.
>
> Secondly, this library does not exactly fulfill the promise of "tying
> together third-party libraries with ease". Boost.Async does not work
> with any coroutine library out of the box. For example, my coroutine
> types are incompatible with it and the code doesn't compile. Same goes
> for `asio::awaitable`. There is definitely some integration effort
> necessary to wrangle these distinct types to work together.
>
> Related to this, I also dislike exposing the whole C++ coroutine
> machinery as some kind of a first-class "standard" API for integrating
> different coroutine types together. There is evidently no "standard" API
> for integrating different coroutine types as arbitrary coroutine types
> won't simply work together out of the box. Additionally, having used the
> standard coroutine customization points myself, I can't say I like this
> API at all and I consider it a necessary evil. It is very subtle,
> error-prone and hard to reason about. This is definitely not something
> that should be exposed to the users of the library, even experts.
> Implementing one coroutine type is hard enough, but making sure that two
> coroutine types from different authors work together is even harder.
> Considering the complexity of the standard coroutine API, even if my
> coroutine types compiled with this library's types, I would not trust
> the whole thing to actually work in all the edge cases. I believe
> `asio::awaitable` made the right choice here.
>
> I have built the library and played with the examples a bit. I've tried
> integrating it with my own coroutine types as I've described above. The
> documentation is filled with examples, which is good, but the reference
> section is lacking a bit in a sense that it doesn't document the
> signature, arguments, the return type, preconditions etc. as you would
> expect. I don't see this is a major issue because the documentation can
> always be improved, and perhaps we still have to discover how to
> effectively document coroutines because of how unique and different they
> are.
>
> I vote to REJECT this library on the basis outlined above. This is
> definitely an admirable effort and I want to see more progress for C++
> in this direction, because the standardization efforts are not promising
> and there are still no open-source libraries comparable to the likes of
> Rust's Tokio and Kotlin's standard library. Unfortunately, I don't think
> this library is a step in the right direction.
>
> All the best,
> Janko Dedic
>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>