From 001d04bfc89c3dc9ab06f703ff15cbb1c66f1dc9 Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" Date: Wed, 30 Oct 2019 17:00:22 +0100 Subject: openpgp,buffered-reader: Optimize Vec::truncate manually - On debug builds, Vec::truncate is very, very slow. For instance, running the decrypt_test_stream test takes 51 seconds on my (Neal's) computer using Vec::truncate and <0.1 seconds using `unsafe { v.set_len(len); }`. The issue is that the compiler calls drop on every element that is dropped, even though a u8 doesn't have a drop implementation. The compiler optimizes this away at high optimization levels, but those levels make debugging harder. --- buffered-reader/src/generic.rs | 3 +-- buffered-reader/src/lib.rs | 19 +++++++++++++++++++ openpgp/src/armor.rs | 3 ++- openpgp/src/crypto/ecdh.rs | 3 ++- openpgp/src/crypto/symmetric.rs | 7 ++++--- openpgp/src/lib.rs | 19 +++++++++++++++++++ openpgp/src/parse/partial_body.rs | 5 +++-- openpgp/src/serialize/mod.rs | 4 ++-- openpgp/src/serialize/tpk.rs | 7 ++++--- 9 files changed, 56 insertions(+), 14 deletions(-) diff --git a/buffered-reader/src/generic.rs b/buffered-reader/src/generic.rs index de1a52c7..7b5dd540 100644 --- a/buffered-reader/src/generic.rs +++ b/buffered-reader/src/generic.rs @@ -131,7 +131,6 @@ impl Generic { if amount_read > 0 { // We read something. - if let Some(ref buffer) = self.buffer { // We need to copy in the old data. buffer_new[0..amount_buffered] @@ -139,7 +138,7 @@ impl Generic { &buffer[self.cursor..self.cursor + amount_buffered]); } - buffer_new.truncate(amount_buffered + amount_read); + vec_truncate(&mut buffer_new, amount_buffered + amount_read); buffer_new.shrink_to_fit(); self.buffer = Some(buffer_new.into_boxed_slice()); diff --git a/buffered-reader/src/lib.rs b/buffered-reader/src/lib.rs index 129eaa9c..bf0f899f 100644 --- a/buffered-reader/src/lib.rs +++ b/buffered-reader/src/lib.rs @@ -275,6 +275,25 @@ pub use self::file_unix::File; // The default buffer size. const DEFAULT_BUF_SIZE: usize = 8 * 1024; +// On debug builds, Vec::truncate is very, very slow. For +// instance, running the decrypt_test_stream test takes 51 seconds on +// my (Neal's) computer using Vec::truncate and <0.1 seconds using +// `unsafe { v.set_len(len); }`. +// +// The issue is that the compiler calls drop on every element that is +// dropped, even though a u8 doesn't have a drop implementation. The +// compiler optimizes this away at high optimization levels, but those +// levels make debugging harder. +fn vec_truncate(v: &mut Vec, len: usize) { + if cfg!(debug_assertions) { + if len < v.len() { + unsafe { v.set_len(len); } + } + } else { + v.truncate(len); + } +} + /// The generic `BufferReader` interface. pub trait BufferedReader : io::Read + fmt::Debug + fmt::Display { /// Returns a reference to the internal buffer. diff --git a/openpgp/src/armor.rs b/openpgp/src/armor.rs index 85a0a967..c593fdee 100644 --- a/openpgp/src/armor.rs +++ b/openpgp/src/armor.rs @@ -35,6 +35,7 @@ use std::str; use std::borrow::Cow; use quickcheck::{Arbitrary, Gen}; +use crate::vec_truncate; use crate::packet::prelude::*; use crate::packet::header::BodyLength; use crate::packet::header::ctb::{CTBNew, CTBOld}; @@ -931,7 +932,7 @@ fn base64_filter(mut bytes: Cow<[u8]>, base64_data_max: usize, (Cow::Borrowed(&s[..base64_len]), unfiltered_complete_len, prefix_remaining), Cow::Owned(mut v) => { - v.truncate(base64_len); + vec_truncate(&mut v, base64_len); (Cow::Owned(v), unfiltered_complete_len, prefix_remaining) } } diff --git a/openpgp/src/crypto/ecdh.rs b/openpgp/src/crypto/ecdh.rs index d16de842..15228f59 100644 --- a/openpgp/src/crypto/ecdh.rs +++ b/openpgp/src/crypto/ecdh.rs @@ -1,5 +1,6 @@ //! Elliptic Curve Diffie-Hellman. +use crate::vec_truncate; use crate::Error; use crate::packet::{ Key, @@ -426,7 +427,7 @@ pub fn pkcs5_unpad(sk: Protected, target_len: usize) -> Result { } if good { - buf.truncate(target_len); + vec_truncate(&mut buf, target_len); Ok(buf.into()) } else { let sk: Protected = buf.into(); diff --git a/openpgp/src/crypto/symmetric.rs b/openpgp/src/crypto/symmetric.rs index 5ca999ad..e1d78275 100644 --- a/openpgp/src/crypto/symmetric.rs +++ b/openpgp/src/crypto/symmetric.rs @@ -7,6 +7,7 @@ use std::fmt; use crate::Result; use crate::Error; use crate::SymmetricAlgorithm; +use crate::vec_truncate; use buffered_reader::BufferedReader; @@ -221,7 +222,7 @@ impl io::Read for Decryptor { Ok(amount) => { short_read = amount < to_copy; to_copy = amount; - ciphertext.truncate(to_copy); + vec_truncate(&mut ciphertext, to_copy); }, // We encountered an error, but we did read some. Err(_) if pos > 0 => return Ok(pos), @@ -251,7 +252,7 @@ impl io::Read for Decryptor { Ok(amount) => { // Make sure `ciphertext` is not larger than the // amount of data that was actually read. - ciphertext.truncate(amount); + vec_truncate(&mut ciphertext, amount); // Make sure we don't read more than is available. to_copy = cmp::min(to_copy, ciphertext.len()); @@ -265,7 +266,7 @@ impl io::Read for Decryptor { while self.buffer.len() < ciphertext.len() { self.buffer.push(0u8); } - self.buffer.truncate(ciphertext.len()); + vec_truncate(&mut self.buffer, ciphertext.len()); self.dec.decrypt(&mut self.iv, &mut self.buffer, &ciphertext[..]) .map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, diff --git a/openpgp/src/lib.rs b/openpgp/src/lib.rs index 6886e23d..1dede3d9 100644 --- a/openpgp/src/lib.rs +++ b/openpgp/src/lib.rs @@ -73,6 +73,25 @@ extern crate idna; #[macro_use] mod macros; +// On debug builds, Vec::truncate is very, very slow. For +// instance, running the decrypt_test_stream test takes 51 seconds on +// my (Neal's) computer using Vec::truncate and <0.1 seconds using +// `unsafe { v.set_len(len); }`. +// +// The issue is that the compiler calls drop on every element that is +// dropped, even though a u8 doesn't have a drop implementation. The +// compiler optimizes this away at high optimization levels, but those +// levels make debugging harder. +fn vec_truncate(v: &mut Vec, len: usize) { + if cfg!(debug_assertions) { + if len < v.len() { + unsafe { v.set_len(len); } + } + } else { + v.truncate(len); + } +} + // Like assert!, but checks a pattern. // // assert_match!(Some(_) = x); diff --git a/openpgp/src/parse/partial_body.rs b/openpgp/src/parse/partial_body.rs index bc4e6309..222e2894 100644 --- a/openpgp/src/parse/partial_body.rs +++ b/openpgp/src/parse/partial_body.rs @@ -4,6 +4,8 @@ use std::io; use std::io::{Error, ErrorKind}; use buffered_reader::{buffered_reader_generic_read_impl, BufferedReader}; + +use crate::vec_truncate; use crate::packet::header::BodyLength; use crate::parse::{Cookie, Hashing}; @@ -203,8 +205,7 @@ impl> BufferedReaderPartialBodyFilter { } } } - - buffer.truncate(amount_buffered); + vec_truncate(&mut buffer, amount_buffered); buffer.shrink_to_fit(); // We're done. diff --git a/openpgp/src/serialize/mod.rs b/openpgp/src/serialize/mod.rs index 96f8b17d..609a3b28 100644 --- a/openpgp/src/serialize/mod.rs +++ b/openpgp/src/serialize/mod.rs @@ -92,7 +92,7 @@ pub trait SerializeInto { fn to_vec(&self) -> Result> { let mut o = vec![0; self.serialized_len()]; let len = self.serialize_into(&mut o[..])?; - o.truncate(len); + vec_truncate(&mut o, len); o.shrink_to_fit(); Ok(o) } @@ -139,7 +139,7 @@ pub trait SerializeInto { fn export_to_vec(&self) -> Result> { let mut o = vec![0; self.serialized_len()]; let len = self.export_into(&mut o[..])?; - o.truncate(len); + vec_truncate(&mut o, len); o.shrink_to_fit(); Ok(o) } diff --git a/openpgp/src/serialize/tpk.rs b/openpgp/src/serialize/tpk.rs index 0e8324b5..520cb684 100644 --- a/openpgp/src/serialize/tpk.rs +++ b/openpgp/src/serialize/tpk.rs @@ -640,6 +640,7 @@ impl<'a> SerializeInto for TSK<'a> { #[cfg(test)] mod test { use super::*; + use crate::vec_truncate; use crate::parse::Parse; use crate::serialize::Serialize; use crate::packet::key; @@ -785,7 +786,7 @@ mod test { let mut buf = vec![0; tpk.serialized_len()]; let l = tpk.export_into(&mut buf).unwrap(); - buf.truncate(l); + vec_truncate(&mut buf, l); let tpk_ = TPK::from_bytes(&buf).unwrap(); assert_eq!(tpk_.subkeys().count(), 0); assert_eq!(tpk_.userids().count(), 0); @@ -806,7 +807,7 @@ mod test { let mut buf = vec![0; tpk.serialized_len()]; let l = tpk.armored().export_into(&mut buf).unwrap(); - buf.truncate(l); + vec_truncate(&mut buf, l); let tpk_ = TPK::from_bytes(&buf).unwrap(); assert_eq!(tpk_.subkeys().count(), 0); assert_eq!(tpk_.userids().count(), 0); @@ -828,7 +829,7 @@ mod test { let mut buf = vec![0; tpk.serialized_len()]; let l = tpk.as_tsk().export_into(&mut buf).unwrap(); - buf.truncate(l); + vec_truncate(&mut buf, l); let tpk_ = TPK::from_bytes(&buf).unwrap(); assert_eq!(tpk_.subkeys().count(), 0); assert_eq!(tpk_.userids().count(), 0); -- cgit v1.2.3