From 9619dffaa756d8cfd6ec691699bd6ae4d59ecfb5 Mon Sep 17 00:00:00 2001
From: Justus Winter <justus@sequoia-pgp.org>
Date: Tue, 10 Apr 2018 12:22:41 +0200
Subject: buffered-reader: Fix performance problem.

  - Previously, BufferedReaderLimitor::into_inner() boxed its reader,
    creating a linked list of redirections to follow whenever the
    reader was used.  This lead to a significant slowdown.

  - We can fix this by making BufferedReaderLimitor polymorphic over
    boxed BufferedReaders.  Now into_inner() can just return the
    already boxed reader.

  - All non-trivial implementations of BufferedReader::into_inner()
    have this problem, however, the BufferedReaderLimitor is used once
    for every parsed packet, so it was the most pressing one to fix.

  - In the future, we may make BufferedReader::into_inner() return an
    'impl BufferedReader' instead.
---
 buffered-reader/src/lib.rs     |  2 +-
 buffered-reader/src/limitor.rs | 26 +++++++++++++-------------
 openpgp/src/parse/parse.rs     |  4 ++--
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/buffered-reader/src/lib.rs b/buffered-reader/src/lib.rs
index 0660f729..ae2d44ed 100644
--- a/buffered-reader/src/lib.rs
+++ b/buffered-reader/src/lib.rs
@@ -361,7 +361,7 @@ mod test {
 
         // Try it again with a limitor.
         {
-            let bio = BufferedReaderMemory::new(data);
+            let bio = Box::new(BufferedReaderMemory::new(data));
             let mut bio2 = BufferedReaderLimitor::new(
                 bio, (data.len() / 2) as u64);
             let amount = {
diff --git a/buffered-reader/src/limitor.rs b/buffered-reader/src/limitor.rs
index ee8dc853..eae90b0e 100644
--- a/buffered-reader/src/limitor.rs
+++ b/buffered-reader/src/limitor.rs
@@ -5,14 +5,14 @@ use super::*;
 
 /// A `BufferedReaderLimitor` limits the amount of data that can be
 /// read from a `BufferedReader`.
-pub struct BufferedReaderLimitor<T: BufferedReader<C>, C> {
-    reader: T,
+pub struct BufferedReaderLimitor<'a, C> {
+    reader: Box<'a + BufferedReader<C>>,
     limit: u64,
 
     cookie: C,
 }
 
-impl<T: BufferedReader<C>, C> fmt::Debug for BufferedReaderLimitor<T, C> {
+impl<'a, C> fmt::Debug for BufferedReaderLimitor<'a, C> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.debug_struct("BufferedReaderLimitor")
             .field("limit", &self.limit)
@@ -21,21 +21,21 @@ impl<T: BufferedReader<C>, C> fmt::Debug for BufferedReaderLimitor<T, C> {
     }
 }
 
-impl<T: BufferedReader<()>> BufferedReaderLimitor<T, ()> {
+impl<'a> BufferedReaderLimitor<'a, ()> {
     /// Instantiate a new limitor.  `reader` is the source to wrap.
     /// `limit` is the maximum number of bytes that will be returned
     /// from the source.
-    pub fn new(reader: T, limit: u64) -> Self {
+    pub fn new(reader: Box<'a + BufferedReader<()>>, limit: u64) -> Self {
         Self::with_cookie(reader, limit, ())
     }
 }
 
-impl<T: BufferedReader<C>, C> BufferedReaderLimitor<T, C> {
+impl<'a, C> BufferedReaderLimitor<'a, C> {
     /// Like `new()`, but sets a cookie, which can be retrieved using
     /// the `cookie_ref` and `cookie_mut` methods, and set using
     /// the `cookie_set` method.
-    pub fn with_cookie(reader: T, limit: u64, cookie: C)
-            -> BufferedReaderLimitor<T, C> {
+    pub fn with_cookie(reader: Box<'a + BufferedReader<C>>, limit: u64, cookie: C)
+            -> BufferedReaderLimitor<'a, C> {
         BufferedReaderLimitor {
             reader: reader,
             limit: limit,
@@ -44,14 +44,14 @@ impl<T: BufferedReader<C>, C> BufferedReaderLimitor<T, C> {
     }
 }
 
-impl<T: BufferedReader<C>, C> io::Read for BufferedReaderLimitor<T, C> {
+impl<'a, C> io::Read for BufferedReaderLimitor<'a, C> {
     fn read(&mut self, buf: &mut [u8]) -> Result<usize, io::Error> {
         let len = cmp::min(self.limit, buf.len() as u64) as usize;
         return self.reader.read(&mut buf[0..len]);
     }
 }
 
-impl<T: BufferedReader<C>, C> BufferedReader<C> for BufferedReaderLimitor<T, C> {
+impl<'a, C> BufferedReader<C> for BufferedReaderLimitor<'a, C> {
     fn buffer(&self) -> &[u8] {
         let buf = self.reader.buffer();
         &buf[..cmp::min(buf.len(),
@@ -115,8 +115,8 @@ impl<T: BufferedReader<C>, C> BufferedReader<C> for BufferedReaderLimitor<T, C>
     }
 
     fn into_inner<'b>(self: Box<Self>) -> Option<Box<BufferedReader<C> + 'b>>
-            where Self: 'b {
-        Some(Box::new(self.reader))
+        where Self: 'b {
+        Some(self.reader)
     }
 
     fn cookie_set(&mut self, cookie: C) -> C {
@@ -239,7 +239,7 @@ mod test {
             }
         }
 
-        let reader = BufferedReaderGeneric::new(&input[..], None);
+        let reader = Box::new(BufferedReaderGeneric::new(&input[..], None));
         let size = size / 2;
         let input = &input[..size];
         let mut reader = BufferedReaderLimitor::new(reader, input.len() as u64);
diff --git a/openpgp/src/parse/parse.rs b/openpgp/src/parse/parse.rs
index 23ad7643..8990910c 100644
--- a/openpgp/src/parse/parse.rs
+++ b/openpgp/src/parse/parse.rs
@@ -289,7 +289,7 @@ impl Unknown {
     }
 }
 
-pub fn to_unknown_packet<R: io::Read>(reader: R)
+pub fn to_unknown_packet<R: Read>(reader: R)
         -> Result<Unknown> {
     let mut reader = BufferedReaderGeneric::with_cookie(
         reader, None, BufferedReaderState::default());
@@ -299,7 +299,7 @@ pub fn to_unknown_packet<R: io::Read>(reader: R)
         = match header.length {
             BodyLength::Full(len) =>
                 Box::new(BufferedReaderLimitor::with_cookie(
-                    reader, len as u64, BufferedReaderState::default())),
+                    Box::new(reader), len as u64, BufferedReaderState::default())),
             BodyLength::Partial(len) =>
                 Box::new(BufferedReaderPartialBodyFilter::with_cookie(
                     reader, len, BufferedReaderState::default())),
-- 
cgit v1.2.3