Repository: shssoichiro/oxipng (language: rust)

Issue:
Please support running in parallel across multiple files
If I run `oxipng -Z *.png` on a directory full of PNG files, it runs 2 combinations at a time on one file at a time, using 100% of two CPUs and leaving all the others idle.

I'd love to have oxipng run on all the PNG files given to it in parallel, up to the limits of available CPUs. That would make it easier to run oxipng on a large number of images quickly.


Discussion / hints:
Oxipng would then need some hotkeys to pause (and later to resume) such processing — the keys you'd press when you suddenly need some free CPU cores.

[Software flow control](https://en.wikipedia.org/wiki/Software_flow_control) of Ctrl+S and Ctrl+Q does not pause immediately (it just prevents new threads from running, but the threads already in progress may take many minutes to finish in Zopfli mode).

Oxipng that is not running in parallel across multiple files (i. e. is running in a simple `for` loop) can be somewhat “paused” by terminating the current instance of oxipng (by Ctrl+C) and can be later resumed by finally answering “No” to the operating system's question of whether the outer `for` loop should be broken:

![(screenshot)](https://user-images.githubusercontent.com/1088720/89332420-0841e580-d69c-11ea-85e9-d89c64196970.png)

Of course by such “pause” you'd lose the results of your current file's processing and would have to re-run oxipng on that file later or in a parallel window. But at least there's some way to free some CPU cores from oxipng when you need them.

But if there's no outer loop and oxipng loops itself in parallel across multiple files — well, then you won't Ctrl+C to terminate; you'd fear to lose all the progress and won't be able to resume. And thus such improved oxipng would likely need some pause/resume hotkeys of its own.
On Tue, Aug 04, 2020 at 12:03:07PM -0700, Mithgol wrote:
> Oxipng would then need some hotkeys to pause (and later to resume) such processing — the keys you'd press when you suddenly need some free CPU cores.

That seems like an entirely unrelated feature request.

Also, you can hit Ctrl-Z (or send SIGSTOP) to suspend a process.

I'm not sure if this is something that I'd like to add in the immediate future. Maybe down the road, or if someone has extra free time and would like to submit a PR for it? The reason I say this is because you can already accomplish this using the [parallel tool](https://saveriomiroddi.github.io/Running-shell-commands-in-parallel-via-gnu-parallel/) on Mac/Linux or [this similar tool for Windows](https://github.com/shenwei356/rush).
@shssoichiro There are a few reasons to integrate it into oxipng. oxipng itself runs multiple threads (to test multiple ways to shrink a file in parallel), so running multiple instances of oxipng can produce an unexpectedly large number of threads. If oxipng handles it itself, it can maintain a queue to keep all CPUs busy without overload.
> If oxipng handles it itself, it can maintain a queue to keep all CPUs busy without overload.

E.g., #169


It would be ridiculously bloated to reinvent xargs -P/parallel in all tools. If you want some kind of nested threading optimization, do like GNU make -l does: only spawn new threads if the load average isn't too high.
I don't think all tools should reimplement parallelism, only tools that already have their own internal parallelism and naturally operate on multiple files. oxipng doesn't necessarily have to have the parallelism it already does, since that *could* be reimplemented outside of oxipng (generate temporary files and see which one is smallest), but there's value in the integration and simplicity.
Quoting myself from  https://github.com/shssoichiro/oxipng/issues/169#issuecomment-799861460 😊:
> [Leanify _just_ got proper parallel processing support.](https://github.com/JayXon/Leanify/pull/67)  Is there anything _there_ that'd be useful _here_, particularly the use of [TaskFlow](https://github.com/taskflow/taskflow)?

Fix patch:
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, B>(a: impl FnOnce() -> A, b: impl FnOnce() -> B) -> (A, B) {
     (a(), b())
 }
 
+#[allow(dead_code)]
 pub fn spawn<A>(a: impl FnOnce() -> A) -> A {
     a()
 }


Test patch:
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() {