Simplify comparer (#208)

* Skip baselines earlier in the evaluator

We know that we'll throw them away anyway, so there is no point in even sending them to Sender and comparing them with others once we used them for the best candidate size.

* Simplify comparison logic

src/colors.rs

@@ -56,7 +56,7 @@ impl ColorType {
     }
 }
 
-#[derive(Debug, PartialEq, PartialOrd, Clone, Copy)]
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)]
 /// The number of bits to be used per channel per pixel
 pub enum BitDepth {
     /// One bit per channel per pixel

src/evaluate.rs

@@ -24,50 +24,20 @@ use std::thread;
 
 struct Candidate {
     image: PngData,
-    // if false, that's baseline file to throw away
-    is_reduction: bool,
     filter: u8,
     // first wins tie-breaker
     nth: usize,
 }
 
-#[derive(Default)]
-struct Comparator {
-    best_result: Option<Candidate>,
-}
-
-impl Comparator {
-    fn evaluate(&mut self, new: Candidate) {
-        // a tie-breaker is required to make evaluation deterministic
-        let is_best = if let Some(ref old) = self.best_result {
-            // choose smallest compressed, or if compresses the same, smallest uncompressed, or cheaper filter
-            let new = (
-                new.image.idat_data.len(),
-                new.image.raw.data.len(),
-                new.image.raw.ihdr.bit_depth,
-                new.filter,
-                new.nth,
-            );
-            let old = (
-                old.image.idat_data.len(),
-                old.image.raw.data.len(),
-                old.image.raw.ihdr.bit_depth,
-                old.filter,
-                old.nth,
-            );
-            // <= instead of < is important, because best_candidate_size has been set already,
-            // so the current result may be comparing its size with itself
-            new <= old
-        } else {
-            true
-        };
-        if is_best {
-            self.best_result = if new.is_reduction { Some(new) } else { None };
-        }
-    }
-
-    fn get_result(self) -> Option<PngData> {
-        self.best_result.map(|res| res.image)
+impl Candidate {
+    fn cmp_key(&self) -> impl Ord {
+        (
+            self.image.idat_data.len(),
+            self.image.raw.data.len(),
+            self.image.raw.ihdr.bit_depth,
+            self.filter,
+            self.nth,
+        )
     }
 }
 
@@ -81,10 +51,10 @@ pub(crate) struct Evaluator {
     eval_send: SyncSender<Candidate>,
     // the thread helps evaluate images asynchronously
     #[cfg(feature = "parallel")]
-    eval_thread: thread::JoinHandle<Option<PngData>>,
+    eval_thread: thread::JoinHandle<Option<Candidate>>,
     // in non-parallel mode, images are evaluated synchronously
     #[cfg(not(feature = "parallel"))]
-    eval_comparator: std::cell::RefCell<Comparator>,
+    eval_best_candidate: Option<Candidate>,
 }
 
 impl Evaluator {
@@ -98,29 +68,27 @@ impl Evaluator {
             #[cfg(feature = "parallel")]
             eval_send: tx,
             #[cfg(feature = "parallel")]
-            eval_thread: thread::spawn(move || {
-                let mut comparator = Comparator::default();
-                for candidate in rx {
-                    comparator.evaluate(candidate);
-                }
-                comparator.get_result()
-            }),
+            eval_thread: thread::spawn(move || rx.into_iter().min_by_key(Candidate::cmp_key)),
             #[cfg(not(feature = "parallel"))]
-            eval_comparator: Default::default(),
+            eval_best_candidate: None,
         }
     }
 
     /// Wait for all evaluations to finish and return smallest reduction
     /// Or `None` if all reductions were worse than baseline.
     #[cfg(feature = "parallel")]
-    pub fn get_result(self) -> Option<PngData> {
+    fn get_best_candidate(self) -> Option<Candidate> {
         drop(self.eval_send); // disconnect the sender, breaking the loop in the thread
         self.eval_thread.join().expect("eval thread")
     }
 
     #[cfg(not(feature = "parallel"))]
+    fn get_best_candidate(self) -> Option<Candidate> {
+        self.eval_best_candidate
+    }
+
     pub fn get_result(self) -> Option<PngData> {
-        self.eval_comparator.into_inner().get_result()
+        self.get_best_candidate().map(|candidate| candidate.image)
     }
 
     /// Set baseline image. It will be used only to measure minimum compression level required
@@ -162,6 +130,10 @@ impl Evaluator {
                     &deadline,
                 ) {
                     best_candidate_size.set_min(idat_data.len());
+                    // ignore baseline images after this point
+                    if !is_reduction {
+                        return;
+                    }
                     // the rest is shipped to the evavluation/collection thread
                     let new = Candidate {
                         image: PngData {
@@ -169,7 +141,6 @@ impl Evaluator {
                             raw: Arc::clone(&image),
                         },
                         filter,
-                        is_reduction,
                         nth,
                     };
 
@@ -180,7 +151,10 @@ impl Evaluator {
 
                     #[cfg(not(feature = "parallel"))]
                     {
-                        self.eval_comparator.borrow_mut().evaluate(new);
+                        match self.eval_best_candidate {
+                            Some(prev) if prev.cmp_key() < new.cmp_key() => {}
+                            _ => self.eval_best_candidate = Some(new),
+                        }
                     }
                 }
             });