From 5d553f5d71495afe4819e04baed0483014a86eef Mon Sep 17 00:00:00 2001 From: Marcelo Zimbres Date: Fri, 15 Mar 2024 11:31:30 +0100 Subject: [PATCH 1/2] Some fixes in the article about the costs of async abstractions [skip ci] --- doc/on-the-costs-of-async-abstractions.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/doc/on-the-costs-of-async-abstractions.md b/doc/on-the-costs-of-async-abstractions.md index 55615318..d728c584 100644 --- a/doc/on-the-costs-of-async-abstractions.md +++ b/doc/on-the-costs-of-async-abstractions.md @@ -17,7 +17,7 @@ here influenced my views on the proposed (a.k.a. Senders and Receivers), which is likely to become the basis of networking in upcoming C++ standards. -Although the analysis presented here uses the Redis communication +Although the analysis presented in this article uses the Redis communication protocol for illustration I expect it to be useful in general since [RESP3](https://github.com/antirez/RESP3/blob/master/spec.md) shares many similarities with other widely used protocols such as HTTP. @@ -98,7 +98,7 @@ This is illustrated in the diagram below |<---- offset threshold ---->| | | "+PONG\r\n:100\r\n+OK\r\n_\r\n+PONG\r\n" - | # Initial message + | # Initial offset "+PONG\r\n:100\r\n+OK\r\n_\r\n+PONG\r\n" |<------>| # After 1st message @@ -110,7 +110,7 @@ This is illustrated in the diagram below |<--------------------->| # After 3rd message "+PONG\r\n:100\r\n+OK\r\n_\r\n+PONG\r\n" - |<-------------------------->| # 4th message crosses the threashold + |<-------------------------->| # Threshold crossed after the 4th message "+PONG\r\n" | # After rotation @@ -255,7 +255,7 @@ avoided, this is what worked for Boost.Redis `try_send_via_dispatch` for a more aggressive optimization). 3. Coalescing of individual requests into a single payload to reduce - the number of necessary writes on the socket,this is only + the number of necessary writes on the socket, this is only possible because Redis supports pipelining (good protocols help!). @@ -282,8 +282,8 @@ avoided, this is what worked for Boost.Redis `resp3::async_read` is IO-less. Sometimes it is not possible to avoid asynchronous operations that -complete synchronously, in the following sections we will therefore -see how favoring throughput over fairness works in Boost.Asio. +complete synchronously, in the following sections we will see how to +favor throughput over fairness in Boost.Asio. ### Calling the continuation inline @@ -299,7 +299,7 @@ async_read_until(socket, buffer, "\r\n", continuation); // Immediate completions are executed in exec2 (otherwise equal to the // version above). The completion is called inline if exec2 is the -same // executor that is running the operation. +// same executor that is running the operation. async_read_until(socket, buffer, "\r\n", bind_immediate_executor(exec2, completion)); ``` @@ -388,7 +388,7 @@ Although faster, this strategy has some downsides - Requires additional layers of complexity such as `bind_immediate_executor` in addition to `bind_executor`. - - Not compliat with more strict + - Non-compliat with more strict [guidelines](https://en.wikipedia.org/wiki/The_Power_of_10:_Rules_for_Developing_Safety-Critical_Code) that prohibits reentrat code. @@ -432,7 +432,7 @@ instructing the asynchronous operation to call the completion inline on immediate completion. It turns out however that coroutine support for _tail-calls_ provide a way to completely sidestep this problem. This feature is described by -[Backer](https://lewissbaker.github.io/2020/05/11/understanding_symmetric_transfer) +[Lewis Baker](https://lewissbaker.github.io/2020/05/11/understanding_symmetric_transfer) as follows > A tail-call is one where the current stack-frame is popped before @@ -581,7 +581,7 @@ reentracy, allowing [sixteen](https://github.com/NVIDIA/stdexec/blob/83cdb92d316e8b3bca1357e2cf49fc39e9bed403/include/exec/trampoline_scheduler.hpp#L52) levels of inline calls by default. While in Boost.Asio it is possible to use reentracy as an optimization for a corner cases, here it is made its -_modus operandi_, my opinion about this has already been stated in a +_modus operandi_, the downsides of this approach have already been stated in a previous section so I won't repeat it here. Also the fact that a special scheduler is needed by specific From b6e1280075fa0f2c71af3c7b89ef2717aa922557 Mon Sep 17 00:00:00 2001 From: Marcelo Zimbres Date: Wed, 20 Mar 2024 11:28:15 +0100 Subject: [PATCH 2/2] Fixes narrowing conversion. NOTE: I had to disable the TLS tests because I shotdown the server I was running on my domain occase.de. Once this ticket is merged I will open a new one to fix that and reenable the tests. --- README.md | 8 ++++++++ include/boost/redis/resp3/impl/parser.ipp | 8 ++++---- include/boost/redis/resp3/parser.hpp | 6 ++---- test/CMakeLists.txt | 3 ++- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 90d771e3..a27c435a 100644 --- a/README.md +++ b/README.md @@ -702,6 +702,14 @@ https://lists.boost.org/Archives/boost/2023/01/253944.php. Sets `"default"` as the default value of `config::username`. This makes it simpler to use the `requirepass` configuration in Redis. +* ([Issue 189](https://github.com/boostorg/redis/issues/189)). + Fixes narrowing convertion by using `std::size_t` instead of + `std::uint64_t` for the sizes of bulks and aggregates. The code + relies now on `std::from_chars` returning an error if a value + greater than 32 is received on platforms on which the size + of`std::size_t` is 32. + + ### Boost 1.84 (First release in Boost) * Deprecates the `async_receive` overload that takes a response. Users diff --git a/include/boost/redis/resp3/impl/parser.ipp b/include/boost/redis/resp3/impl/parser.ipp index 8a70739b..eda9991d 100644 --- a/include/boost/redis/resp3/impl/parser.ipp +++ b/include/boost/redis/resp3/impl/parser.ipp @@ -1,4 +1,4 @@ -/* Copyright (c) 2018-2023 Marcelo Zimbres Silva (mzimbres@gmail.com) +/* Copyright (c) 2018-2024 Marcelo Zimbres Silva (mzimbres@gmail.com) * * Distributed under the Boost Software License, Version 1.0. (See * accompanying file LICENSE.txt) @@ -13,7 +13,7 @@ namespace boost::redis::resp3 { -void to_int(int_type& i, std::string_view sv, system::error_code& ec) +void to_int(std::size_t& i, std::string_view sv, system::error_code& ec) { auto const res = std::from_chars(sv.data(), sv.data() + std::size(sv), i); if (res.ec != std::errc()) @@ -29,7 +29,7 @@ void parser::reset() { depth_ = 0; sizes_ = {{1}}; - bulk_length_ = (std::numeric_limits::max)(); + bulk_length_ = (std::numeric_limits::max)(); bulk_ = type::invalid; consumed_ = 0; sizes_[0] = 2; // The sentinel must be more than 1. @@ -189,7 +189,7 @@ parser::consume_impl( case type::attribute: case type::map: { - int_type l = -1; + std::size_t l = -1; to_int(l, elem, ec); if (ec) return {}; diff --git a/include/boost/redis/resp3/parser.hpp b/include/boost/redis/resp3/parser.hpp index 40cc4056..d40e239a 100644 --- a/include/boost/redis/resp3/parser.hpp +++ b/include/boost/redis/resp3/parser.hpp @@ -1,4 +1,4 @@ -/* Copyright (c) 2018-2023 Marcelo Zimbres Silva (mzimbres@gmail.com) +/* Copyright (c) 2018-2024 Marcelo Zimbres Silva (mzimbres@gmail.com) * * Distributed under the Boost Software License, Version 1.0. (See * accompanying file LICENSE.txt) @@ -16,8 +16,6 @@ namespace boost::redis::resp3 { -using int_type = std::uint64_t; - class parser { public: using node_type = basic_node; @@ -38,7 +36,7 @@ class parser { std::array sizes_; // Contains the length expected in the next bulk read. - int_type bulk_length_; + std::size_t bulk_length_; // The type of the next bulk. Contains type::invalid if no bulk is // expected. diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 049469f3..d5fc0793 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -30,7 +30,8 @@ macro(make_test TEST_NAME STANDARD) endmacro() make_test(test_conn_quit 17) -make_test(test_conn_tls 17) +# TODO: Configure a Redis server with TLS in the CI and reenable this test. +#make_test(test_conn_tls 17) make_test(test_low_level 17) make_test(test_conn_exec_retry 17) make_test(test_conn_exec_error 17)