From 99019795193d9338840b3d0381468e04790b9236 Mon Sep 17 00:00:00 2001 From: "Jesse F. Williamson" Date: Mon, 3 Aug 2026 12:37:51 -0700 Subject: [PATCH] denc: simplify internal helper templates Signed-off-by: Jesse F. Williamson --- src/include/denc.h | 446 +++++++++++++++++------------------------ src/include/encoding.h | 19 +- 2 files changed, 204 insertions(+), 261 deletions(-) diff --git a/src/include/denc.h b/src/include/denc.h index 22df7feab1f2..b9494f8bd107 100644 --- a/src/include/denc.h +++ b/src/include/denc.h @@ -276,15 +276,11 @@ namespace _denc { template concept is_any_of = (std::same_as || ...); -template struct underlying_type { - using type = T; -}; -template -struct underlying_type>> { - using type = std::underlying_type_t; -}; template -using underlying_type_t = typename underlying_type::type; +using underlying_type_t = typename std::conditional_t< + std::is_enum_v, + std::underlying_type, + std::type_identity>::type; } template @@ -346,37 +342,20 @@ struct denc_traits { // up a contiguous_appender etc is likely to be slower. namespace _denc { -template struct ExtType { - using type = void; -}; - -template -requires _denc::is_any_of -struct ExtType { - using type = ceph_le16; -}; - -template -requires _denc::is_any_of -struct ExtType { - using type = ceph_le32; -}; - -template -requires _denc::is_any_of -struct ExtType { - using type = ceph_le64; -}; - -template<> -struct ExtType { - using type = uint8_t; -}; template -using ExtType_t = typename ExtType::type; +using ExtType_t = std::conditional_t< + std::same_as, + uint8_t, + std::conditional_t< + _denc::is_any_of, + ceph_le16, + std::conditional_t< + _denc::is_any_of, + ceph_le32, + std::conditional_t< + _denc::is_any_of, + ceph_le64, + void>>>>; } // namespace _denc template @@ -407,16 +386,17 @@ struct denc_traits } }; -// varint -// -// high bit of each byte indicates another byte follows. -template -inline void denc_varint(T v, size_t& p) { - p += sizeof(T) + 1; -} +namespace _denc { +// Maximum bytes needed when each byte carries seven payload bits. template -inline void denc_varint(T v, ceph::buffer::list::contiguous_appender& p) { +inline constexpr size_t varint_bound = + (sizeof(_denc::underlying_type_t) * 8 + 6) / 7; + +template +inline void encode_varint(T v, It& p) +{ + // Low 7 bits are payload; high bit means another byte follows. uint8_t byte = v & 0x7f; v >>= 7; while (v) { @@ -428,16 +408,100 @@ inline void denc_varint(T v, ceph::buffer::list::contiguous_appender& p) { get_pos_add<__u8>(p) = byte; } -template -inline void denc_varint(T& v, ceph::buffer::ptr::const_iterator& p) { - uint8_t byte = *(__u8*)p.get_pos_add(1); - v = byte & 0x7f; +template +inline T decode_varint(It& p) +{ + using U = std::make_unsigned_t<_denc::underlying_type_t>; + auto byte = get_pos_add<__u8>(p); + U v = byte & 0x7f; int shift = 7; while (byte & 0x80) { byte = get_pos_add<__u8>(p); - v |= (T)(byte & 0x7f) << shift; + // Reassemble subsequent 7-bit payload groups above the previous ones. + v |= static_cast(byte & 0x7f) << shift; shift += 7; } + return static_cast(v); +} + +inline unsigned low_zero_nibbles(uint64_t v) +{ + auto nibbles = v ? std::countr_zero(v) / 4 : 0u; + // The lowz encoding reserves only two bits for this count. + return nibbles > 3 ? 3 : nibbles; +} + +inline uint64_t signed_magnitude(int64_t v) +{ + // Avoid negating INT64_MIN in the signed domain. + return v < 0 ? + static_cast(-(v + 1)) + 1 : + static_cast(v); +} + +inline int64_t apply_signed_magnitude(uint64_t magnitude, bool negative) +{ + return negative ? -static_cast(magnitude) : + static_cast(magnitude); +} + +inline uint64_t pack_signed_varint(int64_t v) +{ + // Low bit carries the sign; upper bits carry the magnitude. + return (signed_magnitude(v) << 1) | static_cast(v < 0); +} + +inline int64_t unpack_signed_varint(uint64_t v) +{ + return apply_signed_magnitude(v >> 1, v & 1); +} + +inline uint64_t pack_lowz_varint(uint64_t v) +{ + const auto lowznib = low_zero_nibbles(v); + // Low two bits carry the stripped zero-nibble count. + return (v >> (lowznib * 4) << 2) | lowznib; +} + +inline uint64_t unpack_lowz_varint(uint64_t v) +{ + const auto lowznib = v & 3; + return (v >> 2) << (lowznib * 4); +} + +inline uint64_t pack_signed_lowz_varint(int64_t v) +{ + const auto magnitude = signed_magnitude(v); + const auto lowznib = low_zero_nibbles(magnitude); + // Bit 0 carries sign; bits 1-2 carry the stripped zero-nibble count. + return ((magnitude >> (lowznib * 4)) << 3) | + (static_cast(lowznib) << 1) | + static_cast(v < 0); +} + +inline int64_t unpack_signed_lowz_varint(uint64_t v) +{ + const auto lowznib = (v & 6) >> 1; + const auto magnitude = (v >> 3) << (lowznib * 4); + return apply_signed_magnitude(magnitude, v & 1); +} + +} // namespace _denc + +// Variable-length integer encodings, with signed and low-zero variants. +template +inline void denc_varint(T v, size_t& p) { + p += _denc::varint_bound; +} + +template +inline void denc_varint(T v, ceph::buffer::list::contiguous_appender& p) { + _denc::encode_varint(v, p); +} + +template +inline void denc_varint(T& v, ceph::buffer::ptr::const_iterator& p) { + v = _denc::decode_varint(p); } @@ -446,48 +510,33 @@ inline void denc_varint(T& v, ceph::buffer::ptr::const_iterator& p) { // low bit = 1 = negative, 0 = positive // high bit of every byte indicates whether another byte follows. inline void denc_signed_varint(int64_t v, size_t& p) { - p += sizeof(v) + 2; + denc_varint(_denc::pack_signed_varint(v), p); } template requires (!is_const_iterator) void denc_signed_varint(int64_t v, It& p) { - if (v < 0) { - v = (-v << 1) | 1; - } else { - v <<= 1; - } - denc_varint(v, p); + denc_varint(_denc::pack_signed_varint(v), p); } template inline void denc_signed_varint(T& v, It& p) { - int64_t i = 0; + uint64_t i = 0; denc_varint(i, p); - if (i & 1) { - v = -(i >> 1); - } else { - v = i >> 1; - } + v = _denc::unpack_signed_varint(i); } -// varint + lowz encoding +// varint and lowz (low-zero) encoding: // // first(low) 2 bits = how many low zero bits (nibbles) // high bit of each byte = another byte follows // (so, 5 bits data in first byte, 7 bits data thereafter) inline void denc_varint_lowz(uint64_t v, size_t& p) { - p += sizeof(v) + 2; + denc_varint(_denc::pack_lowz_varint(v), p); } inline void denc_varint_lowz(uint64_t v, ceph::buffer::list::contiguous_appender& p) { - int lowznib = v ? (std::countr_zero(v) / 4) : 0; - if (lowznib > 3) - lowznib = 3; - v >>= lowznib * 4; - v <<= 2; - v |= lowznib; - denc_varint(v, p); + denc_varint(_denc::pack_lowz_varint(v), p); } template @@ -495,10 +544,7 @@ inline void denc_varint_lowz(T& v, ceph::buffer::ptr::const_iterator& p) { uint64_t i = 0; denc_varint(i, p); - int lowznib = (i & 3); - i >>= 2; - i <<= lowznib * 4; - v = i; + v = _denc::unpack_lowz_varint(i); } // signed varint + lowz encoding @@ -508,41 +554,20 @@ inline void denc_varint_lowz(T& v, ceph::buffer::ptr::const_iterator& p) // high bit of each byte = another byte follows // (so, 4 bits data in first byte, 7 bits data thereafter) inline void denc_signed_varint_lowz(int64_t v, size_t& p) { - p += sizeof(v) + 2; + denc_varint(_denc::pack_signed_lowz_varint(v), p); } template requires (!is_const_iterator) inline void denc_signed_varint_lowz(int64_t v, It& p) { - bool negative = false; - if (v < 0) { - v = -v; - negative = true; - } - unsigned lowznib = v ? (std::countr_zero(std::bit_cast(v)) / 4) : 0u; - if (lowznib > 3) - lowznib = 3; - v >>= lowznib * 4; - v <<= 3; - v |= lowznib << 1; - v |= (int)negative; - denc_varint(v, p); + denc_varint(_denc::pack_signed_lowz_varint(v), p); } template inline void denc_signed_varint_lowz(T& v, It& p) { - int64_t i = 0; + uint64_t i = 0; denc_varint(i, p); - int lowznib = (i & 6) >> 1; - if (i & 1) { - i >>= 3; - i <<= lowznib * 4; - v = -i; - } else { - i >>= 3; - i <<= lowznib * 4; - v = i; - } + v = _denc::unpack_signed_lowz_varint(i); } @@ -677,21 +702,29 @@ denc(T& o, } namespace _denc { -template +template +concept has_legacy_decode = requires( + T& v, + ceph::buffer::list::const_iterator& p) { + v.decode(p); + }; + +// Prefer an explicit legacy member decode; otherwise use non-contiguous traits. +template, typename = void> struct has_legacy_denc : std::false_type {}; + template -struct has_legacy_denc() - .decode(std::declval< - ceph::buffer::list::const_iterator&>()))> - : std::true_type { +struct has_legacy_denc : std::true_type { static void decode(T& v, ceph::buffer::list::const_iterator& p) { v.decode(p); } }; + template -struct has_legacy_denc::need_contiguous>> : std::true_type { +struct has_legacy_denc< + T, + false, + std::enable_if_t::need_contiguous>> : std::true_type { static void decode(T& v, ceph::buffer::list::const_iterator& p) { denc_traits::decode(v, p); } @@ -918,10 +951,11 @@ struct denc_traits< }; namespace _denc { - template class C, typename Details, typename ...Ts> + // Shared container codec; Details supplies reserve and insertion policy. + template struct container_base { private: - using container = C; + using container = Container; using T = typename Details::T; public: @@ -1028,30 +1062,16 @@ namespace _denc { } }; - template - class container_has_reserve { - template struct SFINAE_match; - template - static std::true_type test(SFINAE_match*); - - template - static std::false_type test(...); - - public: - static constexpr bool value = decltype( - test>(0))::value; + template + concept has_reserve = requires(Container& c, typename Container::size_type n) { + c.reserve(n); }; - template - inline constexpr bool container_has_reserve_v = - container_has_reserve::value; - template struct container_details_base { using T = typename Container::value_type; static void reserve(Container& c, size_t s) { - if constexpr (container_has_reserve_v) { + if constexpr (has_reserve) { c.reserve(s); } } @@ -1070,117 +1090,26 @@ template struct denc_traits< std::list, typename std::enable_if_t::supported>> - : public _denc::container_base>, - T, Ts...> {}; + : public _denc::container_base< + std::list, + _denc::pushback_details>> {}; template struct denc_traits< std::vector, typename std::enable_if_t::supported>> - : public _denc::container_base>, - T, Ts...> {}; + : public _denc::container_base< + std::vector, + _denc::pushback_details>> {}; template struct denc_traits< boost::container::small_vector, - typename std::enable_if_t::supported>> { -private: - using container = boost::container::small_vector; -public: - using traits = denc_traits; - - static constexpr bool supported = true; - static constexpr bool featured = traits::featured; - static constexpr bool bounded = false; - static constexpr bool need_contiguous = traits::need_contiguous; - - template - static void bound_encode(const container& s, size_t& p, uint64_t f = 0) { - p += sizeof(uint32_t); - if constexpr (traits::bounded) { - if (!s.empty()) { - const auto elem_num = s.size(); - size_t elem_size = 0; - if constexpr (traits::featured) { - denc(*s.begin(), elem_size, f); - } else { - denc(*s.begin(), elem_size); - } - p += elem_size * elem_num; - } - } else { - for (const T& e : s) { - if constexpr (traits::featured) { - denc(e, p, f); - } else { - denc(e, p); - } - } - } - } - - template - static void encode(const container& s, - ceph::buffer::list::contiguous_appender& p, - uint64_t f = 0) { - denc((uint32_t)s.size(), p); - if constexpr (traits::featured) { - encode_nohead(s, p, f); - } else { - encode_nohead(s, p); - } - } - static void decode(container& s, ceph::buffer::ptr::const_iterator& p, - uint64_t f = 0) { - uint32_t num; - denc(num, p); - decode_nohead(num, s, p, f); - } - template - static std::enable_if_t - decode(container& s, ceph::buffer::list::const_iterator& p) { - uint32_t num; - denc(num, p); - decode_nohead(num, s, p); - } - - // nohead - static void encode_nohead(const container& s, ceph::buffer::list::contiguous_appender& p, - uint64_t f = 0) { - for (const T& e : s) { - if constexpr (traits::featured) { - denc(e, p, f); - } else { - denc(e, p); - } - } - } - static void decode_nohead(size_t num, container& s, - ceph::buffer::ptr::const_iterator& p, - uint64_t f=0) { - s.clear(); - s.reserve(num); - while (num--) { - T t; - denc(t, p, f); - s.push_back(std::move(t)); - } - } - template - static std::enable_if_t - decode_nohead(size_t num, container& s, - ceph::buffer::list::const_iterator& p) { - s.clear(); - s.reserve(num); - while (num--) { - T t; - denc(t, p); - s.push_back(std::move(t)); - } - } -}; + typename std::enable_if_t::supported>> + : public _denc::container_base< + boost::container::small_vector, + _denc::pushback_details< + boost::container::small_vector>> {}; namespace _denc { template @@ -1197,28 +1126,22 @@ template struct denc_traits< std::set, std::enable_if_t::supported>> - : public _denc::container_base>, - T, Ts...> {}; + : public _denc::container_base< + std::set, + _denc::setlike_details>> {}; template struct denc_traits< boost::container::flat_set, std::enable_if_t::supported>> : public _denc::container_base< - boost::container::flat_set, - _denc::setlike_details>, - T, Ts...> {}; + boost::container::flat_set, + _denc::setlike_details>> {}; namespace _denc { + // Maps use the same hinted emplacement path as sets. template - struct maplike_details : public container_details_base { - using T = typename Container::value_type; - template - static void insert(Container& c, Args&& ...args) { - c.emplace_hint(c.cend(), std::forward(args)...); - } - }; + using maplike_details = setlike_details; } template @@ -1226,9 +1149,9 @@ struct denc_traits< std::map, std::enable_if_t::supported && denc_traits::supported>> - : public _denc::container_base>, - A, B, Ts...> {}; + : public _denc::container_base< + std::map, + _denc::maplike_details>> {}; template struct denc_traits< @@ -1236,10 +1159,8 @@ struct denc_traits< std::enable_if_t::supported && denc_traits::supported>> : public _denc::container_base< - boost::container::flat_map, - _denc::maplike_details>, - A, B, Ts...> {}; + boost::container::flat_map, + _denc::maplike_details>> {}; template struct denc_traits< @@ -1378,11 +1299,6 @@ public: }; namespace _denc { - template - void clear_optional(Optional& v) { - v.reset(); - } - template struct optional_base { using traits = denc_traits; @@ -1424,7 +1340,7 @@ namespace _denc { return; } - clear_optional(v); + v.reset(); } static void encode_nohead(const Optional& v, @@ -1444,7 +1360,7 @@ namespace _denc { return; } - clear_optional(v); + v.reset(); } }; } @@ -1709,18 +1625,25 @@ inline std::enable_if_t decode_nohead( namespace ceph::denc_detail { +// Fixed size of the DENC wrapper header written by DENC_START: inline constexpr auto struct_header_len() noexcept { - return sizeof(__u8) + sizeof(__u8) + sizeof(uint32_t); + return sizeof(__u8) // 1 byte: encoded struct version + + sizeof(__u8) // 1 byte: oldest decoder version accepted + + sizeof(uint32_t); // 4 bytes: payload length following header } +// Bound-size pass: account for the fixed DENC header without writing it: +// The unused parameters are an artifact of the DENC_HELPERS macros. inline void start(size_t& p, __u8 *, __u8 *, char **, uint32_t *) { p += struct_header_len(); } +// Bound-size pass: all body sizing already happened, so nothing is patched: inline void finish(size_t&, char **, uint32_t *) {} +// Encode pass: write version/compat and reserve space for payload length: inline void start(::ceph::buffer::list::contiguous_appender& p, __u8 *struct_v, __u8 *struct_compat, char **len_pos, uint32_t *start_oob_off) @@ -1731,6 +1654,7 @@ inline void start(::ceph::buffer::list::contiguous_appender& p, *start_oob_off = p.get_out_of_band_offset(); } +// Encode pass: patch the reserved length, including out-of-band bytes: inline void finish(::ceph::buffer::list::contiguous_appender& p, char **len_pos, uint32_t *start_oob_off) { @@ -1740,6 +1664,7 @@ inline void finish(::ceph::buffer::list::contiguous_appender& p, std::memcpy(*len_pos, &struct_len, sizeof(struct_len)); } +// Decode pass: read and validate the header before decoding body fields: inline void start(::ceph::buffer::ptr::const_iterator& p, __u8 *struct_v, __u8 *struct_compat, char **start_pos, uint32_t *struct_len, @@ -1755,6 +1680,7 @@ inline void start(::ceph::buffer::ptr::const_iterator& p, *start_pos = const_cast(p.get_pos()); } +// Decode pass: reject overread and skip unread fields from newer encoders: inline void finish(::ceph::buffer::ptr::const_iterator& p, char **start_pos, uint32_t *struct_len, const char *func) diff --git a/src/include/encoding.h b/src/include/encoding.h index 71a49bac7259..e12a74e0d3d8 100644 --- a/src/include/encoding.h +++ b/src/include/encoding.h @@ -257,7 +257,6 @@ inline void encode_bytes(const void *data, size_t len, bufferlist& bl) } // namespace encoding_detail -// string inline void encode(std::string_view s, bufferlist& bl, uint64_t features=0) { encoding_detail::encode_bytes(s.data(), s.length(), bl); @@ -324,15 +323,19 @@ inline void decode(buffer::ptr& bp, bufferlist::const_iterator& p) bufferlist s; p.copy(len, s); + // Return without assignment: if (!len) { return; } + // ...if the buffer::list contains only a single buffer, we + // re-use it: if (1 == s.get_num_buffers()) { bp = s.front(); return; } + // ...flatten the buffer::list: bp = buffer::copy(s.c_str(), s.length()); } @@ -469,6 +472,8 @@ std::optional decode_optional(bufferlist::const_iterator& p); template void for_each_count(unsigned n, FnT&& fn); +// "nohead" as in just the elements, without the container length: +// (i.e.: [x][x][x] rather than [sz][x][x][x], like encode_range()) template void encode_range_nohead(const RangeT& r, bufferlist& bl); @@ -1352,6 +1357,7 @@ void encode_pair_range(const MapT& m, bufferlist& bl, uint64_t features) encode_pair_range_nohead(m, bl, features); } +// Decodes n kv entries into containers that support operator[]: template void decode_map_entries_by_subscript(unsigned n, MapT& m, IteratorT& p) @@ -1376,6 +1382,8 @@ void decode_map_entries_by_emplace(unsigned n, MapT& m, }); } +// Decide between encoding associative containers by operator[] or by +// emplace(): template void decode_map_entries(unsigned n, MapT& m, IteratorT& p) { @@ -1475,6 +1483,7 @@ inline void decode(T &o, const bufferlist& bl) namespace ceph::encoding_detail { +// Fixed size of the versioned encode/decode wrapper header. inline constexpr auto struct_header_len() noexcept { return sizeof(__u8) + sizeof(__u8) + sizeof(ceph_le32); @@ -1486,6 +1495,7 @@ struct struct_header final { __u32 len = 0; }; +// Read the encoded version, compatibility version, and payload length. template struct_header read_struct_header(IteratorT& bl) { @@ -1499,6 +1509,7 @@ struct_header read_struct_header(IteratorT& bl) return header; } +// Reject payloads that require a newer decoder than this code provides. inline void check_decode_compat(__u8 code_v, __u8 struct_v, __u8 struct_compat, const char *func) @@ -1509,6 +1520,7 @@ inline void check_decode_compat(__u8 code_v, __u8 struct_v, } } +// Compute the payload end offset after checking that the body is present. template unsigned checked_struct_end(IteratorT& bl, __u32 struct_len, const char *func) @@ -1520,6 +1532,7 @@ unsigned checked_struct_end(IteratorT& bl, __u32 struct_len, return bl.get_off() + struct_len; } +// DECODE_START path: read and validate the standard wrapper header. template unsigned decode_struct_start(__u8 code_v, VersionT& struct_v, __u8& struct_compat, __u32& struct_len, @@ -1534,6 +1547,7 @@ unsigned decode_struct_start(__u8 code_v, VersionT& struct_v, return checked_struct_end(bl, struct_len, func); } +// Legacy decode path: read only the wrapper fields present in this version. template unsigned decode_legacy_struct_start(__u8 code_v, VersionT& struct_v, __u8 compat_v, __u8 len_v, @@ -1567,6 +1581,7 @@ unsigned decode_legacy_struct_start(__u8 code_v, VersionT& struct_v, return checked_struct_end(bl, struct_len, func); } +// ENCODE_FINISH path: patch version, compatibility, and payload length. template void finish_encode_struct(FillerT& filler, __u8 struct_v, __u8& struct_compat, ceph_le32& struct_len, @@ -1586,6 +1601,7 @@ void finish_encode_struct(FillerT& filler, __u8 struct_v, reinterpret_cast(&struct_len)); } +// DECODE_FINISH path: reject overread and skip unread trailing fields. template void finish_decode_struct(IteratorT& bl, unsigned struct_end, const char *func) @@ -1603,6 +1619,7 @@ void finish_decode_struct(IteratorT& bl, unsigned struct_end, } } +// Preserve an unknown encoded payload, including its wrapper header. template void decode_unknown(PayloadT& payload, IteratorT& bl, const char *func) { -- 2.47.3