From 583e6ac6f0bf1fd311621ea7584d160bd6d7c192 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 29 May 2021 18:32:53 +0000 Subject: [PATCH] Impl `serialized_length()` without `LengthCalculatingWriter` With the new `serialized_length()` method potentially being significantly more efficient than `LengthCalculatingWriter`, this commit ensures we call `serialized_length()` when calculating length of a larger struct. Specifically, prior to this commit a call to `serialized_length()` on a large object serialized with `impl_writeable`, `impl_writeable_len_match`, or `encode_varint_length_prefixed_tlv` (and `impl_writeable_tlv_based`) would always serialize all inner fields of that object using `LengthCalculatingWriter`. This would ignore any `serialized_length()` overrides by inner fields. Instead, we override `serialized_length()` on all of the above by calculating the serialized size using calls to `serialized_length()` on inner fields. Further, writes to `LengthCalculatingWriter` should never fail as its `write` method never returns an error. Thus, any write failures indicate a bug in an object's write method or in our object-creation sanity checking. We `.expect()` such write calls here. As of this commit, on an Intel 2687W v3, the serialization benchmarks take: test routing::network_graph::benches::read_network_graph ... bench: 2,039,451,296 ns/iter (+/- 4,329,821) test routing::network_graph::benches::write_network_graph ... bench: 166,685,412 ns/iter (+/- 352,537) --- lightning/src/util/ser_macros.rs | 91 +++++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 12 deletions(-) diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 745d3dd17..45435ecdd 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -47,29 +47,36 @@ macro_rules! encode_tlv { } } } -macro_rules! encode_varint_length_prefixed_tlv { - ($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { { - use util::ser::{BigSize, LengthCalculatingWriter}; +macro_rules! get_varint_length_prefixed_tlv_length { + ({$(($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)?; + BigSize($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)?; + 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)?; + BigSize($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)?; + BigSize(field_len as u64).write(&mut len).expect("No in-memory data may fail to serialize"); len.0 += field_len; } )* } + len.0 + } } +} - BigSize(len.0 as u64).write($stream)?; +macro_rules! encode_varint_length_prefixed_tlv { + ($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)),* }); + BigSize(len as u64).write($stream)?; encode_tlv!($stream, { $(($type, $field)),* }, { $(($optional_type, $optional_field)),* }); } } } @@ -167,13 +174,29 @@ macro_rules! impl_writeable { if $len != 0 { use util::ser::LengthCalculatingWriter; let mut len_calc = LengthCalculatingWriter(0); - $( self.$field.write(&mut len_calc)?; )* + $( self.$field.write(&mut len_calc).expect("No in-memory data may fail to serialize"); )* assert_eq!(len_calc.0, $len); + assert_eq!(self.serialized_length(), $len); } } $( self.$field.write(w)?; )* Ok(()) } + + #[inline] + fn serialized_length(&self) -> usize { + if $len == 0 || cfg!(any(test, feature = "fuzztarget")) { + let mut len_calc = 0; + $( len_calc += self.$field.serialized_length(); )* + if $len != 0 { + // In tests, assert that the hard-coded length matches the actual one + assert_eq!(len_calc, $len); + } else { + return len_calc; + } + } + $len + } } impl ::util::ser::Readable for $st { @@ -186,7 +209,7 @@ macro_rules! impl_writeable { } } macro_rules! impl_writeable_len_match { - ($struct: ident, $cmp: tt, {$({$match: pat, $length: expr}),*}, {$($field:ident),*}) => { + ($struct: ident, $cmp: tt, ($calc_len: expr), {$({$match: pat, $length: expr}),*}, {$($field:ident),*}) => { impl Writeable for $struct { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { let len = match *self { @@ -198,12 +221,30 @@ macro_rules! impl_writeable_len_match { // In tests, assert that the hard-coded length matches the actual one use util::ser::LengthCalculatingWriter; let mut len_calc = LengthCalculatingWriter(0); - $( self.$field.write(&mut len_calc)?; )* + $( self.$field.write(&mut len_calc).expect("No in-memory data may fail to serialize"); )* assert!(len_calc.0 $cmp len); + assert_eq!(len_calc.0, self.serialized_length()); } $( self.$field.write(w)?; )* Ok(()) } + + #[inline] + fn serialized_length(&self) -> usize { + if $calc_len || cfg!(any(test, feature = "fuzztarget")) { + let mut len_calc = 0; + $( len_calc += self.$field.serialized_length(); )* + if !$calc_len { + assert_eq!(len_calc, match *self { + $($match => $length,)* + }); + } + return len_calc + } + match *self { + $($match => $length,)* + } + } } impl ::util::ser::Readable for $struct { @@ -214,8 +255,11 @@ macro_rules! impl_writeable_len_match { } } }; + ($struct: ident, $cmp: tt, {$({$match: pat, $length: expr}),*}, {$($field:ident),*}) => { + impl_writeable_len_match!($struct, $cmp, (true), { $({ $match, $length }),* }, { $($field),* }); + }; ($struct: ident, {$({$match: pat, $length: expr}),*}, {$($field:ident),*}) => { - impl_writeable_len_match!($struct, ==, { $({ $match, $length }),* }, { $($field),* }); + impl_writeable_len_match!($struct, ==, (false), { $({ $match, $length }),* }, { $($field),* }); } } @@ -319,6 +363,14 @@ macro_rules! _write_tlv_fields { write_tlv_fields!($stream, {$(($type, $field)),*} , {$(($optional_type, $optional_field)),*, $(($optional_type_2, $optional_field_2)),*}); } } +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)),*}) + }; + ({$(($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)),*}) + } +} macro_rules! _read_tlv_fields { ($stream: expr, {$(($reqtype: expr, $reqfield: ident)),* $(,)*}, {}, {$(($type: expr, $field: ident)),* $(,)*}) => { read_tlv_fields!($stream, {$(($reqtype, $reqfield)),*}, {$(($type, $field)),*}); @@ -346,6 +398,21 @@ macro_rules! impl_writeable_tlv_based { }); Ok(()) } + + #[inline] + fn serialized_length(&self) -> usize { + let len = _get_tlv_len!({ + $(($reqtype, self.$reqfield)),* + }, { + $(($type, self.$field)),* + }, { + $(($vectype, Some(::util::ser::VecWriteWrapper(&self.$vecfield)))),* + }); + use util::ser::{BigSize, LengthCalculatingWriter}; + let mut len_calc = LengthCalculatingWriter(0); + BigSize(len as u64).write(&mut len_calc).expect("No in-memory data may fail to serialize"); + len + len_calc.0 + } } impl ::util::ser::Readable for $st { -- 2.39.5