PS-10243 feature: Add data structures for GTID set operations (part 1) (#76)

https://perconadev.atlassian.net/browse/PS-10243

Changed the definition of the 'util::bounded_string_storage' - instead of being
'std::array<std::byte, N>', it is now
'boost::container::static_vector<std::byte, N>' which is more appropriate
because it holds real size in one of its members eliminating the necessity to
do 'strlen()' every time we want to use this data as a string.
This affected 'server_version' member in the FORMAT_DESCRIPTION event
post header and 'tag' in the the GTID_TAGGED_LOG event body.

In addition, changed the underlying container for storing 'binlog' member (file
name) in the ROTATE event body: instead of 'std::string', we now use
'boost::container::small_vector<std::byte, 64> which will let us avoid dynamic
allocations for file names up to 64 bytes.

Author: percona-ysorokin

src/binsrv/event/format_description_post_header_impl.cpp

@@ -21,7 +21,6 @@
 #include <stdexcept>
 #include <string>
 #include <string_view>
-#include <tuple>
 
 #include <boost/align/align_up.hpp>
 
@@ -64,24 +63,27 @@ generic_post_header_impl<code_type::format_description>::
   */
 
   static_assert(server_version_length ==
-                    std::tuple_size_v<decltype(server_version_)>,
+                    decltype(server_version_)::static_capacity,
                 "mismatch in "
                 "generic_post_header_impl<code_type::format_description>::"
                 "server_version_length");
 
   // TODO: initialize size_in_bytes directly based on the sum of fields
   // widths instead of this static_assert
   static_assert(
-      sizeof binlog_version_ + std::tuple_size_v<decltype(server_version_)> +
+      sizeof binlog_version_ + decltype(server_version_)::static_capacity +
               sizeof create_timestamp_ + sizeof common_header_length_ +
-              std::tuple_size_v<decltype(post_header_lengths_)> ==
+              sizeof post_header_lengths_ ==
           get_size_in_bytes(latest_known_protocol_server_version),
       "mismatch in "
       "generic_post_header_impl<code_type::format_description>::size_in_bytes");
   // make sure we did OK with data members reordering
   static_assert(sizeof *this ==
                     boost::alignment::align_up(
-                        get_size_in_bytes(latest_known_protocol_server_version),
+                        sizeof binlog_version_ + sizeof server_version_ +
+                            sizeof create_timestamp_ +
+                            sizeof common_header_length_ +
+                            sizeof post_header_lengths_,
                         alignof(decltype(*this))),
                 "inefficient data member reordering in "
                 "generic_post_header_impl<code_type::format_description>");
@@ -93,7 +95,10 @@ generic_post_header_impl<code_type::format_description>::
 
   auto remainder = portion;
   util::extract_fixed_int_from_byte_span(remainder, binlog_version_);
-  util::extract_byte_array_from_byte_span(remainder, server_version_);
+  server_version_.resize(decltype(server_version_)::capacity());
+  util::extract_byte_span_from_byte_span(remainder,
+                                         util::byte_span{server_version_});
+  util::normalize_for_c_str(server_version_);
   util::extract_fixed_int_from_byte_span(remainder, create_timestamp_);
   util::extract_fixed_int_from_byte_span(remainder, common_header_length_);
   const auto expected_subrange_length{

src/binsrv/event/gtid_log_post_header.cpp

@@ -84,9 +84,9 @@ gtid_log_post_header::gtid_log_post_header(util::const_byte_span portion) {
 
   // TODO: initialize size_in_bytes directly based on the sum of fields
   // widths instead of this static_assert
-  static_assert(sizeof flags_ + std::tuple_size_v<decltype(uuid_)> +
-                        sizeof gno_ + sizeof logical_ts_code_ +
-                        sizeof last_committed_ + sizeof sequence_number_ ==
+  static_assert(sizeof flags_ + sizeof uuid_ + sizeof gno_ +
+                        sizeof logical_ts_code_ + sizeof last_committed_ +
+                        sizeof sequence_number_ ==
                     size_in_bytes,
                 "mismatch in gtid_log_event_post_header::size_in_bytes");
   // make sure we did OK with data members reordering

src/binsrv/event/gtid_tagged_log_body_impl.cpp

@@ -59,9 +59,9 @@ generic_body_impl<code_type::gtid_tagged_log>::generic_body_impl(
   static_assert(
       sizeof *this ==
           boost::alignment::align_up(
-              sizeof flags_ + std::tuple_size_v<decltype(uuid_)> + sizeof gno_ +
-                  std::tuple_size_v<decltype(tag_)> + sizeof last_committed_ +
-                  sizeof sequence_number_ + sizeof immediate_commit_timestamp_ +
+              sizeof flags_ + sizeof uuid_ + sizeof gno_ + sizeof tag_ +
+                  sizeof last_committed_ + sizeof sequence_number_ +
+                  sizeof immediate_commit_timestamp_ +
                   sizeof original_commit_timestamp_ +
                   sizeof transaction_length_ + sizeof original_server_version_ +
                   sizeof immediate_server_version_ +
@@ -297,6 +297,7 @@ void generic_body_impl<code_type::gtid_tagged_log>::process_field_data(
     // Extracting tag (length encoded as varlent int and raw character array)
     std::size_t extracted_tag_length{};
     varlen_int_extractor(remainder, extracted_tag_length, "tag length");
+    tag_.resize(extracted_tag_length);
     const std::span<tag_storage::value_type> tag_subrange{std::data(tag_),
                                                           extracted_tag_length};
     if (!util::extract_byte_span_from_byte_span_checked(remainder,

src/binsrv/event/rotate_body_impl.cpp

@@ -15,6 +15,7 @@
 
 #include "binsrv/event/rotate_body_impl.hpp"
 
+#include <iterator>
 #include <ostream>
 
 #include "binsrv/event/code_type.hpp"
@@ -30,7 +31,7 @@ generic_body_impl<code_type::rotate>::generic_body_impl(
   // no need to check if member reordering is OK as this class has
   // only one member for holding data of varying length
 
-  binlog_.assign(util::as_string_view(portion));
+  binlog_.assign(std::begin(portion), std::end(portion));
 }
 
 std::ostream &operator<<(std::ostream &output,

src/binsrv/event/rotate_body_impl.hpp

@@ -21,15 +21,19 @@
 #include <string>
 #include <string_view>
 
-#include "util/byte_span_fwd.hpp"
+#include <boost/container/small_vector.hpp>
+
+#include "util/byte_span.hpp"
 
 namespace binsrv::event {
 
 template <> class [[nodiscard]] generic_body_impl<code_type::rotate> {
 public:
-  // TODO: change this to a container with larger buffer for SBO to avoid
-  //       memory allocations (e.g. boost::container::small_vector)
-  using binlog_storage = std::string;
+  static constexpr std::size_t expected_max_binlog_name_length{64U};
+  // TODO: in c++26 change to std::inplace_vector
+  using binlog_storage =
+      boost::container::small_vector<std::byte,
+                                     expected_max_binlog_name_length>;
 
   explicit generic_body_impl(util::const_byte_span portion);
 
@@ -38,7 +42,7 @@ template <> class [[nodiscard]] generic_body_impl<code_type::rotate> {
   }
 
   [[nodiscard]] std::string_view get_binlog() const noexcept {
-    return {binlog_};
+    return util::as_string_view(binlog_);
   }
 
 private:

src/util/bounded_string_storage.hpp

@@ -25,17 +25,20 @@
 namespace util {
 
 template <std::size_t N>
-std::string_view
+[[nodiscard]] std::string_view
 to_string_view(const bounded_string_storage<N> &storage) noexcept {
-  // in case when every byte of the array is significant (non-zero)
-  // we cannot rely on std::string_view::string_view(const char*)
-  // constructor as '\0' character will never be found
-  auto result{util::as_string_view(storage)};
-  auto position{result.find('\0')};
-  if (position != std::string_view::npos) {
-    result.remove_suffix(std::size(result) - position);
+  return util::as_string_view(storage);
+}
+
+template <std::size_t N>
+void normalize_for_c_str(bounded_string_storage<N> &storage) {
+  // in case when every byte of the container is significant (non-zero)
+  // we do not change the size of the container
+  if (const auto begin_it{std::cbegin(storage)}, end_it{std::cend(storage)},
+      zero_it{std::find(begin_it, end_it, std::byte{})};
+      zero_it != end_it) {
+    storage.resize(static_cast<std::size_t>(std::distance(begin_it, zero_it)));
   }
-  return result;
 }
 
 } // namespace util

src/util/bounded_string_storage_fwd.hpp

@@ -16,13 +16,14 @@
 #ifndef UTIL_BOUNDED_STRING_STORAGE_FWD_HPP
 #define UTIL_BOUNDED_STRING_STORAGE_FWD_HPP
 
-#include <array>
 #include <cstddef>
 
+#include <boost/container/static_vector.hpp>
+
 namespace util {
 
 template <std::size_t N>
-using bounded_string_storage = std::array<std::byte, N>;
+using bounded_string_storage = boost::container::static_vector<std::byte, N>;
 
 } // namespace util