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.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | MarcoFalke, jonatack, sipa, john-moffett |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
1007-}
1008-
1009-template<typename Stream>
1010-inline void UnserializeMany(Stream& s)
1011+template<typename Stream, typename... Args>
1012+void SerializeMany(Stream& s, const Args&... args)
SerializeMany isn’t inline like the neighboring methods.
inline from any template function as it won’t make any difference.
This looks like a very nice cleanup (and in a widely included and large file).
Have started running a node on this branch.
Initial approach ACK 513f4659b0a2ac4e2454ad2425b0fec8d7f64f04
1044-{
1045-}
1046-
1047-template<typename Stream, typename Arg, typename... Args>
1048-void SerializeMany(Stream& s, const Arg& arg, const Args&... args)
1049+template<typename Stream, typename... Args>
1051-}
1052-
1053-template<typename Stream, typename Arg, typename... Args>
1054-inline void UnserializeMany(Stream& s, Arg&& arg, Args&&... args)
1055+template<typename Stream, typename... Args>
1056+inline void UnserializeMany(Stream& s, Args&... args)
48adef0e41700e4b5d0072c71ffaa2387cdeccbb: Any reason to change this to disallow temporaries (&&)?
Also this commit can be squashed in the previous one?
Also can remove inline. (template enforces inline already)
Ah, actually I didn’t think about temporaries at all… I changed it to be more in line with Serialize which uses const Args&... args, so Args&... args felt more appropriate.
I’ll change it back to &&
767-inline void Serialize(Stream& os, const prevector<N, T>& v)
768-{
769- Serialize_impl(os, v, T());
770+void Serialize(Stream& os, const prevector<N, T>& v)
771+{
772+ if constexpr (std::is_same_v<T, unsigned char>) {
is_same_v works, because I presume T can never include const in a prevector, similar to how std::vector<const int> does not exist.
very nice, ACK 513f4659b0a2ac4e2454ad2425b0fec8d7f64f04 🙊
Signature:
0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: very nice, ACK 513f4659b0a2ac4e2454ad2425b0fec8d7f64f04 🙊
3GEzCyPu57ooJeHDaJ2Nuwgozpyvft2GzjmZzH1LsuCmxm4NLtc+ONqj570HEbSuzwFAnZ/pZOvN9NSTjIWsxDw==
Instead of recursively calling `SerializeMany` and peeling off one
argument at a time, use a fold expression. 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.
Instead of recursively calling `UnserializeMany` and peeling off one
argument at a time, use a fold expression. 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.
This gets rid of unnecessarily creating a temporary object T() to call
the right function.
This gets rid of unnecessarily creating a temporary object T() to call
the right function.
This gets rid of unnecessarily creating a temporary object T() to call
the right function.
This gets rid of unnecessarily creating a temporary object T() to call
the right function.
&& in UnserializeMany.
only change is to add a missing &. lgtm, re-ACK f054bd072afb72d8dae7adc521ce15c13b236700 📦
Signature:
0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: only change is to add a missing `&`. lgtm, re-ACK f054bd072afb72d8dae7adc521ce15c13b236700 📦
3qVTNWRmP4fK55EiOWjNDYq7LnIwUAwWHe96FjQdkKJV0imPWgVxuLQVRwAbyPEiKRxWUArJKVfajpftB3UDzCw==
