Skip to content

Commit e6949a1

Browse files
committed
Drop writer size hinting/message vec preallocation
In order to avoid significant malloc traffic, messages previously explicitly stated their serialized length allowing for Vec preallocation during the message serialization pipeline. This added some amount of complexity in the serialization code, but did avoid some realloc() calls. Instead, here, we drop all the complexity in favor of a fixed 2KiB buffer for all message serialization. This should not only be simpler with a similar reduction in realloc() traffic, but also may reduce heap fragmentation by allocating identically-sized buffers more often.
1 parent 91a7000 commit e6949a1

File tree

10 files changed

+31
-176
lines changed

10 files changed

+31
-176
lines changed

fuzz/src/chanmon_consistency.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,6 @@ impl Writer for VecWriter {
9494
self.0.extend_from_slice(buf);
9595
Ok(())
9696
}
97-
fn size_hint(&mut self, size: usize) {
98-
self.0.reserve_exact(size);
99-
}
10097
}
10198

10299
struct TestChainMonitor {

fuzz/src/chanmon_deser.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@ impl Writer for VecWriter {
1818
self.0.extend_from_slice(buf);
1919
Ok(())
2020
}
21-
fn size_hint(&mut self, size: usize) {
22-
self.0.reserve_exact(size);
23-
}
2421
}
2522

2623
#[inline]

fuzz/src/msg_targets/utils.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,9 @@ use lightning::util::ser::Writer;
1313
pub struct VecWriter(pub Vec<u8>);
1414
impl Writer for VecWriter {
1515
fn write_all(&mut self, buf: &[u8]) -> Result<(), ::std::io::Error> {
16-
assert!(self.0.capacity() >= self.0.len() + buf.len());
1716
self.0.extend_from_slice(buf);
1817
Ok(())
1918
}
20-
fn size_hint(&mut self, size: usize) {
21-
self.0.reserve_exact(size);
22-
}
2319
}
2420

2521
// We attempt to test the strictest behavior we can for a given message, however, some messages
@@ -43,6 +39,7 @@ macro_rules! test_msg {
4339
msg.write(&mut w).unwrap();
4440

4541
assert_eq!(w.0.len(), p);
42+
assert_eq!(msg.serialized_length(), p);
4643
assert_eq!(&r.into_inner()[..p], &w.0[..p]);
4744
}
4845
}
@@ -60,6 +57,7 @@ macro_rules! test_msg_simple {
6057
if let Ok(msg) = <$MsgType as Readable>::read(&mut r) {
6158
let mut w = VecWriter(Vec::new());
6259
msg.write(&mut w).unwrap();
60+
assert_eq!(msg.serialized_length(), w.0.len());
6361

6462
let msg = <$MsgType as Readable>::read(&mut ::std::io::Cursor::new(&w.0)).unwrap();
6563
let mut w_two = VecWriter(Vec::new());
@@ -82,6 +80,7 @@ macro_rules! test_msg_exact {
8280
let mut w = VecWriter(Vec::new());
8381
msg.write(&mut w).unwrap();
8482
assert_eq!(&r.into_inner()[..], &w.0[..]);
83+
assert_eq!(msg.serialized_length(), w.0.len());
8584
}
8685
}
8786
}
@@ -99,6 +98,7 @@ macro_rules! test_msg_hole {
9998
let mut w = VecWriter(Vec::new());
10099
msg.write(&mut w).unwrap();
101100
let p = w.0.len() as usize;
101+
assert_eq!(msg.serialized_length(), p);
102102

103103
assert_eq!(w.0.len(), p);
104104
assert_eq!(&r.get_ref()[..$hole], &w.0[..$hole]);

lightning/src/chain/transaction.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ impl OutPoint {
7575
}
7676
}
7777

78-
impl_writeable!(OutPoint, 0, { txid, index });
78+
impl_writeable!(OutPoint, { txid, index });
7979

8080
#[cfg(test)]
8181
mod tests {

lightning/src/ln/features.rs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,6 @@ impl InitFeatures {
392392
/// Writes all features present up to, and including, 13.
393393
pub(crate) fn write_up_to_13<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
394394
let len = cmp::min(2, self.flags.len());
395-
w.size_hint(len + 2);
396395
(len as u16).write(w)?;
397396
for i in (0..len).rev() {
398397
if i == 0 {
@@ -584,12 +583,6 @@ impl<T: sealed::Context> Features<T> {
584583
(byte & unknown_features) != 0
585584
})
586585
}
587-
588-
/// The number of bytes required to represent the feature flags present. This does not include
589-
/// the length bytes which are included in the serialized form.
590-
pub(crate) fn byte_count(&self) -> usize {
591-
self.flags.len()
592-
}
593586
}
594587

595588
impl<T: sealed::DataLossProtect> Features<T> {
@@ -702,7 +695,6 @@ impl<T: sealed::ShutdownAnySegwit> Features<T> {
702695

703696
impl<T: sealed::Context> Writeable for Features<T> {
704697
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
705-
w.size_hint(self.flags.len() + 2);
706698
(self.flags.len() as u16).write(w)?;
707699
for f in self.flags.iter().rev() { // Swap back to big-endian
708700
f.write(w)?;
@@ -835,7 +827,7 @@ mod tests {
835827
#[test]
836828
fn convert_to_context_with_unknown_flags() {
837829
// Ensure the `from` context has fewer known feature bytes than the `to` context.
838-
assert!(InvoiceFeatures::known().byte_count() < NodeFeatures::known().byte_count());
830+
assert!(InvoiceFeatures::known().flags.len() < NodeFeatures::known().flags.len());
839831
let invoice_features = InvoiceFeatures::known().set_unknown_feature_optional();
840832
assert!(invoice_features.supports_unknown_bits());
841833
let node_features: NodeFeatures = invoice_features.to_context();

0 commit comments

Comments
 (0)