From 8ad49bf19eb1719128a107db63a960af085181e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Sewi=C5=82o?= <95349104+ksew1@users.noreply.github.com> Date: Tue, 3 Sep 2024 02:13:18 -0700 Subject: [PATCH] Optimize the coverage computation logic to avoid redundant processing of trace files referencing the same Sierra program. (#42) Closes #41 --- .../src/coverage_data/function.rs | 3 +- .../src/data_loader/loaded_data.rs | 70 +++++++++++++++++++ crates/cairo-coverage/src/data_loader/mod.rs | 5 ++ .../src/{input => }/data_loader/types.rs | 0 crates/cairo-coverage/src/input/data.rs | 62 ++++++---------- .../src/input/data_loader/mod.rs | 1 - crates/cairo-coverage/src/input/mod.rs | 2 - .../src/input/sierra_to_cairo_map.rs | 4 +- .../src/input/unique_executed_sierra_ids.rs | 11 +++ crates/cairo-coverage/src/main.rs | 7 +- crates/cairo-coverage/tests/e2e.rs | 55 ++++++++++++++- 11 files changed, 170 insertions(+), 50 deletions(-) create mode 100644 crates/cairo-coverage/src/data_loader/loaded_data.rs create mode 100644 crates/cairo-coverage/src/data_loader/mod.rs rename crates/cairo-coverage/src/{input => }/data_loader/types.rs (100%) delete mode 100644 crates/cairo-coverage/src/input/data_loader/mod.rs diff --git a/crates/cairo-coverage/src/coverage_data/function.rs b/crates/cairo-coverage/src/coverage_data/function.rs index 1ba92a2..111a38f 100644 --- a/crates/cairo-coverage/src/coverage_data/function.rs +++ b/crates/cairo-coverage/src/coverage_data/function.rs @@ -1,4 +1,5 @@ -use crate::input::{InputData, LineRange, SierraToCairoMap}; +use crate::data_loader::LineRange; +use crate::input::{InputData, SierraToCairoMap}; use crate::types::{FileLocation, FunctionName, HitCount, LineNumber}; use std::collections::HashMap; diff --git a/crates/cairo-coverage/src/data_loader/loaded_data.rs b/crates/cairo-coverage/src/data_loader/loaded_data.rs new file mode 100644 index 0000000..5ce59cf --- /dev/null +++ b/crates/cairo-coverage/src/data_loader/loaded_data.rs @@ -0,0 +1,70 @@ +use anyhow::Context; +use anyhow::Result; +use cairo_lang_sierra::debug_info::DebugInfo; +use cairo_lang_sierra::program::{Program, ProgramArtifact, VersionedProgram}; +use camino::Utf8PathBuf; +use derived_deref::Deref; +use serde::de::DeserializeOwned; +use std::collections::HashMap; +use std::fs; +use trace_data::CallTrace; + +type SourceSierraPath = String; + +#[derive(Deref)] +pub struct LoadedDataMap(HashMap); + +pub struct LoadedData { + pub program: Program, + pub debug_info: DebugInfo, + pub call_traces: Vec, +} + +impl LoadedDataMap { + pub fn load(call_trace_paths: &Vec) -> Result { + let mut map: HashMap = HashMap::new(); + for call_trace_path in call_trace_paths { + let call_trace: CallTrace = read_and_deserialize(call_trace_path)?; + + let source_sierra_path = &call_trace + .cairo_execution_info + .as_ref() + .context("Missing key 'cairo_execution_info' in call trace. Perhaps you have outdated scarb?")? + .source_sierra_path; + + if let Some(loaded_data) = map.get_mut(&source_sierra_path.to_string()) { + loaded_data.call_traces.push(call_trace); + } else { + let VersionedProgram::V1 { + program: + ProgramArtifact { + program, + debug_info, + }, + .. + } = read_and_deserialize(source_sierra_path)?; + + map.insert( + source_sierra_path.to_string(), + LoadedData { + program, + debug_info: debug_info + .context(format!("Debug info not found in: {source_sierra_path}"))?, + call_traces: vec![call_trace], + }, + ); + } + } + Ok(Self(map)) + } +} + +fn read_and_deserialize(file_path: &Utf8PathBuf) -> anyhow::Result { + fs::read_to_string(file_path) + .context(format!("Failed to read file at path: {file_path}")) + .and_then(|content| { + serde_json::from_str(&content).context(format!( + "Failed to deserialize JSON content from file at path: {file_path}" + )) + }) +} diff --git a/crates/cairo-coverage/src/data_loader/mod.rs b/crates/cairo-coverage/src/data_loader/mod.rs new file mode 100644 index 0000000..09d9bdb --- /dev/null +++ b/crates/cairo-coverage/src/data_loader/mod.rs @@ -0,0 +1,5 @@ +mod loaded_data; +mod types; + +pub use loaded_data::{LoadedData, LoadedDataMap}; +pub use types::{CodeLocation, CoverageAnnotations, LineRange, ProfilerAnnotations}; diff --git a/crates/cairo-coverage/src/input/data_loader/types.rs b/crates/cairo-coverage/src/data_loader/types.rs similarity index 100% rename from crates/cairo-coverage/src/input/data_loader/types.rs rename to crates/cairo-coverage/src/data_loader/types.rs diff --git a/crates/cairo-coverage/src/input/data.rs b/crates/cairo-coverage/src/input/data.rs index e2ae1a5..c4afbea 100644 --- a/crates/cairo-coverage/src/input/data.rs +++ b/crates/cairo-coverage/src/input/data.rs @@ -1,13 +1,10 @@ +use crate::data_loader::LoadedData; use crate::input::test_function_filter::TestFunctionFilter; use crate::input::{create_sierra_to_cairo_map, SierraToCairoMap, UniqueExecutedSierraIds}; use anyhow::{Context, Result}; -use cairo_lang_sierra::program::{Program, ProgramArtifact, VersionedProgram}; +use cairo_lang_sierra::program::Program; use cairo_lang_sierra_to_casm::compiler::{CairoProgram, SierraToCasmConfig}; use cairo_lang_sierra_to_casm::metadata::{calc_metadata, MetadataComputationConfig}; -use camino::Utf8PathBuf; -use serde::de::DeserializeOwned; -use std::fs; -use trace_data::CallTrace; const SNFORGE_TEST_EXECUTABLE: &str = "snforge_internal_test_executable"; @@ -17,27 +14,14 @@ pub struct InputData { } impl InputData { - pub fn new(call_trace_path: &Utf8PathBuf, include_test_functions: bool) -> Result { - let call_trace: CallTrace = read_and_deserialize(call_trace_path)?; - - let source_sierra_path = &call_trace - .cairo_execution_info - .as_ref() - .context("Missing key 'cairo_execution_info' in call trace")? - .source_sierra_path; - - let VersionedProgram::V1 { - program: - ProgramArtifact { - program, - debug_info, - }, - .. - } = read_and_deserialize(source_sierra_path)?; - - let debug_info = - debug_info.context(format!("Debug info not found in: {source_sierra_path}"))?; - + pub fn new( + LoadedData { + program, + debug_info, + call_traces, + }: &LoadedData, + include_test_functions: bool, + ) -> Result { let test_function_filter = TestFunctionFilter::new( debug_info .executables @@ -46,10 +30,18 @@ impl InputData { include_test_functions, ); - let sierra_to_cairo_map = create_sierra_to_cairo_map(&debug_info, &test_function_filter)?; - let casm = compile_to_casm(&program)?; - let unique_executed_sierra_ids = - UniqueExecutedSierraIds::new(&casm, &call_trace, &sierra_to_cairo_map)?; + let sierra_to_cairo_map = create_sierra_to_cairo_map(debug_info, &test_function_filter)?; + let casm = compile_to_casm(program)?; + let unique_executed_sierra_ids = call_traces + .iter() + .map(|call_trace| UniqueExecutedSierraIds::new(&casm, call_trace, &sierra_to_cairo_map)) + .collect::>>()? + .into_iter() + .reduce(|mut acc, unique_executed_sierra_ids| { + acc.extend(unique_executed_sierra_ids.clone().into_iter()); + acc + }) + .context("Failed to create unique executed sierra ids")?; Ok(Self { unique_executed_sierra_ids, @@ -70,13 +62,3 @@ fn compile_to_casm(program: &Program) -> Result { ) .context("Failed to compile sierra to casm") } - -fn read_and_deserialize(file_path: &Utf8PathBuf) -> Result { - fs::read_to_string(file_path) - .context(format!("Failed to read file at path: {file_path}")) - .and_then(|content| { - serde_json::from_str(&content).context(format!( - "Failed to deserialize JSON content from file at path: {file_path}" - )) - }) -} diff --git a/crates/cairo-coverage/src/input/data_loader/mod.rs b/crates/cairo-coverage/src/input/data_loader/mod.rs deleted file mode 100644 index cd40856..0000000 --- a/crates/cairo-coverage/src/input/data_loader/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub mod types; diff --git a/crates/cairo-coverage/src/input/mod.rs b/crates/cairo-coverage/src/input/mod.rs index 9ca8873..a090f81 100644 --- a/crates/cairo-coverage/src/input/mod.rs +++ b/crates/cairo-coverage/src/input/mod.rs @@ -1,10 +1,8 @@ mod data; -mod data_loader; mod sierra_to_cairo_map; mod test_function_filter; mod unique_executed_sierra_ids; pub use data::InputData; -pub use data_loader::types::LineRange; pub use sierra_to_cairo_map::{create_sierra_to_cairo_map, SierraToCairoMap}; pub use unique_executed_sierra_ids::UniqueExecutedSierraIds; diff --git a/crates/cairo-coverage/src/input/sierra_to_cairo_map.rs b/crates/cairo-coverage/src/input/sierra_to_cairo_map.rs index 6a949d7..c6f84e1 100644 --- a/crates/cairo-coverage/src/input/sierra_to_cairo_map.rs +++ b/crates/cairo-coverage/src/input/sierra_to_cairo_map.rs @@ -1,6 +1,4 @@ -use crate::input::data_loader::types::{ - CodeLocation, CoverageAnnotations, LineRange, ProfilerAnnotations, -}; +use crate::data_loader::{CodeLocation, CoverageAnnotations, LineRange, ProfilerAnnotations}; use crate::input::test_function_filter::TestFunctionFilter; use crate::types::{FileLocation, FunctionName}; use anyhow::{Context, Result}; diff --git a/crates/cairo-coverage/src/input/unique_executed_sierra_ids.rs b/crates/cairo-coverage/src/input/unique_executed_sierra_ids.rs index 3d575a8..63b1dca 100644 --- a/crates/cairo-coverage/src/input/unique_executed_sierra_ids.rs +++ b/crates/cairo-coverage/src/input/unique_executed_sierra_ids.rs @@ -10,6 +10,17 @@ use trace_data::{CallTrace, CasmLevelInfo}; #[derive(Deref)] pub struct UniqueExecutedSierraIds(HashMap); +impl Extend<(StatementIdx, usize)> for UniqueExecutedSierraIds { + fn extend>(&mut self, iter: T) { + for (key, value) in iter { + self.0 + .entry(key) + .and_modify(|e| *e += value) + .or_insert(value); + } + } +} + impl UniqueExecutedSierraIds { pub fn new( casm: &CairoProgram, diff --git a/crates/cairo-coverage/src/main.rs b/crates/cairo-coverage/src/main.rs index 812834d..1e4d3b8 100644 --- a/crates/cairo-coverage/src/main.rs +++ b/crates/cairo-coverage/src/main.rs @@ -1,10 +1,12 @@ mod cli; mod coverage_data; +mod data_loader; mod input; mod output; mod types; use crate::coverage_data::create_files_coverage_data_with_hits; +use crate::data_loader::LoadedDataMap; use crate::input::InputData; use crate::output::lcov::LcovFormat; use anyhow::{Context, Result}; @@ -24,8 +26,9 @@ fn main() -> Result<()> { .open(output_path) .context(format!("Failed to open output file at path: {output_path}"))?; - for trace_file in &cli.trace_files { - let input_data = InputData::new(trace_file, cli.include_test_functions)?; + let loaded_data = LoadedDataMap::load(&cli.trace_files)?; + for (_, loaded_data) in loaded_data.iter() { + let input_data = InputData::new(loaded_data, cli.include_test_functions)?; let coverage_data = create_files_coverage_data_with_hits(&input_data); let output_data = LcovFormat::from(coverage_data); diff --git a/crates/cairo-coverage/tests/e2e.rs b/crates/cairo-coverage/tests/e2e.rs index be9b0e8..8e9902c 100644 --- a/crates/cairo-coverage/tests/e2e.rs +++ b/crates/cairo-coverage/tests/e2e.rs @@ -95,13 +95,66 @@ fn scarb_template() { } #[test] -#[ignore] // Output to big fix in next test fn complex_calculator() { let output = run_test_project("complex_calculator").unwrap(); assert_eq!( output, indoc! { " + TN: + SF:tests/data/complex_calculator/src/lib.cairo + FN:2,2,complex_calculator::add + FNDA:2,complex_calculator::add + FN:17,21,complex_calculator::divide + FNDA:1,complex_calculator::divide + FN:25,30,complex_calculator::factorial + FNDA:12,complex_calculator::factorial + FN:45,49,complex_calculator::fibonacci + FNDA:2,complex_calculator::fibonacci + FN:53,63,complex_calculator::is_prime + FNDA:84,complex_calculator::is_prime + FN:10,10,complex_calculator::multiply + FNDA:2,complex_calculator::multiply + FN:35,40,complex_calculator::power + FNDA:10,complex_calculator::power + FN:6,6,complex_calculator::subtract + FNDA:2,complex_calculator::subtract + FN:14,14,complex_calculator::unsafe_divide + FNDA:0,complex_calculator::unsafe_divide + FNF:9 + FNH:8 + DA:2,2 + DA:6,2 + DA:10,2 + DA:14,0 + DA:17,1 + DA:18,1 + DA:19,1 + DA:21,0 + DA:25,1 + DA:28,12 + DA:29,10 + DA:30,10 + DA:35,2 + DA:38,10 + DA:39,6 + DA:40,6 + DA:45,2 + DA:46,1 + DA:47,0 + DA:48,1 + DA:49,0 + DA:53,2 + DA:54,4 + DA:55,0 + DA:58,0 + DA:59,84 + DA:60,80 + DA:61,0 + DA:63,80 + LF:29 + LH:22 + end_of_record " } );