Avoid calling libsecp serialization fns when calculating length
authorMatt Corallo <git@bluematt.me>
Sat, 29 May 2021 18:24:16 +0000 (18:24 +0000)
committerMatt Corallo <git@bluematt.me>
Tue, 1 Jun 2021 15:47:01 +0000 (15:47 +0000)
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
lightning/src/util/ser_macros.rs

index e971c6c145142a1f677437afccc51bc107c287b8..61cd1e60de0ae0a57691fa8b5b858c766c451523 100644 (file)
@@ -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<W: Writer>(&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<W: Writer>(&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: Read>(r: &mut R) -> Result<Self, DecodeError> {
-               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<W: Writer>(&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<T: Writeable> Writeable for Option<T> {
                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)?;
                        }
                }
index b8119ddd5bd26a9ddb1580f2ef09c660110a7a2f..745d3dd17bc47d71254661c1cdc2fbd315c0af6e 100644 (file)
@@ -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;
                                }
                        )*
                }