chriskohlhoff / asio

Asio C++ Library
http://think-async.com/Asio
4.81k stars 1.2k forks source link

buffered_stream doesn't seem to be compatible with ssl::stream #1265

Open david-enveil opened 1 year ago

david-enveil commented 1 year ago

I tried to post an issue for Boost.Asio and was redirected here. If this isn't the right place to post this, I apologize.

We have some C++ networking code in our codebase that uses OpenSSL directly. To make the code easier to maintain, I'm trying to replace it with the equivalent code that uses Boost.Asio. The code sets up a TLS connection between a server and client (using mutual authentication), and allows both sides to send and receive arrays of bytes.

When I ran our benchmarks, the code using Boost.Asio took nearly 5 times longer to run than the code that uses OpenSSL. Since our application sends many small messages back and forth, my guess is that the slowdown is due to a lack of buffering.

I see that Boost.Asio seems to have support for buffered I/O. For example, the buffered_stream class seems to do exactly what I'm looking for. However, I can't see how to get the constructor for that class to compile when the underlying stream is of type ssl::stream. The buffered_stream constructor takes a single lvalue argument (which is used to construct the underlying stream), but the constructor for ssl::stream requires 2 arguments.

Is it possible to use buffered_stream with ssl:stream? If not, would it be possible to add a constructor to buffered_stream to allow it to be used with ssl::stream?

I also posted a question for this on StackOverflow (link), but I haven't received a satisfactory answer yet.

david-enveil commented 1 year ago

I think it might be sufficient to update the constructors in buffered_stream to use forwarding references, as follows.

Current code:

  /// Construct, passing the specified argument to initialise the next layer.
  template <typename Arg>
  explicit buffered_stream(Arg& a)
    : inner_stream_impl_(a),
      stream_impl_(inner_stream_impl_)
  {
  }

  /// Construct, passing the specified argument to initialise the next layer.
  template <typename Arg>
  explicit buffered_stream(Arg& a, std::size_t read_buffer_size,
      std::size_t write_buffer_size)
    : inner_stream_impl_(a, write_buffer_size),
      stream_impl_(inner_stream_impl_, read_buffer_size)
  {
  }

Proposed code:

  /// Construct, passing the specified argument to initialise the next layer.
  template <typename Arg>
  explicit buffered_stream(Arg&& a)
    : inner_stream_impl_(std::forward<Arg>(a)),
      stream_impl_(inner_stream_impl_)
  {
  }

  /// Construct, passing the specified argument to initialise the next layer.
  template <typename Arg>
  explicit buffered_stream(Arg&& a, std::size_t read_buffer_size,
      std::size_t write_buffer_size)
    : inner_stream_impl_(std::forward<Arg>(a), write_buffer_size),
      stream_impl_(inner_stream_impl_, read_buffer_size)
  {
  }

Similar changes would be required to the buffered_write_stream constructors. The buffered_read_stream constructors would not be required to change, but it might be worth updating it in a similar way, for consistency with the other classes.

I'm not a C++ expert, but I think these changes should be backward-compatible with existing code, since forwarding references can act as either lvalues or rvalues, depending on the context.