Require BufRead instead of just Read for inputs.

This commit makes a breaking API change - it changes the `R: Read`
constraint (in `Decoder` and `Reader` structs) to the `R: BufRead`
constraint.  This helps performance by avoiding copying the input data
into an additional, intermediate `BufReader` that used to be stored in
the (internal) `ReadDecoder::reader` field (after these changes that
field is `R` rather than `BufReader`).  In particular, some input types
(e.g. when decoding from a `&[u8]`) already implement `BufRead` and for
such types it is wasteful to introduce additional buffering via
`BufReader`.

The impact of the change is significant, but relatively small - this
means that it mostly shows up in `noncompressed...` benchmarks which
magnify the cost of code paths that are not related to `fdeflate` nor
`unfilter`.  Impact on benchmark runtime looks as follows (run once, and
then rerun after compiling before+after with a fresh nightly `rustc`):

* kodim02.png:
    - No change in performance detected (p = 0.08 > 0.05)
    - [+1.3713% +1.7241% +2.0960%] (p = 0.00 < 0.05)
* kodim07.png:
    - [-1.1466% -0.6693% -0.2705%] (p = 0.00 < 0.05)
    - No change in performance detected. (p = 0.35 > 0.05)
* kodim17.png:
    - [-2.3062% -1.2878% +0.1746%] (p = 0.05 < 0.05)
    - [-2.7355% -1.9939% -0.7986%] (p = 0.00 < 0.05)
* kodim23.png:
    - No change in performance detected. (p = 0.51 > 0.05)
    - [-1.4834% -1.0648% -0.6692%] (p = 0.00 < 0.05)
* Lohengrin...png:
    - [-2.0606% -1.7935% -1.4756%] (p = 0.00 < 0.05)
    - [-4.2412% -3.6723% -3.0327%] (p = 0.00 < 0.05)
* Transparency.png:
    - [+1.4991% +1.8812% +2.3429%] (p = 0.00 < 0.05)
    - [-0.7939% -0.5746% -0.3590%] (p = 0.00 < 0.05)
* noncompressed-8x8.png:
    - [-2.2881% -1.4801% -0.4110%] (p = 0.00 < 0.05)
    - [-7.5687% -7.2013% -6.8838%] (p = 0.00 < 0.05)
* noncompressed-128x128.png:
    - [-12.495% -12.132% -11.760%] (p = 0.00 < 0.05)
    - [-10.597% -10.230% -9.8399%] (p = 0.00 < 0.05)

CHANGES.md

@@ -1,4 +1,9 @@
-## Unreleased
+## Unreleased (TODO: breaking changes require increasing semver to 0.18.x)
+
+* Performance improvements.
+* Breaking API changes (required by the performance-related work):
+  - Changed the generic constraints of `png::Decoder<R>` and `png::Reader<R>`
+    to require `R: BufRead` (instead of just `R: Read`).
 
 ## 0.17.10
 

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "png"
-version = "0.17.10"
+version = "0.17.10" # TODO: Increase semver to account for breaking API changes
 license = "MIT OR Apache-2.0"
 
 description = "PNG decoding and encoding library in pure Rust"

examples/show.rs

@@ -8,12 +8,12 @@ use glium::{
     texture::{ClientFormat, RawImage2d},
     BlitTarget, Rect, Surface,
 };
-use std::{borrow::Cow, env, fs::File, io, path};
+use std::{borrow::Cow, env, fs::File, io, io::BufReader, path};
 
 /// Load the image using `png`
 fn load_image(path: &path::PathBuf) -> io::Result<RawImage2d<'static, u8>> {
     use png::ColorType::*;
-    let mut decoder = png::Decoder::new(File::open(path)?);
+    let mut decoder = png::Decoder::new(BufReader::new(File::open(path)?));
     decoder.set_transformations(png::Transformations::normalize_to_color8());
     let mut reader = decoder.read_info()?;
     let mut img_data = vec![0; reader.output_buffer_size()];

src/decoder/mod.rs

@@ -4,7 +4,7 @@ mod zlib;
 pub use self::stream::{DecodeOptions, Decoded, DecodingError, StreamingDecoder};
 use self::stream::{FormatErrorInner, CHUNCK_BUFFER_SIZE};
 
-use std::io::{BufRead, BufReader, Read};
+use std::io::BufRead;
 use std::mem;
 use std::ops::Range;
 
@@ -68,7 +68,7 @@ impl Default for Limits {
 }
 
 /// PNG Decoder
-pub struct Decoder<R: Read> {
+pub struct Decoder<R: BufRead> {
     read_decoder: ReadDecoder<R>,
     /// Output transformations
     transform: Transformations,
@@ -124,17 +124,17 @@ impl<'data> Row<'data> {
     }
 }
 
-impl<R: Read> Decoder<R> {
+impl<R: BufRead> Decoder<R> {
     /// Create a new decoder configuration with default limits.
     pub fn new(r: R) -> Decoder<R> {
         Decoder::new_with_limits(r, Limits::default())
     }
 
     /// Create a new decoder configuration with custom limits.
-    pub fn new_with_limits(r: R, limits: Limits) -> Decoder<R> {
+    pub fn new_with_limits(reader: R, limits: Limits) -> Decoder<R> {
         Decoder {
             read_decoder: ReadDecoder {
-                reader: BufReader::with_capacity(CHUNCK_BUFFER_SIZE, r),
+                reader,
                 decoder: StreamingDecoder::new(),
                 at_eof: false,
             },
@@ -144,10 +144,10 @@ impl<R: Read> Decoder<R> {
     }
 
     /// Create a new decoder configuration with custom `DecodeOptions`.
-    pub fn new_with_options(r: R, decode_options: DecodeOptions) -> Decoder<R> {
+    pub fn new_with_options(reader: R, decode_options: DecodeOptions) -> Decoder<R> {
         Decoder {
             read_decoder: ReadDecoder {
-                reader: BufReader::with_capacity(CHUNCK_BUFFER_SIZE, r),
+                reader,
                 decoder: StreamingDecoder::new_with_options(decode_options),
                 at_eof: false,
             },
@@ -166,17 +166,20 @@ impl<R: Read> Decoder<R> {
     ///
     /// ```
     /// use std::fs::File;
+    /// use std::io::BufReader;
     /// use png::{Decoder, Limits};
     /// // This image is 32×32, 1bit per pixel. The reader buffers one row which requires 4 bytes.
     /// let mut limits = Limits::default();
     /// limits.bytes = 3;
-    /// let mut decoder = Decoder::new_with_limits(File::open("tests/pngsuite/basi0g01.png").unwrap(), limits);
+    /// let reader = BufReader::new(File::open("tests/pngsuite/basi0g01.png").unwrap());
+    /// let mut decoder = Decoder::new_with_limits(reader, limits);
     /// assert!(decoder.read_info().is_err());
     ///
     /// // This image is 32x32 pixels, so the decoder will allocate less than 10Kib
     /// let mut limits = Limits::default();
     /// limits.bytes = 10*1024;
-    /// let mut decoder = Decoder::new_with_limits(File::open("tests/pngsuite/basi0g01.png").unwrap(), limits);
+    /// let reader = BufReader::new(File::open("tests/pngsuite/basi0g01.png").unwrap());
+    /// let mut decoder = Decoder::new_with_limits(reader, limits);
     /// assert!(decoder.read_info().is_ok());
     /// ```
     pub fn set_limits(&mut self, limits: Limits) {
@@ -253,8 +256,10 @@ impl<R: Read> Decoder<R> {
     /// eg.
     /// ```
     /// use std::fs::File;
+    /// use std::io::BufReader;
     /// use png::Decoder;
-    /// let mut decoder = Decoder::new(File::open("tests/pngsuite/basi0g01.png").unwrap());
+    /// let reader = BufReader::new(File::open("tests/pngsuite/basi0g01.png").unwrap());
+    /// let mut decoder = Decoder::new(reader);
     /// decoder.set_ignore_text_chunk(true);
     /// assert!(decoder.read_info().is_ok());
     /// ```
@@ -274,13 +279,13 @@ impl<R: Read> Decoder<R> {
     }
 }
 
-struct ReadDecoder<R: Read> {
-    reader: BufReader<R>,
+struct ReadDecoder<R: BufRead> {
+    reader: R,
     decoder: StreamingDecoder,
     at_eof: bool,
 }
 
-impl<R: Read> ReadDecoder<R> {
+impl<R: BufRead> ReadDecoder<R> {
     /// Returns the next decoded chunk. If the chunk is an ImageData chunk, its contents are written
     /// into image_data.
     fn decode_next(&mut self, image_data: &mut Vec<u8>) -> Result<Option<Decoded>, DecodingError> {
@@ -338,7 +343,7 @@ impl<R: Read> ReadDecoder<R> {
 /// PNG reader (mostly high-level interface)
 ///
 /// Provides a high level that iterates over lines or whole images.
-pub struct Reader<R: Read> {
+pub struct Reader<R: BufRead> {
     decoder: ReadDecoder<R>,
     bpp: BytesPerPixel,
     subframe: SubframeInfo,
@@ -396,7 +401,7 @@ enum SubframeIdx {
     End,
 }
 
-impl<R: Read> Reader<R> {
+impl<R: BufRead> Reader<R> {
     /// Reads all meta data until the next frame data starts.
     /// Requires IHDR before the IDAT and fcTL before fdAT.
     fn read_until_image_data(&mut self) -> Result<(), DecodingError> {

src/decoder/stream.rs

@@ -1380,12 +1380,13 @@ mod tests {
     use crate::{Decoder, DecodingError};
     use byteorder::WriteBytesExt;
     use std::fs::File;
-    use std::io::Write;
+    use std::io::{BufReader, Write};
 
     #[test]
     fn image_gamma() -> Result<(), ()> {
         fn trial(path: &str, expected: Option<ScaledFloat>) {
-            let decoder = crate::Decoder::new(File::open(path).unwrap());
+            let decoder =
+                crate::Decoder::new(BufReader::new(BufReader::new(File::open(path).unwrap())));
             let reader = decoder.read_info().unwrap();
             let actual: Option<ScaledFloat> = reader.info().source_gamma;
             assert!(actual == expected);
@@ -1426,7 +1427,7 @@ mod tests {
     #[test]
     fn image_source_chromaticities() -> Result<(), ()> {
         fn trial(path: &str, expected: Option<SourceChromaticities>) {
-            let decoder = crate::Decoder::new(File::open(path).unwrap());
+            let decoder = crate::Decoder::new(BufReader::new(File::open(path).unwrap()));
             let reader = decoder.read_info().unwrap();
             let actual: Option<SourceChromaticities> = reader.info().source_chromaticities;
             assert!(actual == expected);
@@ -1618,7 +1619,9 @@ mod tests {
         // https://github.com/image-rs/image/issues/1825#issuecomment-1321798639,
         // but the 2nd iCCP chunk has been altered manually (see the 2nd comment below for more
         // details).
-        let decoder = crate::Decoder::new(File::open("tests/bugfixes/issue#1825.png").unwrap());
+        let decoder = crate::Decoder::new(BufReader::new(
+            File::open("tests/bugfixes/issue#1825.png").unwrap(),
+        ));
         let reader = decoder.read_info().unwrap();
         let icc_profile = reader.info().icc_profile.clone().unwrap().into_owned();
 

src/encoder.rs

@@ -1680,7 +1680,7 @@ mod tests {
 
     use rand::{thread_rng, Rng};
     use std::fs::File;
-    use std::io::{Cursor, Write};
+    use std::io::{BufReader, Cursor, Write};
     use std::{cmp, io};
 
     #[test]
@@ -1697,7 +1697,7 @@ mod tests {
                 }
                 eprintln!("{}", path.display());
                 // Decode image
-                let decoder = Decoder::new(File::open(path).unwrap());
+                let decoder = Decoder::new(BufReader::new(File::open(path).unwrap()));
                 let mut reader = decoder.read_info().unwrap();
                 let mut buf = vec![0; reader.output_buffer_size()];
                 let info = reader.next_frame(&mut buf).unwrap();
@@ -1742,7 +1742,7 @@ mod tests {
                     continue;
                 }
                 // Decode image
-                let decoder = Decoder::new(File::open(path).unwrap());
+                let decoder = Decoder::new(BufReader::new(File::open(path).unwrap()));
                 let mut reader = decoder.read_info().unwrap();
                 let mut buf = vec![0; reader.output_buffer_size()];
                 let info = reader.next_frame(&mut buf).unwrap();
@@ -1786,7 +1786,7 @@ mod tests {
         for &bit_depth in &[1u8, 2, 4, 8] {
             // Do a reference decoding, choose a fitting palette image from pngsuite
             let path = format!("tests/pngsuite/basn3p0{}.png", bit_depth);
-            let decoder = Decoder::new(File::open(&path).unwrap());
+            let decoder = Decoder::new(BufReader::new(File::open(&path).unwrap()));
             let mut reader = decoder.read_info().unwrap();
 
             let mut decoded_pixels = vec![0; reader.output_buffer_size()];

src/lib.rs

@@ -5,16 +5,18 @@
 //! ## The decoder
 //!
 //! The most important types for decoding purposes are [`Decoder`](struct.Decoder.html) and
-//! [`Reader`](struct.Reader.html). They both wrap a `std::io::Read`.
-//! `Decoder` serves as a builder for `Reader`. Calling `Decoder::read_info` reads from the `Read` until the
-//! image data is reached.
+//! [`Reader`](struct.Reader.html). They both wrap a `std::io::BufRead`.
+//! `Decoder` serves as a builder for `Reader`. Calling `Decoder::read_info` reads from the
+//! `BufRead` until the image data is reached.
 //!
 //! ### Using the decoder
 //! ```
 //! use std::fs::File;
+//! use std::io::BufReader;
 //! // The decoder is a build for reader and can be used to set various decoding options
 //! // via `Transformations`. The default output transformation is `Transformations::IDENTITY`.
-//! let decoder = png::Decoder::new(File::open("tests/pngsuite/basi0g01.png").unwrap());
+//! let reader = BufReader::new(File::open("tests/pngsuite/basi0g01.png").unwrap());
+//! let decoder = png::Decoder::new(reader);
 //! let mut reader = decoder.read_info().unwrap();
 //! // Allocate the output buffer.
 //! let mut buf = vec![0; reader.output_buffer_size()];

src/text_metadata.rs

@@ -24,17 +24,18 @@
 //!
 //!  ```
 //!  use std::fs::File;
+//!  use std::io::BufReader;
 //!  use std::iter::FromIterator;
 //!  use std::path::PathBuf;
 //!
 //!  // Opening a png file that has a zTXt chunk
-//!  let decoder = png::Decoder::new(
-//!      File::open(PathBuf::from_iter([
+//!  let decoder = png::Decoder::new(BufReader::new(
+//!     File::open(PathBuf::from_iter([
 //!          "tests",
 //!          "text_chunk_examples",
 //!          "ztxt_example.png",
 //!      ]))
-//!      .unwrap(),
+//!      .unwrap()),
 //!  );
 //!  let mut reader = decoder.read_info().unwrap();
 //!  // If the text chunk is before the image data frames, `reader.info()` already contains the text.

tests/check_testimages.rs

@@ -46,7 +46,7 @@ where
     path.push(results_path);
     let mut ref_results = BTreeMap::new();
     let mut failures = vec![];
-    for line in BufReader::new(File::open(path).unwrap()).lines() {
+    for line in BufReader::new(BufReader::new(File::open(path).unwrap())).lines() {
         let line = line.unwrap();
         let parts: Vec<_> = line.split(": ").collect();
         if parts.is_empty() {
@@ -98,7 +98,7 @@ where
 #[test]
 fn render_images() {
     process_images("results.txt", &TEST_SUITES, |path| {
-        let mut decoder = png::Decoder::new(File::open(path)?);
+        let mut decoder = png::Decoder::new(BufReader::new(File::open(path)?));
         decoder.set_transformations(png::Transformations::normalize_to_color8());
         let mut reader = decoder.read_info()?;
         let mut img_data = vec![0; reader.output_buffer_size()];
@@ -121,7 +121,7 @@ fn render_images() {
 #[test]
 fn render_images_identity() {
     process_images("results_identity.txt", &TEST_SUITES, |path| {
-        let decoder = png::Decoder::new(File::open(&path)?);
+        let decoder = png::Decoder::new(BufReader::new(File::open(&path)?));
         let mut reader = decoder.read_info()?;
         let mut img_data = vec![0; reader.output_buffer_size()];
         let info = reader.next_frame(&mut img_data)?;
@@ -146,7 +146,7 @@ fn render_images_identity() {
 #[test]
 fn render_images_alpha() {
     process_images("results_alpha.txt", &TEST_SUITES, |path| {
-        let mut decoder = png::Decoder::new(File::open(&path)?);
+        let mut decoder = png::Decoder::new(BufReader::new(File::open(&path)?));
         decoder.set_transformations(png::Transformations::ALPHA);
         let mut reader = decoder.read_info()?;
         let mut img_data = vec![0; reader.output_buffer_size()];
@@ -187,7 +187,7 @@ fn apng_images() {
             count
         };
 
-        let mut decoder = png::Decoder::new(File::open(&path)?);
+        let mut decoder = png::Decoder::new(BufReader::new(File::open(&path)?));
         decoder.set_transformations(png::Transformations::normalize_to_color8());
         let mut reader = decoder.read_info()?;
         let mut img_data = vec![0; reader.output_buffer_size()];