From c632fffc3ddb98da0ba918a77317fdda504116f3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 29 May 2021 18:24:16 +0000 Subject: [PATCH 1/1] Avoid calling libsecp serialization fns when calculating length When writing out libsecp256k1 objects during serialization in a TLV, we potentially calculate the TLV length twice before performing the actual serialization (once when calculating the total TLV-stream length and once when calculating the length of the secp256k1-object-containing TLV). Because the lengths of secp256k1 objects is a constant, we'd ideally like LLVM to entirely optimize out those calls and simply know the expected length. However, without cross-language LTO, there is no way for LLVM to verify that there are no side-effects of the calls to libsecp256k1, leaving LLVM with no way to optimize them out. This commit adds a new method to `Writeable` which returns the length of an object once serialized. It is implemented by default using `LengthCalculatingWriter` (which LLVM generally optimizes out for Rust objects) and overrides it for libsecp256k1 objects. As of this commit, on an Intel 2687W v3, the serialization benchmarks take: test routing::network_graph::benches::read_network_graph ... bench: 2,035,402,164 ns/iter (+/- 1,855,357) test routing::network_graph::benches::write_network_graph ... bench: 308,235,267 ns/iter (+/- 140,202) --- lightning/src/util/ser.rs | 32 ++++++++++++++++++++++++++------ lightning/src/util/ser_macros.rs | 24 +++++++++--------------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index e971c6c1..61cd1e60 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -19,7 +19,7 @@ use core::cmp; use bitcoin::secp256k1::Signature; use bitcoin::secp256k1::key::{PublicKey, SecretKey}; -use bitcoin::secp256k1::constants::{PUBLIC_KEY_SIZE, COMPACT_SIGNATURE_SIZE}; +use bitcoin::secp256k1::constants::{PUBLIC_KEY_SIZE, SECRET_KEY_SIZE, COMPACT_SIGNATURE_SIZE}; use bitcoin::blockdata::script::Script; use bitcoin::blockdata::transaction::{OutPoint, Transaction, TxOut}; use bitcoin::consensus; @@ -185,6 +185,16 @@ pub trait Writeable { msg.0[..2].copy_from_slice(&(len as u16 - 2).to_be_bytes()); msg.0 } + + /// Gets the length of this object after it has been serialized. This can be overridden to + /// optimize cases where we prepend an object with its length. + // Note that LLVM optimizes this away in most cases! Check that it isn't before you override! + #[inline] + fn serialized_length(&self) -> usize { + let mut len_calc = LengthCalculatingWriter(0); + self.write(&mut len_calc).expect("No in-memory data may fail to serialize"); + len_calc.0 + } } impl<'a, T: Writeable> Writeable for &'a T { @@ -561,6 +571,10 @@ impl Writeable for PublicKey { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { self.serialize().write(w) } + #[inline] + fn serialized_length(&self) -> usize { + PUBLIC_KEY_SIZE + } } impl Readable for PublicKey { @@ -575,15 +589,19 @@ impl Readable for PublicKey { impl Writeable for SecretKey { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { - let mut ser = [0; 32]; + let mut ser = [0; SECRET_KEY_SIZE]; ser.copy_from_slice(&self[..]); ser.write(w) } + #[inline] + fn serialized_length(&self) -> usize { + SECRET_KEY_SIZE + } } impl Readable for SecretKey { fn read(r: &mut R) -> Result { - let buf: [u8; 32] = Readable::read(r)?; + let buf: [u8; SECRET_KEY_SIZE] = Readable::read(r)?; match SecretKey::from_slice(&buf) { Ok(key) => Ok(key), Err(_) => return Err(DecodeError::InvalidValue), @@ -610,6 +628,10 @@ impl Writeable for Signature { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { self.serialize_compact().write(w) } + #[inline] + fn serialized_length(&self) -> usize { + COMPACT_SIGNATURE_SIZE + } } impl Readable for Signature { @@ -666,9 +688,7 @@ impl Writeable for Option { match *self { None => 0u8.write(w)?, Some(ref data) => { - let mut len_calc = LengthCalculatingWriter(0); - data.write(&mut len_calc).expect("No in-memory data may fail to serialize"); - BigSize(len_calc.0 as u64 + 1).write(w)?; + BigSize(data.serialized_length() as u64 + 1).write(w)?; data.write(w)?; } } diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index b8119ddd..745d3dd1 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -10,7 +10,7 @@ macro_rules! encode_tlv { ($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { { #[allow(unused_imports)] - use util::ser::{BigSize, LengthCalculatingWriter}; + use util::ser::BigSize; // Fields must be serialized in order, so we have to potentially switch between optional // fields and normal fields while serializing. Thus, we end up having to loop over the type // counts. @@ -30,9 +30,7 @@ macro_rules! encode_tlv { $( if i == $type { BigSize($type).write($stream)?; - let mut len_calc = LengthCalculatingWriter(0); - $field.write(&mut len_calc)?; - BigSize(len_calc.0 as u64).write($stream)?; + BigSize($field.serialized_length() as u64).write($stream)?; $field.write($stream)?; } )* @@ -40,9 +38,7 @@ macro_rules! encode_tlv { if i == $optional_type { if let Some(ref field) = $optional_field { BigSize($optional_type).write($stream)?; - let mut len_calc = LengthCalculatingWriter(0); - field.write(&mut len_calc)?; - BigSize(len_calc.0 as u64).write($stream)?; + BigSize(field.serialized_length() as u64).write($stream)?; field.write($stream)?; } } @@ -59,18 +55,16 @@ macro_rules! encode_varint_length_prefixed_tlv { { $( BigSize($type).write(&mut len)?; - let mut field_len = LengthCalculatingWriter(0); - $field.write(&mut field_len)?; - BigSize(field_len.0 as u64).write(&mut len)?; - len.0 += field_len.0; + let field_len = $field.serialized_length(); + BigSize(field_len as u64).write(&mut len)?; + len.0 += field_len; )* $( if let Some(ref field) = $optional_field { BigSize($optional_type).write(&mut len)?; - let mut field_len = LengthCalculatingWriter(0); - field.write(&mut field_len)?; - BigSize(field_len.0 as u64).write(&mut len)?; - len.0 += field_len.0; + let field_len = field.serialized_length(); + BigSize(field_len as u64).write(&mut len)?; + len.0 += field_len; } )* } -- 2.30.2