From 163020ef9265dd2b3dd6b503f74bee949d1acab3 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 4 Aug 2023 13:35:34 +0100 Subject: [PATCH] Merge bitcoin/bitcoin#28203: refactor: serialization simplifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit f054bd072afb72d8dae7adc521ce15c13b236700 refactor: use "if constexpr" in std::vector's Unserialize() (Martin Leitner-Ankerl) 088caa68fb8efd8624709d643913b8a7e1218f8a refactor: use "if constexpr" in std::vector's Serialize() (Martin Leitner-Ankerl) 0fafaca4d3bbf0c0b5bfe1ec617ab15252ea51e6 refactor: use "if constexpr" in prevector's Unserialize() (Martin Leitner-Ankerl) c8839ec5cd81ba9ae88081747c49ecc758973dd1 refactor: use "if constexpr" in prevector's Serialize() (Martin Leitner-Ankerl) 1403d181c106bc271ad2522adebde07c7850069b refactor: use fold expressions instead of recursive calls in UnserializeMany() (Martin Leitner-Ankerl) bd08a008b42dac921bd9c031637e378899c1cd1d refactor: use fold expressions instead of recursive calls in SerializeMany() (Martin Leitner-Ankerl) Pull request description: This simplifies the serialization code a bit and should also make it a bit faster. * use fold expressions instead of recursive calls. This simplifies the code, makes it most likely faster because it reduces the number of function calls, and compiles faster because there are fewer template instantiations. * use `if constexpr` instead of unnecessarily creating a temporary object only to call the right overload. This is used for `std::vector` and `prevector` serialization. ACKs for top commit: MarcoFalke: only change is to add a missing `&`. lgtm, re-ACK f054bd072afb72d8dae7adc521ce15c13b236700 📦 jonatack: ACK f054bd072afb72d8dae7adc521ce15c13b236700 sipa: utACK f054bd072afb72d8dae7adc521ce15c13b236700 john-moffett: ACK f054bd072afb72d8dae7adc521ce15c13b236700 Tree-SHA512: 0417bf2d6be486c581732297945449211fc3481bac82964e27628b38ef55a47dfa58d730148aeaf1b19fa8eb1076489cc646ceebb178162a9afa59034601501d --- src/serialize.h | 183 ++++++++++++++++-------------------------------- 1 file changed, 61 insertions(+), 122 deletions(-) diff --git a/src/serialize.h b/src/serialize.h index ef3053b717..14e56baef4 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -787,23 +787,14 @@ template void Unserialize(Stream& is, std::basic_st * prevector * prevectors of unsigned char are a special case and are intended to be serialized as a single opaque blob. */ -template void Serialize_impl(Stream& os, const prevector& v, const unsigned char&); -template void Serialize_impl(Stream& os, const prevector& v, const V&); template inline void Serialize(Stream& os, const prevector& v); -template void Unserialize_impl(Stream& is, prevector& v, const unsigned char&); -template void Unserialize_impl(Stream& is, prevector& v, const V&); template inline void Unserialize(Stream& is, prevector& v); /** * vector * vectors of unsigned char are a special case and are intended to be serialized as a single opaque blob. */ -template void Serialize_impl(Stream& os, const std::vector& v, const unsigned char&); -template void Serialize_impl(Stream& os, const std::vector& v, const bool&); -template void Serialize_impl(Stream& os, const std::vector& v, const V&); template inline void Serialize(Stream& os, const std::vector& v); -template void Unserialize_impl(Stream& is, std::vector& v, const unsigned char&); -template void Unserialize_impl(Stream& is, std::vector& v, const V&); template inline void Unserialize(Stream& is, std::vector& v); /** @@ -961,122 +952,82 @@ void Unserialize(Stream& is, std::basic_string_view& str) /** * prevector */ -template -void Serialize_impl(Stream& os, const prevector& v, const unsigned char&) +template +void Serialize(Stream& os, const prevector& v) { - WriteCompactSize(os, v.size()); - if (!v.empty()) - os.write(MakeByteSpan(v)); -} - -template -void Serialize_impl(Stream& os, const prevector& v, const V&) -{ - Serialize(os, Using>(v)); -} - -template -inline void Serialize(Stream& os, const prevector& v) -{ - Serialize_impl(os, v, T()); -} - - -template -void Unserialize_impl(Stream& is, prevector& v, const unsigned char&) -{ - // Limit size per read so bogus size value won't cause out of memory - v.clear(); - unsigned int nSize = ReadCompactSize(is); - unsigned int i = 0; - while (i < nSize) - { - unsigned int blk = std::min(nSize - i, (unsigned int)(1 + 4999999 / sizeof(T))); - v.resize_uninitialized(i + blk); - is.read(AsWritableBytes(Span{&v[i], blk})); - i += blk; + if constexpr (std::is_same_v) { + WriteCompactSize(os, v.size()); + if (!v.empty()) + os.write(MakeByteSpan(v)); + } else { + Serialize(os, Using>(v)); } } -template -void Unserialize_impl(Stream& is, prevector& v, const V&) -{ - Unserialize(is, Using>(v)); -} -template -inline void Unserialize(Stream& is, prevector& v) +template +void Unserialize(Stream& is, prevector& v) { - Unserialize_impl(is, v, T()); + if constexpr (std::is_same_v) { + // Limit size per read so bogus size value won't cause out of memory + v.clear(); + unsigned int nSize = ReadCompactSize(is); + unsigned int i = 0; + while (i < nSize) { + unsigned int blk = std::min(nSize - i, (unsigned int)(1 + 4999999 / sizeof(T))); + v.resize_uninitialized(i + blk); + is.read(AsWritableBytes(Span{&v[i], blk})); + i += blk; + } + } else { + Unserialize(is, Using>(v)); + } } - /** * vector */ -template -void Serialize_impl(Stream& os, const std::vector& v, const unsigned char&) +template +void Serialize(Stream& os, const std::vector& v) { - WriteCompactSize(os, v.size()); - if (!v.empty()) - os.write(MakeByteSpan(v)); -} - -template -void Serialize_impl(Stream& os, const std::vector& v, const bool&) -{ - // A special case for std::vector, as dereferencing - // std::vector::const_iterator does not result in a const bool& - // due to std::vector's special casing for bool arguments. - WriteCompactSize(os, v.size()); - for (bool elem : v) { - ::Serialize(os, elem); + if constexpr (std::is_same_v) { + WriteCompactSize(os, v.size()); + if (!v.empty()) + os.write(MakeByteSpan(v)); + } else if constexpr (std::is_same_v) { + // A special case for std::vector, as dereferencing + // std::vector::const_iterator does not result in a const bool& + // due to std::vector's special casing for bool arguments. + WriteCompactSize(os, v.size()); + for (bool elem : v) { + ::Serialize(os, elem); + } + } else { + Serialize(os, Using>(v)); } } -template -void Serialize_impl(Stream& os, const std::vector& v, const V&) -{ - Serialize(os, Using>(v)); -} -template -inline void Serialize(Stream& os, const std::vector& v) +template +void Unserialize(Stream& is, std::vector& v) { - Serialize_impl(os, v, T()); -} - - -template -void Unserialize_impl(Stream& is, std::vector& v, const unsigned char&) -{ - // Limit size per read so bogus size value won't cause out of memory - v.clear(); - unsigned int nSize = ReadCompactSize(is); - unsigned int i = 0; - while (i < nSize) - { - unsigned int blk = std::min(nSize - i, (unsigned int)(1 + 4999999 / sizeof(T))); - v.resize(i + blk); - is.read(AsWritableBytes(Span{&v[i], blk})); - i += blk; + if constexpr (std::is_same_v) { + // Limit size per read so bogus size value won't cause out of memory + v.clear(); + unsigned int nSize = ReadCompactSize(is); + unsigned int i = 0; + while (i < nSize) { + unsigned int blk = std::min(nSize - i, (unsigned int)(1 + 4999999 / sizeof(T))); + v.resize(i + blk); + is.read(AsWritableBytes(Span{&v[i], blk})); + i += blk; + } + } else { + Unserialize(is, Using>(v)); } } -template -void Unserialize_impl(Stream& is, std::vector& v, const V&) -{ - Unserialize(is, Using>(v)); -} - -template -inline void Unserialize(Stream& is, std::vector& v) -{ - Unserialize_impl(is, v, T()); -} - - /** * pair @@ -1386,28 +1337,16 @@ public: int GetVersion() const { return nVersion; } }; -template -void SerializeMany(Stream& s) +template +void SerializeMany(Stream& s, const Args&... args) { + (::Serialize(s, args), ...); } -template -void SerializeMany(Stream& s, const Arg& arg, const Args&... args) +template +inline void UnserializeMany(Stream& s, Args&&... args) { - ::Serialize(s, arg); - ::SerializeMany(s, args...); -} - -template -inline void UnserializeMany(Stream& s) -{ -} - -template -inline void UnserializeMany(Stream& s, Arg&& arg, Args&&... args) -{ - ::Unserialize(s, arg); - ::UnserializeMany(s, args...); + (::Unserialize(s, args), ...); } template