Repository: xd009642/tarpaulin
Pull request #931 (2022-01-05 08:44:07)

remove a couple of allocations
this PR removes quite a few `collect` calls at the expense (probably) of readability. In particular it complicates a few function signatures my introducing opaque types.

I'm not sure how much of a performance bottleneck these are anyway, but i leave it up to you what you want to do with this.

xd009642: All of the changes so far look pretty sensible so I'm happy merging once there's something that works :+1: 
xd009642: Problem is given the most of the time will be spent in compilation or waiting on ptrace signal/stop/delivery loop it's very hard to definitively measure performance :sweat_smile: 
xd009642: I thought that might be the case, but just in case it wasn't :upside_down_face: 

Patch:
diff --git a/src/cargo.rs b/src/cargo.rs
index 9e6738a..e5c30d6 100644
--- a/src/cargo.rs
+++ b/src/cargo.rs
@@ -279,21 +279,17 @@ fn run_cargo(
                 Ok(Message::BuildScriptExecuted(bs))
                     if !(bs.linked_libs.is_empty() || bs.linked_paths.is_empty()) =>
                 {
-                    let mut temp_paths = bs
-                        .linked_paths
-                        .iter()
-                        .filter_map(|x| {
-                            if x.as_std_path().exists() {
-                                Some(x.as_std_path().to_path_buf())
-                            } else if let Some(index) = x.as_str().find('=') {
-                                Some(PathBuf::from(&x.as_str()[(index + 1)..]))
-                            } else {
-                                warn!("Couldn't resolve linker path: {}", x.as_str());
-                                None
-                            }
-                        })
-                        .collect::<Vec<PathBuf>>();
-                    for p in temp_paths.drain(..) {
+                    let temp_paths = bs.linked_paths.iter().filter_map(|x| {
+                        if x.as_std_path().exists() {
+                            Some(x.as_std_path().to_path_buf())
+                        } else if let Some(index) = x.as_str().find('=') {
+                            Some(PathBuf::from(&x.as_str()[(index + 1)..]))
+                        } else {
+                            warn!("Couldn't resolve linker path: {}", x.as_str());
+                            None
+                        }
+                    });
+                    for p in temp_paths {
                         if !paths.contains(&p) {
                             paths.push(p);
                         }
diff --git a/src/report/cobertura.rs b/src/report/cobertura.rs
index 6cd744b..fa2b9f0 100644
--- a/src/report/cobertura.rs
+++ b/src/report/cobertura.rs
@@ -370,7 +370,6 @@ fn render_class(config: &Config, traces: &TraceMap, file: &Path) -> Option<Class
         let line_rate = covered / coverable as f64;
         let lines = traces
             .get_child_traces(file)
-            .iter()
             .map(|x| render_line(x))
             .collect();
 
diff --git a/src/report/coveralls.rs b/src/report/coveralls.rs
index ac3a810..c80a12e 100644
--- a/src/report/coveralls.rs
+++ b/src/report/coveralls.rs
@@ -88,7 +88,7 @@ pub fn export(coverage_data: &TraceMap, config: &Config) -> Result<(), RunError>
             let mut lines: HashMap<usize, usize> = HashMap::new();
             let fcov = coverage_data.get_child_traces(file);
 
-            for c in &fcov {
+            for c in fcov {
                 match c.stats {
                     CoverageStat::Line(hits) => {
                         lines.insert(c.line as usize, hits as usize);
diff --git a/src/report/mod.rs b/src/report/mod.rs
index 9b44f8f..eb112b3 100644
--- a/src/report/mod.rs
+++ b/src/report/mod.rs
@@ -151,8 +151,8 @@ fn print_summary(config: &Config, result: &TraceMap) {
         }
         let path = config.strip_base_dir(file);
         if last.contains_file(file) && last.coverable_in_path(file) > 0 {
-            let last_percent = coverage_percentage(&last.get_child_traces(file));
-            let current_percent = coverage_percentage(&result.get_child_traces(file));
+            let last_percent = coverage_percentage(last.get_child_traces(file));
+            let current_percent = coverage_percentage(result.get_child_traces(file));
             let delta = 100.0f64 * (current_percent - last_percent);
             println!(
                 "|| {}: {}/{} {:+.2}%",
diff --git a/src/source_analysis/expressions.rs b/src/source_analysis/expressions.rs
index e42e12a..449929b 100644
--- a/src/source_analysis/expressions.rs
+++ b/src/source_analysis/expressions.rs
@@ -241,11 +241,9 @@ impl SourceAnalysis {
         if self.check_attr_list(&call.attrs, ctx) {
             if !call.args.is_empty() && call.span().start().line != call.span().end().line {
                 let lines = get_coverable_args(&call.args);
-                let lines = get_line_range(call)
-                    .filter(|x| !lines.contains(x))
-                    .collect::<Vec<_>>();
+                let lines = get_line_range(call).filter(|x| !lines.contains(x));
                 let analysis = self.get_line_analysis(ctx.file.to_path_buf());
-                analysis.add_to_ignore(&lines);
+                analysis.add_to_ignore(lines);
             }
             self.process_expr(&call.func, ctx);
         } else {
@@ -262,11 +260,9 @@ impl SourceAnalysis {
             let start = meth.receiver.span().end().line + 1;
             let range = get_line_range(meth);
             let lines = get_coverable_args(&meth.args);
-            let lines = (start..range.end)
-                .filter(|x| !lines.contains(x))
-                .collect::<Vec<_>>();
+            let lines = (start..range.end).filter(|x| !lines.contains(x));
             let analysis = self.get_line_analysis(ctx.file.to_path_buf());
-            analysis.add_to_ignore(&lines);
+            analysis.add_to_ignore(lines);
         } else {
             let analysis = self.get_line_analysis(ctx.file.to_path_buf());
             analysis.ignore_tokens(meth);
@@ -326,11 +322,9 @@ impl SourceAnalysis {
                 }
             }
         }
-        let x = get_line_range(structure)
-            .filter(|x| !cover.contains(x))
-            .collect::<Vec<usize>>();
+        let x = get_line_range(structure).filter(|x| !cover.contains(x));
         let analysis = self.get_line_analysis(ctx.file.to_path_buf());
-        analysis.add_to_ignore(&x);
+        analysis.add_to_ignore(x);
         // struct expressions are never unreachable by themselves
         SubResult::Ok
     }
diff --git a/src/source_analysis/items.rs b/src/source_analysis/items.rs
index 4c7b433..ad41f65 100644
--- a/src/source_analysis/items.rs
+++ b/src/source_analysis/items.rs
@@ -147,8 +147,8 @@ impl SourceAnalysis {
             // Ignore multiple lines of fn decl
             let decl_start = func.sig.fn_token.span().start().line + 1;
             let stmts_start = func.block.span().start().line;
-            let lines = (decl_start..=stmts_start).collect::<Vec<_>>();
-            analysis.add_to_ignore(&lines);
+            let lines = decl_start..=stmts_start;
+            analysis.add_to_ignore(lines);
         }
     }
 
diff --git a/src/source_analysis/macros.rs b/src/source_analysis/macros.rs
index 138408a..b029729 100644
--- a/src/source_analysis/macros.rs
+++ b/src/source_analysis/macros.rs
@@ -35,10 +35,8 @@ impl SourceAnalysis {
             let start = mac.span().start().line + 1;
             let range = get_line_range(mac);
             let lines = process_mac_args(&mac.tokens);
-            let lines = (start..range.end)
-                .filter(|x| !lines.contains(x))
-                .collect::<Vec<_>>();
-            analysis.add_to_ignore(&lines);
+            let lines = (start..range.end).filter(|x| !lines.contains(x));
+            analysis.add_to_ignore(lines);
         }
         SubResult::Ok
     }
diff --git a/src/source_analysis/mod.rs b/src/source_analysis/mod.rs
index a5b2746..3358d98 100644
--- a/src/source_analysis/mod.rs
+++ b/src/source_analysis/mod.rs
@@ -220,12 +220,12 @@ impl LineAnalysis {
     }
 
     /// Adds a line to the list of lines to ignore
-    fn add_to_ignore(&mut self, lines: &[usize]) {
+    fn add_to_ignore(&mut self, lines: impl IntoIterator<Item = usize>) {
         if !self.ignore.contains(&Lines::All) {
             for l in lines {
-                self.ignore.insert(Lines::Line(*l));
-                if self.cover.contains(l) {
-                    self.cover.remove(l);
+                self.ignore.insert(Lines::Line(l));
+                if self.cover.contains(&l) {
+                    self.cover.remove(&l);
                 }
             }
         }
@@ -390,9 +390,8 @@ impl SourceAnalysis {
             .lines()
             .enumerate()
             .filter(|&(_, x)| IGNORABLE.is_match(x))
-            .map(|(i, _)| i + 1)
-            .collect::<Vec<usize>>();
-        analysis.add_to_ignore(&lines);
+            .map(|(i, _)| i + 1);
+        analysis.add_to_ignore(lines);
 
         let lines = ctx
             .file_contents
@@ -403,9 +402,8 @@ impl SourceAnalysis {
                 x.retain(|c| !c.is_whitespace());
                 x == "}else{"
             })
-            .map(|(i, _)| i + 1)
-            .collect::<Vec<usize>>();
-        analysis.add_to_ignore(&lines);
+            .map(|(i, _)| i + 1);
+        analysis.add_to_ignore(lines);
     }
 
     pub(crate) fn visit_generics(&mut self, generics: &Generics, ctx: &Context) {
@@ -469,7 +467,7 @@ fn maybe_ignore_first_line(file: &Path, result: &mut HashMap<PathBuf, LineAnalys
             if !(first.starts_with("pub") || first.starts_with("fn")) {
                 let file = file.to_path_buf();
                 let line_analysis = result.entry(file).or_default();
-                line_analysis.add_to_ignore(&[1]);
+                line_analysis.add_to_ignore([1]);
             }
         }
     }
diff --git a/src/traces.rs b/src/traces.rs
index a8078d2..aa94461 100644
--- a/src/traces.rs
+++ b/src/traces.rs
@@ -134,7 +134,7 @@ impl Ord for Trace {
 }
 
 /// Amount of data coverable in the provided slice traces
-pub fn amount_coverable(traces: &[&Trace]) -> usize {
+pub fn amount_coverable<'a>(traces: impl Iterator<Item = &'a Trace>) -> usize {
     let mut result = 0usize;
     for t in traces {
         result += match t.stats {
@@ -147,7 +147,7 @@ pub fn amount_coverable(traces: &[&Trace]) -> usize {
 }
 
 /// Amount of data covered in the provided trace slice
-pub fn amount_covered(traces: &[&Trace]) -> usize {
+pub fn amount_covered<'a>(traces: impl Iterator<Item = &'a Trace>) -> usize {
     let mut result = 0usize;
     for t in traces {
         result += match t.stats {
@@ -161,8 +161,9 @@ pub fn amount_covered(traces: &[&Trace]) -> usize {
     result
 }
 
-pub fn coverage_percentage(traces: &[&Trace]) -> f64 {
-    (amount_covered(traces) as f64) / (amount_coverable(traces) as f64)
+pub fn coverage_percentage<'a>(traces: impl Iterator<Item = &'a Trace>) -> f64 {
+    let t: Vec<_> = traces.collect();
+    (amount_covered(t.iter().copied()) as f64) / (amount_coverable(t.iter().copied()) as f64)
 }
 
 /// Stores all the program traces mapped to files and provides an interface to
@@ -282,16 +283,12 @@ impl TraceMap {
     /// Gets an immutable reference to a trace from an address. Returns None if
     /// there is no trace at that address
     pub fn get_trace(&self, address: u64) -> Option<&Trace> {
-        self.all_traces()
-            .iter()
-            .find(|x| x.address.contains(&address))
-            .copied()
+        self.all_traces().find(|x| x.address.contains(&address))
     }
 
     pub fn increment_hit(&mut self, address: u64) {
         for trace in self
             .all_traces_mut()
-            .iter_mut()
             .filter(|x| x.address.contains(&address))
         {
             if let CoverageStat::Line(ref mut x) = trace.stats {
@@ -342,39 +339,38 @@ impl TraceMap {
     }
 
     /// Gets all traces below a certain path
-    pub fn get_child_traces(&self, root: &Path) -> Vec<&Trace> {
+    pub fn get_child_traces<'a>(&'a self, root: &'a Path) -> impl Iterator<Item = &'a Trace> + 'a {
         self.traces
             .iter()
-            .filter(|&(k, _)| k.starts_with(root))
+            .filter(move |&(k, _)| k.starts_with(root))
             .flat_map(|(_, v)| v.iter())
-            .collect()
     }
 
     /// Gets all traces in folder, doesn't go into other folders for that you
     /// want get_child_traces
-    pub fn get_traces(&self, root: &Path) -> Vec<&Trace> {
-        if root.is_file() {
-            self.get_child_traces(root)
+    pub fn get_traces<'a>(&'a self, root: &'a Path) -> impl Iterator<Item = &'a Trace> + 'a {
+        let i: Box<dyn Iterator<Item = &'a Trace> + 'a> = if root.is_file() {
+            Box::new(self.get_child_traces(root))
         } else {
-            self.traces
-                .iter()
-                .filter(|&(k, _)| k.parent() == Some(root))
-                .flat_map(|(_, v)| v.iter())
-                .collect()
-        }
+            Box::new(
+                self.traces
+                    .iter()
+                    .filter(move |&(k, _)| k.parent() == Some(root))
+                    .flat_map(|(_, v)| v.iter()),
+            )
+        };
+
+        i
     }
 
     /// Gets all traces
-    pub fn all_traces(&self) -> Vec<&Trace> {
-        self.traces.values().flat_map(|x| x.iter()).collect()
+    pub fn all_traces(&self) -> impl Iterator<Item = &Trace> {
+        self.traces.values().flat_map(|x| x.iter())
     }
 
     /// Gets a vector of all the traces to mutate
-    fn all_traces_mut(&mut self) -> Vec<&mut Trace> {
-        self.traces
-            .values_mut()
-            .flat_map(|x| x.iter_mut())
-            .collect()
+    fn all_traces_mut(&mut self) -> impl Iterator<Item = &mut Trace> {
+        self.traces.values_mut().flat_map(|x| x.iter_mut())
     }
 
     pub fn files(&self) -> Vec<&PathBuf> {
@@ -382,11 +378,11 @@ impl TraceMap {
     }
 
     pub fn coverable_in_path(&self, path: &Path) -> usize {
-        amount_coverable(self.get_child_traces(path).as_slice())
+        amount_coverable(self.get_child_traces(path))
     }
 
     pub fn covered_in_path(&self, path: &Path) -> usize {
-        amount_covered(self.get_child_traces(path).as_slice())
+        amount_covered(self.get_child_traces(path))
     }
 
     /// Give the total amount of coverable points in the code. This will vary
@@ -394,17 +390,17 @@ impl TraceMap {
     /// lines whereas for condition or decision it will count the number of
     /// conditions available
     pub fn total_coverable(&self) -> usize {
-        amount_coverable(self.all_traces().as_slice())
+        amount_coverable(self.all_traces())
     }
 
     /// From all the coverable data return the amount covered
     pub fn total_covered(&self) -> usize {
-        amount_covered(self.all_traces().as_slice())
+        amount_covered(self.all_traces())
     }
 
     /// Returns coverage percentage ranging from 0.0-1.0
     pub fn coverage_percentage(&self) -> f64 {
-        coverage_percentage(self.all_traces().as_slice())
+        coverage_percentage(self.all_traces())
     }
 }
 
@@ -502,10 +498,10 @@ mod tests {
         );
 
         t1.merge(&t2);
-        assert_eq!(t1.all_traces().len(), 2);
+        assert_eq!(t1.all_traces().count(), 2);
         assert_eq!(t1.get_trace(5), Some(&a_trace));
         t1.dedup();
-        let all = t1.all_traces();
+        let all = t1.all_traces().collect::<Vec<_>>();
         assert_eq!(all.len(), 1);
         assert_eq!(all[0].stats, CoverageStat::Line(3));
     }
@@ -537,11 +533,11 @@ mod tests {
         );
 
         t1.merge(&t2);
-        assert_eq!(t1.all_traces().len(), 2);
+        assert_eq!(t1.all_traces().count(), 2);
         assert_eq!(t1.get_trace(5), Some(&a_trace));
         t1.dedup();
         let all = t1.all_traces();
-        assert_eq!(all.len(), 2);
+        assert_eq!(all.count(), 2);
     }
 
     #[test]
@@ -572,7 +568,7 @@ mod tests {
             },
         );
         t1.merge(&t2);
-        assert_eq!(t1.all_traces().len(), 1);
+        assert_eq!(t1.all_traces().count(), 1);
         assert_eq!(
             t1.get_trace(1),
             Some(&Trace {
@@ -585,7 +581,7 @@ mod tests {
         );
         // Deduplicating should have no effect.
         t1.dedup();
-        assert_eq!(t1.all_traces().len(), 1);
+        assert_eq!(t1.all_traces().count(), 1);
         assert_eq!(
             t1.get_trace(1),
             Some(&Trace {


Test patch:
diff --git a/src/source_analysis/tests.rs b/src/source_analysis/tests.rs
index 079b074..8288203 100644
--- a/src/source_analysis/tests.rs
+++ b/src/source_analysis/tests.rs
@@ -91,7 +91,7 @@ fn line_analysis_works() {
     assert!(!la.should_ignore(0));
     assert!(!la.should_ignore(10));
 
-    la.add_to_ignore(&[3, 4, 10]);
+    la.add_to_ignore([3, 4, 10]);
     assert!(la.should_ignore(3));
     assert!(la.should_ignore(4));
     assert!(la.should_ignore(10));
diff --git a/tests/line_coverage.rs b/tests/line_coverage.rs
index 104e2c7..6e129a4 100644
--- a/tests/line_coverage.rs
+++ b/tests/line_coverage.rs
@@ -26,7 +26,6 @@ fn simple_project_coverage() {
     assert_eq!(unused_lines, 3);
     let unused_hits = res
         .get_child_traces(&unused_file)
-        .iter()
         .map(|x| x.line)
         .collect::<Vec<_>>();
 
@@ -37,7 +36,7 @@ fn simple_project_coverage() {
 
     let lib_file = test_dir.join("src/lib.rs");
     let lib_traces = res.get_child_traces(&lib_file);
-    for l in &lib_traces {
+    for l in lib_traces {
         if l.line == 6 {
             assert_eq!(CoverageStat::Line(0), l.stats);
         } else if l.line == 8 {
@@ -87,7 +86,7 @@ fn test_threads_1() {
 
     let lib_file = test_dir.join("src/lib.rs");
     let lib_traces = res.get_child_traces(&lib_file);
-    for l in &lib_traces {
+    for l in lib_traces {
         if l.line == 6 {
             assert_eq!(CoverageStat::Line(0), l.stats);
         } else if l.line == 8 {