Deserialize the coverage files directly into the output files (#3410)

* deserialize the coverage files directly into the output files

* reduce the amount of data structure cloning
individual save function for each formats

* cleanup

* unformatted json

* bring back spawn

* remove stray comment

* build fix

* fix build

src/agent/cobertura/src/lib.rs

@@ -12,25 +12,30 @@ impl CoberturaCoverage {
 
         let mut writer = Writer::new_with_indent(cursor, b' ', 2);
 
-        self.write_xml(&mut writer)?;
+        self._write_xml(&mut writer)?;
 
         let text = String::from_utf8(data)?;
         Ok(text)
     }
 }
 
-trait WriteXml {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()>;
+pub trait WriteXml {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()>;
+
+    fn write_xml<W: Write>(&self, writer: W) -> Result<()> {
+        let mut writer = Writer::new(writer);
+        self._write_xml(&mut writer)
+    }
 }
 
 // Only write optional fields if present.
 impl<T> WriteXml for Option<T>
 where
     T: WriteXml,
 {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         if let Some(value) = self {
-            value.write_xml(writer)?;
+            value._write_xml(writer)?;
         }
 
         Ok(())
@@ -41,9 +46,9 @@ impl<T> WriteXml for Vec<T>
 where
     T: WriteXml,
 {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         for value in self {
-            value.write_xml(writer)?;
+            value._write_xml(writer)?;
         }
 
         Ok(())
@@ -101,7 +106,7 @@ pub struct CoberturaCoverage {
 }
 
 impl WriteXml for CoberturaCoverage {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("coverage")
             .with_attributes([
@@ -116,8 +121,8 @@ impl WriteXml for CoberturaCoverage {
                 ("timestamp", uint!(self.timestamp)),
             ])
             .write_inner_content(|w| {
-                self.sources.write_xml(w)?;
-                self.packages.write_xml(w)?;
+                self.sources._write_xml(w)?;
+                self.packages._write_xml(w)?;
 
                 Ok(())
             })?;
@@ -133,10 +138,10 @@ pub struct Sources {
 }
 
 impl WriteXml for Sources {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("sources")
-            .write_inner_content(|w| self.sources.write_xml(w))?;
+            .write_inner_content(|w| self.sources._write_xml(w))?;
 
         Ok(())
     }
@@ -149,7 +154,7 @@ pub struct Source {
 }
 
 impl WriteXml for Source {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("source")
             .with_attributes([("path", string!(self.path))])
@@ -166,10 +171,10 @@ pub struct Packages {
 }
 
 impl WriteXml for Packages {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("packages")
-            .write_inner_content(|w| self.packages.write_xml(w))?;
+            .write_inner_content(|w| self.packages._write_xml(w))?;
 
         Ok(())
     }
@@ -191,7 +196,7 @@ pub struct Package {
 }
 
 impl WriteXml for Package {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("package")
             .with_attributes([
@@ -200,7 +205,7 @@ impl WriteXml for Package {
                 ("branch-rate", float!(self.branch_rate)),
                 ("complexity", uint!(self.complexity)),
             ])
-            .write_inner_content(|w| self.classes.write_xml(w))?;
+            .write_inner_content(|w| self.classes._write_xml(w))?;
 
         Ok(())
     }
@@ -213,10 +218,10 @@ pub struct Classes {
 }
 
 impl WriteXml for Classes {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("classes")
-            .write_inner_content(|w| self.classes.write_xml(w))?;
+            .write_inner_content(|w| self.classes._write_xml(w))?;
 
         Ok(())
     }
@@ -241,7 +246,7 @@ pub struct Class {
 }
 
 impl WriteXml for Class {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("class")
             .with_attributes([
@@ -252,8 +257,8 @@ impl WriteXml for Class {
                 ("complexity", uint!(self.complexity)),
             ])
             .write_inner_content(|w| {
-                self.methods.write_xml(w)?;
-                self.lines.write_xml(w)?;
+                self.methods._write_xml(w)?;
+                self.lines._write_xml(w)?;
                 Ok(())
             })?;
 
@@ -268,10 +273,10 @@ pub struct Methods {
 }
 
 impl WriteXml for Methods {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("methods")
-            .write_inner_content(|w| self.methods.write_xml(w))?;
+            .write_inner_content(|w| self.methods._write_xml(w))?;
 
         Ok(())
     }
@@ -293,7 +298,7 @@ pub struct Method {
 }
 
 impl WriteXml for Method {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("method")
             .with_attributes([
@@ -302,7 +307,7 @@ impl WriteXml for Method {
                 ("line-rate", float!(self.line_rate)),
                 ("branch-rate", float!(self.branch_rate)),
             ])
-            .write_inner_content(|w| self.lines.write_xml(w))?;
+            .write_inner_content(|w| self.lines._write_xml(w))?;
 
         Ok(())
     }
@@ -315,10 +320,10 @@ pub struct Lines {
 }
 
 impl WriteXml for Lines {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("lines")
-            .write_inner_content(|w| self.lines.write_xml(w))?;
+            .write_inner_content(|w| self.lines._write_xml(w))?;
 
         Ok(())
     }
@@ -340,7 +345,7 @@ pub struct Line {
 }
 
 impl WriteXml for Line {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         let condition_coverage = if let Some(s) = &self.condition_coverage {
             s.as_str()
         } else {
@@ -355,7 +360,7 @@ impl WriteXml for Line {
                 ("branch", boolean!(self.branch.unwrap_or_default())),
                 ("condition-coverage", condition_coverage),
             ])
-            .write_inner_content(|w| self.conditions.write_xml(w))?;
+            .write_inner_content(|w| self.conditions._write_xml(w))?;
 
         Ok(())
     }
@@ -368,10 +373,10 @@ pub struct Conditions {
 }
 
 impl WriteXml for Conditions {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("conditions")
-            .write_inner_content(|w| self.conditions.write_xml(w))?;
+            .write_inner_content(|w| self.conditions._write_xml(w))?;
 
         Ok(())
     }
@@ -389,7 +394,7 @@ pub struct Condition {
 }
 
 impl WriteXml for Condition {
-    fn write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
+    fn _write_xml<W: Write>(&self, writer: &mut Writer<W>) -> Result<()> {
         writer
             .create_element("condition")
             .with_attributes([

src/agent/coverage/examples/cobertura.rs

@@ -40,7 +40,7 @@ fn generate_output() -> Result<String> {
         coverage.files.insert(file_path, file);
     }
 
-    CoberturaCoverage::from(coverage).to_string()
+    CoberturaCoverage::from(&coverage).to_string()
 }
 
 #[cfg(test)]

src/agent/coverage/examples/record.rs

@@ -190,7 +190,7 @@ fn dump_modoff(coverage: &BinaryCoverage) -> Result<()> {
 }
 
 fn dump_source_line(binary: &BinaryCoverage, allowlist: AllowList) -> Result<()> {
-    let source = coverage::source::binary_to_source_coverage(binary, allowlist)?;
+    let source = coverage::source::binary_to_source_coverage(binary, &allowlist)?;
 
     for (path, file) in &source.files {
         for (line, count) in &file.lines {
@@ -202,8 +202,8 @@ fn dump_source_line(binary: &BinaryCoverage, allowlist: AllowList) -> Result<()>
 }
 
 fn dump_cobertura(binary: &BinaryCoverage, allowlist: AllowList) -> Result<()> {
-    let source = coverage::source::binary_to_source_coverage(binary, allowlist)?;
-    let cobertura: CoberturaCoverage = source.into();
+    let source = coverage::source::binary_to_source_coverage(binary, &allowlist)?;
+    let cobertura: CoberturaCoverage = (&source).into();
 
     println!("{}", cobertura.to_string()?);
 

src/agent/coverage/src/cobertura.rs

@@ -16,8 +16,8 @@ use crate::source::SourceCoverage;
 // Dir -> Set<FilePath>
 type FileMap<'a> = BTreeMap<&'a str, BTreeSet<&'a FilePath>>;
 
-impl From<SourceCoverage> for CoberturaCoverage {
-    fn from(source: SourceCoverage) -> Self {
+impl From<&SourceCoverage> for CoberturaCoverage {
+    fn from(source: &SourceCoverage) -> Self {
         // The Cobertura data model is organized around `classes` and `methods` contained
         // in `packages`. Our source coverage has no language-level assumptions.
         //
@@ -51,7 +51,7 @@ impl From<SourceCoverage> for CoberturaCoverage {
         // Iterate through the grouped files, accumulating `<package>` elements.
         let (packages, hit_counts): (Vec<Package>, Vec<HitCounts>) = file_map
             .into_iter()
-            .map(|(directory, files)| directory_to_package(&source, directory, files))
+            .map(|(directory, files)| directory_to_package(source, directory, files))
             .unzip();
 
         let hit_count: HitCounts = hit_counts.into_iter().sum();

src/agent/coverage/src/source.rs

@@ -51,14 +51,13 @@ impl From<Line> for u32 {
 
 pub fn binary_to_source_coverage(
     binary: &BinaryCoverage,
-    source_allowlist: impl Into<Option<AllowList>>,
+    source_allowlist: &AllowList,
 ) -> Result<SourceCoverage> {
     use std::collections::btree_map::Entry;
 
     use symbolic::debuginfo::Object;
     use symbolic::symcache::{SymCache, SymCacheConverter};
 
-    let source_allowlist = source_allowlist.into().unwrap_or_default();
     let loader = Loader::new();
 
     let mut source = SourceCoverage::default();

src/agent/coverage/tests/snapshot.rs

@@ -54,7 +54,7 @@ fn windows_snapshot_tests() {
 
         // generate source-line coverage info
         let source =
-            coverage::source::binary_to_source_coverage(&recorded.coverage, source_allowlist)
+            coverage::source::binary_to_source_coverage(&recorded.coverage, &source_allowlist)
                 .expect("binary_to_source_coverage");
 
         let file_coverage = source

src/agent/onefuzz-file-format/src/coverage/binary.rs

@@ -32,8 +32,8 @@ impl BinaryCoverageJson {
 }
 
 // Convert into the latest format.
-impl From<BinaryCoverage> for BinaryCoverageJson {
-    fn from(source: BinaryCoverage) -> Self {
+impl From<&BinaryCoverage> for BinaryCoverageJson {
+    fn from(source: &BinaryCoverage) -> Self {
         v1::BinaryCoverageJson::from(source).into()
     }
 }

src/agent/onefuzz-file-format/src/coverage/binary/v1.rs

@@ -21,8 +21,8 @@ pub struct ModuleCoverageJson {
     pub blocks: BTreeMap<Hex, u32>,
 }
 
-impl From<BinaryCoverage> for BinaryCoverageJson {
-    fn from(binary: BinaryCoverage) -> Self {
+impl From<&BinaryCoverage> for BinaryCoverageJson {
+    fn from(binary: &BinaryCoverage) -> Self {
         let mut modules = BTreeMap::new();
 
         for (path, offsets) in &binary.modules {

src/agent/onefuzz-file-format/src/coverage/source.rs

@@ -32,8 +32,8 @@ impl SourceCoverageJson {
 }
 
 // Convert into the latest format.
-impl From<SourceCoverage> for SourceCoverageJson {
-    fn from(source: SourceCoverage) -> Self {
+impl From<&SourceCoverage> for SourceCoverageJson {
+    fn from(source: &SourceCoverage) -> Self {
         v1::SourceCoverageJson::from(source).into()
     }
 }

src/agent/onefuzz-file-format/src/coverage/source/v1.rs

@@ -23,14 +23,14 @@ pub struct FileCoverageJson {
     pub lines: BTreeMap<LineNumber, HitCount>,
 }
 
-impl From<SourceCoverage> for SourceCoverageJson {
-    fn from(source: SourceCoverage) -> Self {
+impl From<&SourceCoverage> for SourceCoverageJson {
+    fn from(source: &SourceCoverage) -> Self {
         let mut json = SourceCoverageJson::default();
 
-        for (path, file) in source.files {
+        for (path, file) in &source.files {
             let mut file_json = FileCoverageJson::default();
 
-            for (line, count) in file.lines {
+            for (line, count) in &file.lines {
                 let line_number = LineNumber(line.number());
                 let hit_count = count.0;
                 file_json.lines.insert(line_number, hit_count);