From 17204a0d7f2e2a38aa547711462459aa8e3dd5ac Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 28 May 2021 18:43:43 +0000 Subject: [PATCH] Use u8 for TLV type when writing and reading our own structs Once inlined, a very nontrivial portion of the total code generated for reading TLVs is BigSize deserialization. In some naive testing, as much as 50% of total code size is dedicated to the combination of type and length deserialization. While we're stuck with BigSize deserialization for network messages there is nothing requiring we use BigSizes for our own messages, and specifically for types, unless we have > 127 required/non-required types the serialization format remains the same. Thus, we adapt our TLV macros to support either BigSize or U8Wrapper (ie u8 with a wrapper to make it API-compatible with BigSize) for types depending on the context. This results in somewhat less code, though a dissapointingly small performance improvement. Future work, however, can further compact the deserialization logic by taking advantage of known lengths per type. As of this commit, on an Intel 2687W v3, the serialization benchmarks take: test routing::network_graph::benches::read_network_graph ... bench: 1,962,012,392 ns/iter (+/- 2,391,402) test routing::network_graph::benches::write_network_graph ... bench: 166,772,461 ns/iter (+/- 295,302) --- lightning/src/util/ser.rs | 14 +++++++++++ lightning/src/util/ser_macros.rs | 43 +++++++++++++++++++------------- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 26bc79231..af67dca79 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -249,6 +249,20 @@ impl Readable for OptionDeserWrapper { } } +pub(crate) struct U8Wrapper(pub u8); +impl Writeable for U8Wrapper { + #[inline(always)] + fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { + self.0.write(writer) + } +} +impl Readable for U8Wrapper { + #[inline(always)] + fn read(reader: &mut R) -> Result { + Ok(Self(Readable::read(reader)?)) + } +} + const MAX_ALLOC_SIZE: u64 = 64*1024; pub(crate) struct VecWriteWrapper<'a, T: Writeable>(pub &'a Vec); diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 2693fba77..5f37fb996 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -8,7 +8,10 @@ // licenses. macro_rules! encode_tlv { - ($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { { + ($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { + encode_tlv!(::util::ser::BigSize, $stream, {$(($type, $field)),*}, {$(($optional_type, $optional_field)),*}); + }; + ($name_ty: path, $stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { { #[allow(unused_imports)] use util::ser::BigSize; // Fields must be serialized in order, so we have to potentially switch between optional @@ -20,16 +23,16 @@ macro_rules! encode_tlv { #[allow(unused_mut)] let mut max_field: u64 = 0; $( - if $type >= max_field { max_field = $type + 1; } + if $type + 1 >= max_field { max_field = $type + 1; } )* $( - if $optional_type >= max_field { max_field = $optional_type + 1; } + if $optional_type + 1 >= max_field { max_field = $optional_type + 1; } )* #[allow(unused_variables)] for i in 0..max_field { $( if i == $type { - BigSize($type).write($stream)?; + $name_ty($type).write($stream)?; BigSize($field.serialized_length() as u64).write($stream)?; $field.write($stream)?; } @@ -37,7 +40,7 @@ macro_rules! encode_tlv { $( if i == $optional_type { if let Some(ref field) = $optional_field { - BigSize($optional_type).write($stream)?; + $name_ty($optional_type).write($stream)?; BigSize(field.serialized_length() as u64).write($stream)?; field.write($stream)?; } @@ -48,20 +51,20 @@ macro_rules! encode_tlv { } macro_rules! get_varint_length_prefixed_tlv_length { - ({$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),* $(,)*}) => { { + ($name_ty: path, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),* $(,)*}) => { { use util::ser::LengthCalculatingWriter; #[allow(unused_mut)] let mut len = LengthCalculatingWriter(0); { $( - BigSize($type).write(&mut len).expect("No in-memory data may fail to serialize"); + $name_ty($type).write(&mut len).expect("No in-memory data may fail to serialize"); let field_len = $field.serialized_length(); BigSize(field_len as u64).write(&mut len).expect("No in-memory data may fail to serialize"); len.0 += field_len; )* $( if let Some(ref field) = $optional_field { - BigSize($optional_type).write(&mut len).expect("No in-memory data may fail to serialize"); + $name_ty($optional_type).write(&mut len).expect("No in-memory data may fail to serialize"); let field_len = field.serialized_length(); BigSize(field_len as u64).write(&mut len).expect("No in-memory data may fail to serialize"); len.0 += field_len; @@ -73,23 +76,29 @@ macro_rules! get_varint_length_prefixed_tlv_length { } macro_rules! encode_varint_length_prefixed_tlv { - ($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { { + ($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { + encode_varint_length_prefixed_tlv!(::util::ser::BigSize, $stream, {$(($type, $field)),*}, {$(($optional_type, $optional_field)),*}); + }; + ($name_ty: path, $stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { { use util::ser::BigSize; - let len = get_varint_length_prefixed_tlv_length!({ $(($type, $field)),* }, { $(($optional_type, $optional_field)),* }); + let len = get_varint_length_prefixed_tlv_length!($name_ty, { $(($type, $field)),* }, { $(($optional_type, $optional_field)),* }); BigSize(len as u64).write($stream)?; encode_tlv!($stream, { $(($type, $field)),* }, { $(($optional_type, $optional_field)),* }); } } } macro_rules! decode_tlv { - ($stream: expr, {$(($reqtype: expr, $reqfield: ident)),*}, {$(($type: expr, $field: ident)),*}) => { { + ($stream: expr, {$(($reqtype: expr, $reqfield: ident)),*}, {$(($type: expr, $field: ident)),*}) => { + decode_tlv!(::util::ser::BigSize, $stream, {$(($reqtype, $reqfield)),*}, {$(($type, $field)),*}); + }; + ($name_ty: path, $stream: expr, {$(($reqtype: expr, $reqfield: ident)),*}, {$(($type: expr, $field: ident)),*}) => { { use ln::msgs::DecodeError; - let mut last_seen_type: Option = None; + let mut last_seen_type = None; 'tlv_read: loop { use util::ser; // First decode the type of this TLV: - let typ: ser::BigSize = { + let typ: $name_ty = { // We track whether any bytes were read during the consensus_decode call to // determine whether we should break or return ShortRead if we get an // UnexpectedEof. This should in every case be largely cosmetic, but its nice to @@ -291,7 +300,7 @@ macro_rules! write_ver_prefix { /// correctly. macro_rules! write_tlv_fields { ($stream: expr, {$(($type: expr, $field: expr)),* $(,)*}, {$(($optional_type: expr, $optional_field: expr)),* $(,)*}) => { - encode_varint_length_prefixed_tlv!($stream, {$(($type, $field)),*} , {$(($optional_type, $optional_field)),*}); + encode_varint_length_prefixed_tlv!(::util::ser::U8Wrapper, $stream, {$(($type, $field)),*} , {$(($optional_type, $optional_field)),*}); } } @@ -314,7 +323,7 @@ macro_rules! read_tlv_fields { ($stream: expr, {$(($reqtype: expr, $reqfield: ident)),* $(,)*}, {$(($type: expr, $field: ident)),* $(,)*}) => { { let tlv_len = ::util::ser::BigSize::read($stream)?; let mut rd = ::util::ser::FixedLengthReader::new($stream, tlv_len.0); - decode_tlv!(&mut rd, {$(($reqtype, $reqfield)),*}, {$(($type, $field)),*}); + decode_tlv!(::util::ser::U8Wrapper, &mut rd, {$(($reqtype, $reqfield)),*}, {$(($type, $field)),*}); rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?; } } } @@ -363,10 +372,10 @@ macro_rules! _write_tlv_fields { } macro_rules! _get_tlv_len { ({$(($type: expr, $field: expr)),* $(,)*}, {}, {$(($optional_type: expr, $optional_field: expr)),* $(,)*}) => { - get_varint_length_prefixed_tlv_length!({$(($type, $field)),*} , {$(($optional_type, $optional_field)),*}) + get_varint_length_prefixed_tlv_length!(::util::ser::U8Wrapper, {$(($type, $field)),*} , {$(($optional_type, $optional_field)),*}) }; ({$(($type: expr, $field: expr)),* $(,)*}, {$(($optional_type: expr, $optional_field: expr)),* $(,)*}, {$(($optional_type_2: expr, $optional_field_2: expr)),* $(,)*}) => { - get_varint_length_prefixed_tlv_length!({$(($type, $field)),*} , {$(($optional_type, $optional_field)),*, $(($optional_type_2, $optional_field_2)),*}) + get_varint_length_prefixed_tlv_length!(::util::ser::U8Wrapper, {$(($type, $field)),*} , {$(($optional_type, $optional_field)),*, $(($optional_type_2, $optional_field_2)),*}) } } macro_rules! _read_tlv_fields { -- 2.39.5