Improve performance consistency (#202)

* Improve performance consistency

Switch from HashMap / HashSet to IndexMap / IndexSet for consistent iteration order of various options and, as a result, more predictable performance.

Cargo.lock

@@ -192,6 +192,15 @@ dependencies = [
  "png 0.16.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "indexmap"
+version = "1.3.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "autocfg 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rayon 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "inflate"
 version = "0.4.5"
@@ -303,12 +312,14 @@ dependencies = [
  "cloudflare-zlib 0.2.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "crc 1.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "image 0.23.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "indexmap 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "itertools 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "libdeflater 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "miniz_oxide 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "num_cpus 1.12.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "rayon 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "rgb 0.8.16 (registry+https://github.com/rust-lang/crates.io-index)",
+ "vec_map 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "wild 2.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "zopfli 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
@@ -469,6 +480,7 @@ dependencies = [
 "checksum glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574"
 "checksum hermit-abi 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "1010591b26bbfe835e9faeabeb11866061cc7dcebffd56ad7d0942d0e61aefd8"
 "checksum image 0.23.2 (registry+https://github.com/rust-lang/crates.io-index)" = "9062b90712d25bc6bb165d110aa59c6b47c849246e341e7b86a98daff9d49f60"
+"checksum indexmap 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "076f042c5b7b98f31d205f1249267e12a6518c1481e9dae9764af19b707d2292"
 "checksum inflate 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)" = "1cdb29978cc5797bd8dcc8e5bf7de604891df2a8dc576973d71a281e916db2ff"
 "checksum itertools 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "284f18f85651fe11e8a991b2adb42cb078325c996ed026d994719efcfca1d54b"
 "checksum lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"

Cargo.toml

@@ -36,6 +36,7 @@ num_cpus = "^1.0.0"
 zopfli = "^0.4.0"
 miniz_oxide = "0.3"
 rgb = "0.8.11"
+indexmap = { version = "1.3.2", features = ["rayon"] }
 libdeflater = "0.2.0"
 
 [dependencies.rayon]

src/headers.rs

@@ -1,9 +1,9 @@
+use indexmap::IndexSet;
 use crate::colors::{BitDepth, ColorType};
 use crate::error::PngError;
 use crate::PngResult;
 use byteorder::{BigEndian, ReadBytesExt};
 use crc::crc32;
-use std::collections::HashSet;
 use std::io::Cursor;
 
 #[derive(Debug, Clone, Copy)]
@@ -35,7 +35,7 @@ pub enum Headers {
     /// Headers that won't affect rendering (all but cHRM, gAMA, iCCP, sBIT, sRGB, bKGD, hIST, pHYs, sPLT)
     Safe,
     /// Remove all non-critical chunks except these
-    Keep(HashSet<String>),
+    Keep(IndexSet<String>),
     /// All non-critical headers
     All,
 }

src/lib.rs

@@ -28,8 +28,8 @@ use crate::png::PngImage;
 use crate::reduction::*;
 use crc::crc32;
 use image::{DynamicImage, GenericImageView, ImageFormat, Pixel};
+use indexmap::{IndexSet, IndexMap};
 use rayon::prelude::*;
-use std::collections::{BTreeMap, HashMap, HashSet};
 use std::fmt;
 use std::fs::{copy, File};
 use std::io::{stdin, stdout, BufWriter, Read, Write};
@@ -154,7 +154,7 @@ pub struct Options {
     /// Which filters to try on the file (0-5)
     ///
     /// Default: `0,5`
-    pub filter: HashSet<u8>,
+    pub filter: IndexSet<u8>,
     /// Whether to change the interlacing type of the file.
     ///
     /// `None` will not change the current interlacing type.
@@ -166,11 +166,11 @@ pub struct Options {
     /// Which zlib compression levels to try on the file (1-9)
     ///
     /// Default: `9`
-    pub compression: HashSet<u8>,
+    pub compression: IndexSet<u8>,
     /// Which zlib compression strategies to try on the file (0-3)
     ///
     /// Default: `0-3`
-    pub strategies: HashSet<u8>,
+    pub strategies: IndexSet<u8>,
     /// Window size to use when compressing the file, as `2^window` bytes.
     ///
     /// Doesn't affect compression but may affect speed and memory usage.
@@ -179,7 +179,7 @@ pub struct Options {
     /// Default: `15`
     pub window: u8,
     /// Alpha filtering strategies to use
-    pub alphas: HashSet<colors::AlphaOptim>,
+    pub alphas: IndexSet<colors::AlphaOptim>,
     /// Whether to attempt bit depth reduction
     ///
     /// Default: `true`
@@ -286,17 +286,17 @@ impl Options {
 impl Default for Options {
     fn default() -> Options {
         // Default settings based on -o 2 from the CLI interface
-        let mut filter = HashSet::new();
+        let mut filter = IndexSet::new();
         filter.insert(0);
         filter.insert(5);
-        let mut compression = HashSet::new();
+        let mut compression = IndexSet::new();
         compression.insert(9);
-        let mut strategies = HashSet::new();
+        let mut strategies = IndexSet::new();
         for i in 0..4 {
             strategies.insert(i);
         }
         // We always need NoOp to be present
-        let mut alphas = HashSet::new();
+        let mut alphas = IndexSet::new();
         alphas.insert(AlphaOptim::NoOp);
 
         Options {
@@ -502,7 +502,7 @@ fn optimize_png(
         eprintln!("    File size = {} bytes", file_original_size);
     }
 
-    let mut filter = opts.filter.iter().cloned().collect::<Vec<u8>>();
+    let mut filter = opts.filter.clone();
     let compression = &opts.compression;
     let mut strategies = opts.strategies.clone();
 
@@ -512,14 +512,14 @@ fn optimize_png(
             && png.raw.ihdr.color_type != colors::ColorType::Indexed
         {
             if filter.is_empty() {
-                filter.push(5);
+                filter.insert(5);
             }
             if strategies.is_empty() {
                 strategies.insert(1);
             }
         } else {
             if filter.is_empty() {
-                filter.push(0);
+                filter.insert(0);
             }
             if strategies.is_empty() {
                 strategies.insert(0);
@@ -583,8 +583,10 @@ fn optimize_png(
             eprintln!("Trying: {} combinations", results.len());
         }
 
-        let filter_iter = filter.par_iter().with_max_len(1);
+        let filters: IndexMap<u8, Vec<u8>> =
-        let filters: HashMap<u8, Vec<u8>> = filter_iter
+            filter
+            .par_iter()
+            .with_max_len(1)
             .map(|f| {
                 let png = png.clone();
                 (*f, png.raw.filter_image(*f))
@@ -865,15 +867,10 @@ fn perform_strip(png: &mut PngData, opts: &Options) {
         // Strip headers
         Headers::None => (),
         Headers::Keep(ref hdrs) => {
-            let keys: Vec<[u8; 4]> = raw.aux_headers.keys().cloned().collect();
+            raw.aux_headers.retain(|hdr, _| {
-            for hdr in &keys {
+                std::str::from_utf8(hdr)
-                let preserve = std::str::from_utf8(hdr)
+                    .map_or(false, |name| hdrs.contains(name))
-                    .ok()
+            })
-                    .map_or(false, |name| hdrs.contains(name));
-                if !preserve {
-                    raw.aux_headers.remove(hdr);
-                }
-            }
         }
         Headers::Strip(ref hdrs) => {
             for hdr in hdrs {
@@ -893,7 +890,7 @@ fn perform_strip(png: &mut PngData, opts: &Options) {
             }
         }
         Headers::All => {
-            raw.aux_headers = BTreeMap::new();
+            raw.aux_headers = IndexMap::new();
         }
     }
 

src/main.rs

@@ -14,13 +14,13 @@
 #![allow(clippy::cognitive_complexity)]
 
 use clap::{App, AppSettings, Arg, ArgMatches};
+use indexmap::IndexSet;
 use oxipng::AlphaOptim;
 use oxipng::Deflaters;
 use oxipng::Headers;
 use oxipng::Options;
 use oxipng::PngResult;
 use oxipng::{InFile, OutFile};
-use std::collections::HashSet;
 use std::fs::DirBuilder;
 use std::path::PathBuf;
 use std::process::exit;
@@ -500,9 +500,9 @@ fn parse_numeric_range_opts(
     input: &str,
     min_value: u8,
     max_value: u8,
-) -> Result<HashSet<u8>, String> {
+) -> Result<IndexSet<u8>, String> {
     const ERROR_MESSAGE: &str = "Not a valid input";
-    let mut items = HashSet::new();
+    let mut items = IndexSet::new();
 
     // one value
     if let Ok(one_value) = input.parse::<u8>() {

src/png/mod.rs

@@ -6,9 +6,9 @@ use crate::headers::*;
 use crate::interlace::{deinterlace_image, interlace_image};
 use byteorder::{BigEndian, WriteBytesExt};
 use crc::crc32;
+use indexmap::IndexMap;
 use rgb::ComponentSlice;
 use rgb::RGBA8;
-use std::collections::BTreeMap;
 use std::fs::File;
 use std::io::{Read, Seek, SeekFrom};
 use std::iter::Iterator;
@@ -38,7 +38,7 @@ pub struct PngImage {
     /// The pixel value that should be rendered as transparent
     pub transparency_pixel: Option<Vec<u8>>,
     /// All non-critical headers from the PNG are stored here
-    pub aux_headers: BTreeMap<[u8; 4], Vec<u8>>,
+    pub aux_headers: IndexMap<[u8; 4], Vec<u8>>,
 }
 
 /// Contains all data relevant to a PNG image
@@ -97,7 +97,7 @@ impl PngData {
         }
         byte_offset += 8;
         // Read the data headers
-        let mut aux_headers: BTreeMap<[u8; 4], Vec<u8>> = BTreeMap::new();
+        let mut aux_headers: IndexMap<[u8; 4], Vec<u8>> = IndexMap::new();
         let mut idat_headers: Vec<u8> = Vec::new();
         while let Some(header) = parse_next_header(byte_data, &mut byte_offset, fix_errors)? {
             match &header.name {

src/reduction/alpha.rs

@@ -1,3 +1,4 @@
+use indexmap::IndexSet;
 use crate::colors::AlphaOptim;
 use crate::colors::ColorType;
 use crate::evaluate::Evaluator;
@@ -8,22 +9,21 @@ use crate::png::PngImage;
 use crate::rayon::prelude::*;
 #[cfg(feature = "parallel")]
 use rayon::prelude::*;
-use std::collections::HashSet;
 use std::sync::Arc;
 
 pub(crate) fn try_alpha_reductions(
     png: Arc<PngImage>,
-    alphas: &HashSet<AlphaOptim>,
+    alphas: &IndexSet<AlphaOptim>,
     eval: &Evaluator,
 ) {
     if alphas.is_empty() {
         return;
     }
 
-    let alphas = alphas.iter().collect::<Vec<_>>();
+    alphas
-    let alphas_iter = alphas.par_iter().with_max_len(1);
+        .par_iter()
-    alphas_iter
+        .with_max_len(1)
-        .filter_map(|&alpha| filtered_alpha_channel(&png, *alpha))
+        .filter_map(|&alpha| filtered_alpha_channel(&png, alpha))
         .for_each(|image| eval.try_image(Arc::new(image)));
 }
 

src/reduction/color.rs

@@ -1,9 +1,9 @@
 use crate::colors::{BitDepth, ColorType};
 use crate::headers::IhdrData;
 use crate::png::PngImage;
+use indexmap::IndexMap;
 use itertools::Itertools;
 use rgb::{FromSlice, RGB8, RGBA8};
-use std::collections::HashMap;
 use std::hash::Hash;
 
 #[must_use]
@@ -76,7 +76,7 @@ pub fn reduce_rgba_to_grayscale_alpha(png: &PngImage) -> Option<PngImage> {
 
 fn reduce_scanline_to_palette<T>(
     iter: impl IntoIterator<Item = T>,
-    palette: &mut HashMap<T, u8>,
+    palette: &mut IndexMap<T, u8>,
     reduced: &mut Vec<u8>,
 ) -> bool
 where
@@ -105,7 +105,7 @@ pub fn reduced_color_to_palette(png: &PngImage) -> Option<PngImage> {
         return None;
     }
     let mut raw_data = Vec::with_capacity(png.data.len());
-    let mut palette = HashMap::with_capacity(257);
+    let mut palette = IndexMap::with_capacity(257);
     let transparency_pixel = png
         .transparency_pixel
         .as_ref()

src/reduction/mod.rs

@@ -1,10 +1,9 @@
 use crate::colors::{BitDepth, ColorType};
 use crate::headers::IhdrData;
 use crate::png::PngImage;
+use indexmap::map::{IndexMap, Entry::*};
 use rgb::RGBA8;
 use std::borrow::Cow;
-use std::collections::hash_map::Entry::*;
-use std::collections::HashMap;
 
 pub mod alpha;
 use crate::alpha::*;
@@ -79,7 +78,7 @@ pub fn reduced_palette(png: &PngImage) -> Option<PngImage> {
         });
 
         let mut next_index = 0u16;
-        let mut seen = HashMap::with_capacity(palette.len());
+        let mut seen = IndexMap::with_capacity(palette.len());
         for (i, used) in used_enumerated.iter().cloned() {
             if !used {
                 continue;

tests/filters.rs

@@ -1,8 +1,6 @@
-use oxipng;
+use indexmap::IndexSet;
-
 use oxipng::internal_tests::*;
 use oxipng::{InFile, OutFile};
-use std::collections::HashSet;
 use std::fs::remove_file;
 use std::path::Path;
 use std::path::PathBuf;
@@ -11,7 +9,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
     let mut options = oxipng::Options::default();
     options.verbosity = None;
     options.force = true;
-    let mut filter = HashSet::new();
+    let mut filter = IndexSet::new();
     filter.insert(0);
     options.filter = filter;
 
@@ -33,7 +31,7 @@ fn test_it_converts(
 
     let (output, mut opts) = get_opts(&input);
     let png = PngData::new(&input, opts.fix_errors).unwrap();
-    opts.filter = HashSet::new();
+    opts.filter = IndexSet::new();
     opts.filter.insert(filter);
     assert_eq!(png.raw.ihdr.color_type, color_type_in);
     assert_eq!(png.raw.ihdr.bit_depth, bit_depth_in);

tests/flags.rs

@@ -1,8 +1,6 @@
-extern crate oxipng;
+use indexmap::IndexSet;
-
 use oxipng::internal_tests::*;
 use oxipng::{InFile, OutFile};
-use std::collections::HashSet;
 use std::fs::remove_file;
 use std::path::Path;
 use std::path::PathBuf;
@@ -11,7 +9,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
     let mut options = oxipng::Options::default();
     options.verbosity = None;
     options.force = true;
-    let mut filter = HashSet::new();
+    let mut filter = IndexSet::new();
     filter.insert(0);
     options.filter = filter;
 
@@ -342,7 +340,7 @@ fn interlaced_0_to_1_other_filter_mode() {
     let input = PathBuf::from("tests/files/interlaced_0_to_1_other_filter_mode.png");
     let (output, mut opts) = get_opts(&input);
     opts.interlace = Some(1);
-    let mut filter = HashSet::new();
+    let mut filter = IndexSet::new();
     filter.insert(4);
     opts.filter = filter;
 

tests/interlaced.rs

@@ -1,8 +1,6 @@
-extern crate oxipng;
+use indexmap::IndexSet;
-
 use oxipng::internal_tests::*;
 use oxipng::{InFile, OutFile};
-use std::collections::HashSet;
 use std::fs::remove_file;
 use std::path::Path;
 use std::path::PathBuf;
@@ -11,7 +9,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
     let mut options = oxipng::Options::default();
     options.verbosity = None;
     options.force = true;
-    let mut filter = HashSet::new();
+    let mut filter = IndexSet::new();
     filter.insert(0);
     options.filter = filter;
 

tests/reduction.rs

@@ -1,8 +1,6 @@
-use oxipng;
+use indexmap::IndexSet;
-
 use oxipng::internal_tests::*;
 use oxipng::{InFile, OutFile};
-use std::collections::HashSet;
 use std::fs::remove_file;
 use std::path::Path;
 use std::path::PathBuf;
@@ -11,7 +9,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
     let mut options = oxipng::Options::default();
     options.verbosity = None;
     options.force = true;
-    let mut filter = HashSet::new();
+    let mut filter = IndexSet::new();
     filter.insert(0);
     options.filter = filter;
 

tests/regression.rs

@@ -1,8 +1,6 @@
-use oxipng;
+use indexmap::IndexSet;
-
 use oxipng::internal_tests::*;
 use oxipng::{InFile, OutFile};
-use std::collections::HashSet;
 use std::fs::remove_file;
 use std::path::Path;
 use std::path::PathBuf;
@@ -11,7 +9,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
     let mut options = oxipng::Options::default();
     options.verbosity = None;
     options.force = true;
-    let mut filter = HashSet::new();
+    let mut filter = IndexSet::new();
     filter.insert(0);
     options.filter = filter;
 
@@ -309,7 +307,7 @@ fn issue_113_white() {
     let input = "tests/files/issue-113.png";
     let (_, mut opts) = get_opts(Path::new(input));
     opts.interlace = Some(1);
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::Black);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-white-out.png").to_owned(),
@@ -329,7 +327,7 @@ fn issue_113_black() {
     let input = "tests/files/issue-113.png";
     let (_, mut opts) = get_opts(Path::new(input));
     opts.interlace = Some(1);
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::Black);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-black-out.png").to_owned(),
@@ -349,7 +347,7 @@ fn issue_113_right() {
     let input = "tests/files/issue-113.png";
     let (_, mut opts) = get_opts(Path::new(input));
     opts.interlace = Some(1);
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::Right);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-right-out.png").to_owned(),
@@ -369,7 +367,7 @@ fn issue_113_left() {
     let input = "tests/files/issue-113.png";
     let (_, mut opts) = get_opts(Path::new(input));
     opts.interlace = Some(1);
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::Left);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-left-out.png").to_owned(),
@@ -389,7 +387,7 @@ fn issue_113_up() {
     let input = "tests/files/issue-113.png";
     let (_, mut opts) = get_opts(Path::new(input));
     opts.interlace = Some(1);
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::Up);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-up-out.png").to_owned(),
@@ -409,7 +407,7 @@ fn issue_113_down() {
     let input = "tests/files/issue-113.png";
     let (_, mut opts) = get_opts(Path::new(input));
     opts.interlace = Some(1);
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::Down);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-down-out.png").to_owned(),
@@ -441,7 +439,7 @@ fn issue_129() {
 fn issue_133_black() {
     let input = "tests/files/issue-133.png";
     let (_, mut opts) = get_opts(Path::new(input));
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::Black);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-black-out.png").to_owned(),
@@ -460,7 +458,7 @@ fn issue_133_black() {
 fn issue_133_white() {
     let input = "tests/files/issue-133.png";
     let (_, mut opts) = get_opts(Path::new(input));
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::White);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-white-out.png").to_owned(),
@@ -479,7 +477,7 @@ fn issue_133_white() {
 fn issue_133_up() {
     let input = "tests/files/issue-133.png";
     let (_, mut opts) = get_opts(Path::new(input));
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::Up);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-up-out.png").to_owned(),
@@ -498,7 +496,7 @@ fn issue_133_up() {
 fn issue_133_down() {
     let input = "tests/files/issue-133.png";
     let (_, mut opts) = get_opts(Path::new(input));
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::Down);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-down-out.png").to_owned(),
@@ -517,7 +515,7 @@ fn issue_133_down() {
 fn issue_133_right() {
     let input = "tests/files/issue-133.png";
     let (_, mut opts) = get_opts(Path::new(input));
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::Right);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-right-out.png").to_owned(),
@@ -536,7 +534,7 @@ fn issue_133_right() {
 fn issue_133_left() {
     let input = "tests/files/issue-133.png";
     let (_, mut opts) = get_opts(Path::new(input));
-    opts.alphas = HashSet::new();
+    opts.alphas = IndexSet::new();
     opts.alphas.insert(AlphaOptim::Left);
     let output = OutFile::Path(Some(
         Path::new(input).with_extension("-left-out.png").to_owned(),