First class, zero-overhead ASIO integration

[cstratopoulos]

@Naios

You may remember a long while back you and I e-mailed about the possibility of a first-class ASIO integration for continuable. This was not possible at the time due to

1. eager initiation of the async op
2. need to specify explicitly the return_type of a continuation chain, therefore requiring type erasure.

In boost 1.70 requirement 1. was lifted, see the 2nd bullet on the 1.70 release: https://www.boost.org/doc/libs/1_71_0/doc/html/boost_asio/history.html

And now the pending changes in Boost 1.72 remove restriction 2, you can peruse, e.g., https://github.com/boostorg/asio/commit/3f680fe1f732cec6493672425d0de9fac0a11618 https://github.com/boostorg/asio/commit/77b14fff7f63ca69fbb80e6a9fe97ff56f661a43

I have implemented an integration on a fork here: https://github.com/Naios/continuable/compare/master...cstratopoulos:feature/asio-async-result

I wanted to open a corresponding PR, but thought it best to run it by you beforehand as there are some design/integration points I wanted to run by you

I initially tried to do this using cti::promisify, but I think this is not possible. The async_result::initiate behaves like, quoting the docs,

std::forward<Initiation>(initiation)(std::move(handler), std::forward<Args>(args)...)

in our case handler is synthesized by binding a promise to a continuable Resolver type, so we need access to the promise&& parameter of the continuation, which is not present when using promisify. You will see, however, that my implementation duplicates the implementation of promisify to a considerable extent.

Second, I was wondering if you would like this to remain as an example or if you would consider promoting it to the main library, with a suitable macro/config guard. My reasoning is that specializing async_result lies some levels of detail below what a typical user would want to delive into in their hopes of using asio/continuable to be able to write, e.g,, http_request("github.com").then(/* ... */);. This is a composable, pluggable utility rather than the sort of one-off adapters generated by cti::promisify as illustrated in the existing ASIO example.

Naturally, I imagine you do not want to bind an ASIO dependency into the main library. I think this could be reasonable as an opt-in mechanism which guards the entire header with macros, similar, e.g., to coroutine support detection.

If you are open to that, I wanted furthermore to float the possibility that, with some (relatively minor) macro shenanigans around namespaces and include paths, it should be possible to have one header file which provides support for both standalone asio and boost asio.

To sketch an implementation, one might define, e.g., CTI_WITH_STANDALONE_ASIO or CTI_WITH_BOOST_ASIO, which could in turn make CTI_ASIO_NAMESPACE BEGIN resolve to either namespace asio { or namespace boost { namespace asio{. A similar approach should take care of the include paths for async_result and error_code, and I think the remaining code would be largely unchanged.

[Naios]

I saw your issue quite late, sorry. From my point of view there there is still no possibility to make the async_result zero overhead since the new form as stated by the history:

template <typename CompletionToken, typename Signature>
struct async_result
{
  typedef /* ... */ return_type;

  template <typename Initiation,
      typename RawCompletionToken,
      typename... Args>
  static return_type initiate(
      Initiation&& initiation,
      RawCompletionToken&& token,
      Args&&... args);
};

still makes return_type only dependent on the signature and the completion handler but not the initiating args itself (And doesn't explicitly state that it is not required anymore although the implementation might not require it anymore). I don't see a point mentioning return_type is not needed anymore, could you point out the corresponding passage?

I really like your approach in https://github.com/Naios/continuable/compare/master...cstratopoulos:feature/asio-async-result, the asio dependency can be probably removed by converting asio::error_code to auto, and the use_cti_t is probably allowed to be in any namespace we want, are there any other dependencies on asio directly?

Providing a non dependent async_result handler would also be compatible with standalone asio and the boost version regardless of the configuration.

[Naios]

But it seems that this is the intended behaviour now and return_type is not needed by now for C++14 although it is not explicitly stated by the docs:

https://github.com/boostorg/asio/commit/77b14fff7f63ca69fbb80e6a9fe97ff56f661a43

C++14 or later is required to support completion tokens that use per-operation return type deduction. For C++11 or earlier, a completion token's async_result specialisation must still provide the nested typedef return_type.

[cstratopoulos]

Yeah sorry if that was unclear! These changes are going into Boost 1.72 and currently merged to standalone ASIO, but the revision history has yet to be updated to reflect this. But yes the commit message you've found describes the future state of affairs.

You are right about use_cti_t, it can be in any namespace we would like.

And that is a good point about passing the error code as auto, and the handler in general. As long as the signature(s) match it can be used with either version.

The include paths and the definition of async_result, however, do require a dependency on ASIO. The idea is that there already exists a primary class template

namespace boost::asio { // or just asio
template<
    typename CompletionToken,
    typename Signature>
class async_result { /* ... */ };
}

in async_result.hpp, and that ours is a specialization of this class template, so we at least need access to that header but everything else is avoidable.

[Naios]

We probably can forward declare it for both namespaces:

namespace boost::asio {
template<
    typename CompletionToken,
    typename Signature>
class async_result;
}

namespace asio {
template<
    typename CompletionToken,
    typename Signature>
class async_result;
}

and then lift our general implementation into both of those namespaces, this could make it possible to make it available for both asio's while not depending on either of both.

[cstratopoulos]

I was looking into that possibility too, but I think it could create issues. In https://github.com/boostorg/asio/blob/develop/include/boost/asio/async_result.hpp, there is a fair bit of configuration handling to determine what the declaration will look like, whether it will be declared as a conventional class template or a constrained template with C++20 concepts, so we cannot reliably get an accurate forward declaration. Aside from that I think it is considered poor form to forward declare other people's classes.

But maybe we should take a step back--I fully understand the hesitancy to add boost or ASIO as a dependency. I think that is not necessary, and certainly not desirable. Your library provides a low-level concurrency/execution primitive, so it should not depend on a networking library.

Rather, there are two possible implementation choices that occur to me.

One option: we handle this simply with CMake configuration options and macros. That way, someone may include/build continuable with,

• -DCONTINUABLE_WITH_BOOST_ASIO or
• -DCONTINUABLE_WITH_STANDALONE_ASIO

and we use this value to determine which namespace and include paths to expose in a file called something like asio-support.hpp. This contains the async_result specialization, with some #ifdefs selecting the namespace and include path.

Perhaps a simpler option, we could have two files, e.g., boost-asio-support.hpp and standalone-asio-support.hpp. Both of these could use a shared implementation in a detail header.

In any case, we do not tell users, "you need asio to build/use continuable". Rather we tell them, if you want to use asio with continuable, here is the header (and possibly macros/CMake defines) you need. We'll give you a file that contains #include <[boost/]asio/async_result.hpp>, you're responsible for making that resolve correctly. With the CMake option we could make it so that passing the configuration options will result in a call to find_package or find_path, but that might not be necessary, see here for an example.

Hopefully one of those approaches makes more sense/seems reasonable, because again I think it is valid not to want to have ASIO as a dependency.

[Naios]

You are right, the forward declaration is not possible due to this and also probably considered evil. The library contains a "depending" header already for gtest: https://github.com/Naios/continuable/blob/master/include/continuable/continuable-testing.hpp

The ideal solution I could think about is to move those headers into:

• include
  • continuable
  • support
    • asio.hpp
    • gtest.hpp

This would be ok for me as long the headers are not included into https://github.com/Naios/continuable/blob/master/include/continuable/continuable.hpp or set as hard dependency in cmake.

To distinguish the library version:

• -DCONTINUABLE_WITH_BOOST_ASIO
• -DCONTINUABLE_WITH_STANDALONE_ASIO

is probably not needed because asio requires the -DASIO_STANDALONE=1 define anyway if it is used in standalone mode on which we can also check for.

[cstratopoulos]

Good point about checking ASIO_STANDALONE, I think you are right. The support folder sounds like a good idea to me, and indeed there's no need for continuable/continuable.hpp to include asio.hpp.

It sounds like we've resolved the questions and clarified the approach to take, so I should be ready to get started on a PR tomorrow. If you like I can move gtest.hpp when I'm doing that too, just let me know.

[Naios]

Yes that would be great if you could also move gtest.hpp. For the implementation part you might create a corresponding include/continuable/detail/support directory also. Thanks for your effort and also keeping track of the asio changes. I didn't noticed any changes because https://github.com/chriskohlhoff/asio/issues/380 wasn't probably noticed or updated by the author of asio.

[DarthBo]

This looks very interesting! Has any progress been made?

[Naios]

@DarthBo All in all the PR of @cstratopoulos is almost finished. I'll pick it up and complete it to merge it into the master soon.

[cstratopoulos]

Hello :) sorry for dropping the thread on this. My work on the PR fell at an awkward time near the end of one job and between the start of another. @Naios please don't hesitate to ping if there are any questions or ASIO gotchas I can try and help with in wrapping up what's done so far

[Naios]

@cstratopoulos No worries. I really appreciate your contribution and it really helped me to bring this feature in, especially the research about supported asio versions and the compatibility to standalone asio and boost asio is a big feature!

@DarthBo I've finished the PR #28 accordingly and it is available now in the current master branch. See the asio example file for various asio integration code examples:

https://github.com/Naios/continuable/blob/2c76e6c367f8afa25e86f6b4caf62fb1146bf8cc/examples/example-asio/example-asio-integration.cpp#L87-L99

[DarthBo]

Woah, that was insanely fast, thanks! I’ll start playing around with this right away :)