From 4ae64c568bbac8bf34ba89bf8772f9b2be70c4e5 Mon Sep 17 00:00:00 2001 From: andrews05 Date: Tue, 11 Jul 2023 19:45:10 +1200 Subject: [PATCH] Process files in parallel (#531) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR makes the oxipng binary process multiple files in parallel, finally fulfilling #275. There seemed to be some debate about whether oxipng _should_ do this or not but there's a couple of reasons I think it makes sense: 1. The concern seemed mostly around the complexity of such a feature. Not to worry, it was trivial* 🙂 2. Since then, oxipng has dropped from a max of something like 180 simultaneous compression trials down to 10, which is very much a good thing but it does mean it's not utilising any more cores than that. Some benchmarks on around 100 files on a machine with 8 cores: Level | Master time | PR time -|-|- 2 | 28.303 | 19.005 3 | 36.507 | 23.089 5 | 1:10.86 | 1:16.01 *Some additional changes were required in order to make sure sensible output is printed to the terminal, since things won't be in order anymore. Here's some example output from before: ``` Processing: tests/files/fully_optimized.png file size = 67 bytes (0 bytes = 0.00% decrease) File already optimized Processing: tests/files/corrupted_header.png Invalid PNG header detected Processing: tests/files/verbose_mode.png file size = 102480 bytes (12228 bytes = 10.66% decrease) Output: tests/files/verbose_mode.png ``` And after: ``` Processing: tests/files/verbose_mode.png Processing: tests/files/fully_optimized.png Processing: tests/files/corrupted_header.png tests/files/corrupted_header.png: Invalid PNG header detected tests/files/fully_optimized.png: Could not optimize further, no change written 102480 bytes (10.66% smaller): tests/files/verbose_mode.png ``` Closes #275, #84, #169, #196 and #419. [edit] This is the last thing I wanted to land before the next release 🥳 --- src/lib.rs | 26 +++++++++++++++++++++----- src/main.rs | 22 ++++++++++++---------- src/rayon.rs | 1 + tests/flags.rs | 1 + 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 23cc7a93..20ec4750 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -453,13 +453,15 @@ pub fn optimize(input: &InFile, output: &OutFile, opts: &Options) -> PngResult<( // Run the optimizer on the decoded PNG. let mut optimized_output = optimize_png(&mut png, &in_data, opts, deadline)?; + let in_length = in_data.len(); + if is_fully_optimized(in_data.len(), optimized_output.len(), opts) { - info!("File already optimized"); match (output, input) { // if p is None, it also means same as the input path (OutFile::Path(ref p), InFile::Path(ref input_path)) if p.as_ref().map_or(true, |p| p == input_path) => { + info!("{}: Could not optimize further, no change written", input); return Ok(()); } _ => { @@ -468,8 +470,22 @@ pub fn optimize(input: &InFile, output: &OutFile, opts: &Options) -> PngResult<( } } + let savings = if in_length >= optimized_output.len() { + format!( + "{} bytes ({:.2}% smaller)", + optimized_output.len(), + (in_length - optimized_output.len()) as f64 / in_length as f64 * 100_f64 + ) + } else { + format!( + "{} bytes ({:.2}% larger)", + optimized_output.len(), + (optimized_output.len() - in_length) as f64 / in_length as f64 * 100_f64 + ) + }; + if opts.pretend { - info!("Running in pretend mode, no output"); + info!("{}: Running in pretend mode, no output", savings); return Ok(()); } @@ -516,7 +532,7 @@ pub fn optimize(input: &InFile, output: &OutFile, opts: &Options) -> PngResult<( if let Some(metadata_input) = &opt_metadata_preserved { copy_times(metadata_input, output_path)?; } - info!("Output: {}", output_path.display()); + info!("{}: {}", savings, output_path.display()); } } Ok(()) @@ -606,14 +622,14 @@ fn optimize_png( ); } if file_original_size >= output.len() { - info!( + debug!( " file size = {} bytes ({} bytes = {:.2}% decrease)", output.len(), file_original_size - output.len(), (file_original_size - output.len()) as f64 / file_original_size as f64 * 100_f64 ); } else { - info!( + debug!( " file size = {} bytes ({} bytes = {:.2}% increase)", output.len(), output.len() - file_original_size, diff --git a/src/main.rs b/src/main.rs index 19768cbd..6b536fc0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,6 +13,9 @@ #![warn(clippy::range_plus_one)] #![allow(clippy::cognitive_complexity)] +#[cfg(not(feature = "parallel"))] +mod rayon; + use clap::{value_parser, Arg, ArgAction, ArgMatches, Command}; use indexmap::IndexSet; use log::{error, warn}; @@ -21,6 +24,7 @@ use oxipng::Options; use oxipng::RowFilter; use oxipng::StripChunks; use oxipng::{InFile, OutFile}; +use rayon::prelude::*; use std::fs::DirBuilder; #[cfg(feature = "zopfli")] use std::num::NonZeroU8; @@ -313,9 +317,8 @@ Heuristic filter selection strategies: true, ); - let mut success = false; - for (input, output) in files { - match oxipng::optimize(&input, &output, &opts) { + let success = files.into_par_iter().filter(|(input, output)| { + match oxipng::optimize(input, output, &opts) { // For optimizing single files, this will return the correct exit code always. // For recursive optimization, the correct choice is a bit subjective. // We're choosing to return a 0 exit code if ANY file in the set @@ -323,16 +326,15 @@ Heuristic filter selection strategies: // The reason for this is that recursion may pick up files that are not // PNG files, and return an error for them. // We don't really want to return an error code for those files. - Ok(_) => { - success = true; - } + Ok(_) => true, Err(e) => { - error!("{}", e); + error!("{}: {}", input, e); + false } - }; - } + } + }); - if !success { + if success.count() == 0 { exit(1); } } diff --git a/src/rayon.rs b/src/rayon.rs index 21010668..d53ae024 100644 --- a/src/rayon.rs +++ b/src/rayon.rs @@ -75,6 +75,7 @@ pub fn join(a: impl FnOnce() -> A, b: impl FnOnce() -> B) -> (A, B) { (a(), b()) } +#[allow(dead_code)] pub fn spawn(a: impl FnOnce() -> A) -> A { a() } diff --git a/tests/flags.rs b/tests/flags.rs index 5d3bdc2f..006c29b3 100644 --- a/tests/flags.rs +++ b/tests/flags.rs @@ -188,6 +188,7 @@ fn verbose_mode() { "Found better combination:", " zc = 11 f = None ", " IDAT size = ", + " file size = ", ]; assert_eq!(logs.len(), expected_prefixes.len()); for (i, log) in logs.into_iter().enumerate() {