Reduce cloning to improve performance

This improves benchmark performance by about 20-30% (25,000 ns/iter). We do so by removing unecessary cloning of Code instances which are immutable within the context of tree::generate calls.
Tomboyo · Mar 14, 2020 · c5c572d · c5c572d
1 parent cf1cf4f
commit c5c572d
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 57 deletions.
diff --git a/src/code.rs b/src/code.rs
@@ -7,7 +7,7 @@ use crate::response::Response;
 
 pub type Code = Vec<usize>;
 
-pub fn universe<'a>(length: usize, base: usize) -> HashSet<Code> {
+pub fn universe(length: usize, base: usize) -> HashSet<Code> {
     let mut acc = HashSet::new();
 
     let mut data: Vec<usize> = iter::repeat(0).take(length).collect();

diff --git a/src/main.rs b/src/main.rs
@@ -11,16 +11,14 @@ mod options;
 mod response;
 mod tree;
 
-use crate::tree::Tree;
 use crate::tree::rank;
 
 fn main() {
     let options = options::from_stdin();
-    let rank = |tree: &Tree| rank::by_depth(tree) as f64;
     let best_tree = tree::generate_exhaustively(
         options.code_length,
         options.code_base,
-        &rank
+        &rank::by_depth
     );
 
     println!("{:?}", best_tree);

diff --git a/src/response.rs b/src/response.rs
@@ -1,4 +1,4 @@
-#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 // ( correct pegs, misplaced pegs, unused pegs )
 pub struct Response(pub usize, pub usize, pub usize);
 

diff --git a/src/tree.rs b/src/tree.rs
@@ -13,32 +13,38 @@ pub struct Tree {
     children: BTreeMap<Response, Option<Tree>>,
 }
 
+#[derive(Debug, PartialEq, Eq)]
+pub struct RefTree<'a> {
+    guess: &'a Code,
+    children: BTreeMap<Response, Option<RefTree<'a>>>,
+}
+
 pub fn generate_exhaustively<F>(
     code_length: usize,
     code_base: usize,
     rank: &F,
 ) -> Tree
-where F: Fn(&Tree) -> f64 {
+where F: Fn(&RefTree) -> f64 {
     let universe = code::universe(code_length, code_base);
     generate(
-        universe.iter().cloned().collect(),
-        universe.iter().cloned().collect(),
+        universe.iter().collect(),
+        universe.iter().collect(),
         rank
-    )
+    ).to_tree()
 }
 
-pub fn generate<F>(
-    guesses: Vec<Code>,
-    answers: Vec<Code>,
+fn generate<'a, F>(
+    guesses: Vec<&'a Code>,
+    answers: Vec<&'a Code>,
     rank: &F
-) -> Tree
-where F: Fn(&Tree) -> f64 {
+) -> RefTree<'a>
+where F: Fn(&RefTree<'a>) -> f64 {
     guesses.iter()
-        .map(|guess| Tree {
-            guess: guess.clone(),
+        .map(|guess| RefTree {
+            guess,
             children: generate_children(guess, &guesses, &answers, rank)
         })
-        .fold(None, |best, candidate| {
+        .fold(None, |best: Option<RefTree<'a>>, candidate: RefTree<'a>| {
             match best {
                  None => Some(candidate),
                  Some(x) => Some(select(x, candidate, &rank))
@@ -47,21 +53,21 @@ where F: Fn(&Tree) -> f64 {
         .expect("There should be at least one tree")
 }
 
-fn generate_children<F>(
-    guess: &Code,
-    guesses: &Vec<Code>,
-    answers: &Vec<Code>,
+fn generate_children<'a, F>(
+    guess: &'a Code,
+    guesses: &Vec<&'a Code>,
+    answers: &Vec<&'a Code>,
     rank: &F
-) -> BTreeMap<Response, Option<Tree>>
-where F: Fn(&Tree) ->f64 {
+) -> BTreeMap<Response, Option<RefTree<'a>>>
+where F: Fn(&RefTree<'a>) ->f64 {
     let mut children = BTreeMap::new();
     for (response, remaining_answers) in answers_by_response(&guess, &answers) {
         if response::is_correct(&response) {
             children.insert(response, None);
         } else {
             let remaining_guesses = guesses.iter()
                 .cloned()
-                .filter(|x| x != guess)
+                .filter(|x| *x != guess)
                 .collect();
             children.insert(
                 response,
@@ -74,32 +80,46 @@ where F: Fn(&Tree) ->f64 {
     children
 }
 
-fn answers_by_response(
+fn answers_by_response<'a> (
     guess: &Code,
-    answers: &Vec<Code>,
-) -> BTreeMap<Response, Vec<Code>> {
+    answers: &Vec<&'a Code>,
+) -> BTreeMap<Response, Vec<&'a Code>> {
     answers.iter()
         .fold(BTreeMap::new(), |mut map, answer| {
             map.entry(code::compare(guess, answer))
                 .or_insert_with(Vec::new)
-                .push(answer.clone());
+                .push(answer);
             map
         })
 }
 
-fn select<F>(
-    left: Tree,
-    right: Tree,
+fn select<'a, F>(
+    left: RefTree<'a>,
+    right: RefTree<'a>,
     rank: &F
-) -> Tree
-where F: Fn(&Tree) -> f64 {
+) -> RefTree<'a>
+where F: Fn(&RefTree<'a>) -> f64 {
     if rank(&left) <= rank(&right) {
         left
     } else {
         right
     }
 }
 
+impl<'a> RefTree<'a> {
+    fn to_tree(&self) -> Tree {
+        Tree {
+            guess: self.guess.to_vec(),
+            children: self.children.iter()
+                .map(|(response, opt_ref_tree)| match opt_ref_tree {
+                    None => (response.clone(), None),
+                    Some(ref_tree) => (response.clone(), Some(ref_tree.to_tree())),
+                })
+                .collect()
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use test::Bencher;
@@ -109,25 +129,28 @@ mod tests {
 
     #[test]
     fn test_generate() {
+        let c00 = vec![0, 0];
+        let c01 = vec![0, 1];
+
         // prefer trees based on their guess; 0,0 is "best".
-        let rank = |tree: &Tree| match &tree.guess[..] {
+        let rank = |tree: &RefTree| match &tree.guess[..] {
             &[0, 0] => 0f64,
             &[0, 1] => 1f64,
             x => panic!("Unexpected test code {:?}", x)
         };
 
         let actual = generate(
-            vec![vec![0, 0], vec![0, 1]],
-            vec![vec![0, 0], vec![0, 1]],
+            vec![&c00, &c01],
+            vec![&c00, &c01],
             &rank
         );
 
-        let expected = Tree {
-            guess: vec![0, 0],
+        let expected = RefTree {
+            guess: &c00,
             children: btreemap![
                 Response(2, 0, 0) => None,
-                Response(1, 0, 1) => Some(Tree {
-                    guess: vec![0, 1],
+                Response(1, 0, 1) => Some(RefTree {
+                    guess: &c01,
                     children: btreemap![Response(2, 0, 0) => None]
                 }),
             ],
@@ -138,7 +161,7 @@ mod tests {
 
     #[bench]
     fn test_generate_exhausively(bencher: &mut Bencher) {
-        let rank = |tree: &Tree| rank::by_depth(tree) as f64;
+        let rank = |tree: &RefTree| rank::by_depth(tree) as f64;
         bencher.iter(|| generate_exhaustively(2, 2, &rank))
     }
 }
diff --git a/src/tree/rank.rs b/src/tree/rank.rs
@@ -1,15 +1,20 @@
-#[cfg(test)]
-use crate::response::Response;
-use crate::tree::Tree;
+use num_traits::ToPrimitive;
+
+use crate::tree::RefTree;
+#[cfg(test)] use crate::response::Response;
 
-pub fn by_depth(tree: &Tree) -> u32 {
+pub fn by_depth(tree: &RefTree) -> f64 {
+    by_depth_u32(tree).to_f64().unwrap() // todo
+}
+
+fn by_depth_u32(tree: &RefTree) -> u32 {
     let mut max = 1u32;
 
     for (_, child) in tree.children.iter() {
         match child {
             None       => continue,
             Some(tree) => {
-                max = std::cmp::max(max, 1 + by_depth(tree))
+                max = std::cmp::max(max, 1 + by_depth_u32(tree))
             }
         }
     }
@@ -23,23 +28,25 @@ mod tests {
 
     #[test]
     fn test_by_depth() {
-        assert_eq!(3, by_depth(
-            &Tree {
-                guess: vec![0],
+        let arbitrary = vec![0];
+
+        assert_eq!(3f64, by_depth(
+            &RefTree {
+                guess: &arbitrary,
                 children: btreemap![
                     Response(2, 0, 0) => None,
-                    Response(0, 0, 0) => Some(Tree {
-                        guess: vec![0],
+                    Response(0, 0, 0) => Some(RefTree {
+                        guess: &arbitrary,
                         children: btreemap![
                             Response(2, 0, 0) => None
                         ]
                     }),
-                    Response(1, 0, 0) => Some(Tree {
-                        guess: vec![0],
+                    Response(1, 0, 0) => Some(RefTree {
+                        guess: &arbitrary,
                         children: btreemap![
                             Response(2, 0, 0) => None,
-                            Response(1, 0, 0) => Some(Tree {
-                                guess: vec![0],
+                            Response(1, 0, 0) => Some(RefTree {
+                                guess: &arbitrary,
                                 children: btreemap![
                                     Response(2, 0, 0) => None
                                 ]
@@ -53,8 +60,8 @@ mod tests {
 
     #[test]
     fn test_by_depth_minimum_value() {
-        assert_eq!(1, by_depth(
-            &Tree { guess: vec![0], children: btreemap![] }
+        assert_eq!(1f64, by_depth(
+            &RefTree { guess: &vec![0], children: btreemap![] }
         ));
     }
 }