[Async] Review from Janko Dedic
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
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@lists.boost.org> 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
participants (2)
-
Niall Douglas
-
Peter Turcan