Skip to content

Commit

Permalink
Save sierra test code separately for profiler (#2065)
Browse files Browse the repository at this point in the history
## Introduced changes

- Test artifacts file usually had 2 structs in it:
`CompiledTestCrateRaw` for `src/` and `CompiledTestCrateRaw` for
`tests/`, each with a separate sierra program. This made profiler unable
to determine which sierra it should use for function level profiling
(only path to the artifact file was passed to profiler). Now we save the
relevant sierra program to another file and pass a path to the trace.
This way profiler knows what sierra it should use. It also makes
profiler less coupled with snforge (it doesn't have to know forge test
artifact format)

## Checklist

- [x] Linked relevant issue
- [x] Updated relevant documentation
- [x] Added relevant tests
- [x] Performed self-review of the code
- [x] Added changes to `CHANGELOG.md`
  • Loading branch information
piotmag769 authored May 8, 2024
1 parent da1b422 commit 709ad03
Show file tree
Hide file tree
Showing 19 changed files with 233 additions and 95 deletions.
48 changes: 30 additions & 18 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,4 @@ p256 = { version = "0.13.2", features = ["sha256", "ecdsa", "serde"] }
glob = "0.3.1"
sha3 = "0.10.8"
base16ct = { version = "0.2.0", features = ["alloc"] }
fs4 = "0.7"
1 change: 1 addition & 0 deletions crates/forge-runner/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,4 @@ conversions = { path = "../conversions" }
scarb-api = { path = "../scarb-api" }
shared = { path = "../shared" }
universal-sierra-compiler-api = { path = "../universal-sierra-compiler-api" }
fs4.workspace = true
33 changes: 22 additions & 11 deletions crates/forge-runner/src/build_trace_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ use std::fs;
use std::path::PathBuf;
use std::rc::Rc;

use crate::build_trace_data::test_sierra_program_path::VersionedProgramPath;
use blockifier::execution::deprecated_syscalls::DeprecatedSyscallSelector;
use blockifier::execution::entry_point::{CallEntryPoint, CallType};
use blockifier::execution::syscalls::hint_processor::SyscallCounter;
use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
use cairo_vm::vm::trace::trace_entry::TraceEntry;
use camino::{Utf8Path, Utf8PathBuf};
use camino::Utf8PathBuf;
use cheatnet::constants::{TEST_CONTRACT_CLASS_HASH, TEST_ENTRY_POINT_SELECTOR};
use cheatnet::runtime_extensions::forge_runtime_extension::contracts_data::ContractsData;
use cheatnet::state::{CallTrace, CallTraceNode};
Expand All @@ -30,14 +31,17 @@ use trace_data::{
VmExecutionResources,
};

pub mod test_sierra_program_path;

pub const TRACE_DIR: &str = ".snfoundry_trace";

pub const TEST_CODE_CONTRACT_NAME: &str = "SNFORGE_TEST_CODE";
pub const TEST_CODE_FUNCTION_NAME: &str = "SNFORGE_TEST_CODE_FUNCTION";

pub fn build_profiler_call_trace(
value: &Rc<RefCell<CallTrace>>,
contracts_data: &ContractsData,
test_artifacts_path: &Utf8PathBuf,
maybe_versioned_program_path: &Option<VersionedProgramPath>,
) -> ProfilerCallTrace {
let value = value.borrow();

Expand All @@ -52,7 +56,7 @@ pub fn build_profiler_call_trace(
&value.entry_point,
vm_trace,
contracts_data,
test_artifacts_path,
maybe_versioned_program_path,
);

ProfilerCallTrace {
Expand All @@ -65,7 +69,9 @@ pub fn build_profiler_call_trace(
nested_calls: value
.nested_calls
.iter()
.map(|c| build_profiler_call_trace_node(c, contracts_data, test_artifacts_path))
.map(|c| {
build_profiler_call_trace_node(c, contracts_data, maybe_versioned_program_path)
})
.collect(),
cairo_execution_info,
}
Expand All @@ -75,11 +81,12 @@ fn build_cairo_execution_info(
entry_point: &CallEntryPoint,
vm_trace: Option<Vec<ProfilerTraceEntry>>,
contracts_data: &ContractsData,
test_artifacts_path: &Utf8Path,
maybe_test_sierra_program_path: &Option<VersionedProgramPath>,
) -> Option<CairoExecutionInfo> {
let contract_name = get_contract_name(entry_point.class_hash, contracts_data);
let source_sierra_path = contract_name
.and_then(|name| get_source_sierra_path(&name, contracts_data, test_artifacts_path));
let source_sierra_path = contract_name.and_then(|name| {
get_source_sierra_path(&name, contracts_data, maybe_test_sierra_program_path)
});

#[allow(clippy::unnecessary_unwrap)]
if source_sierra_path.is_some() && vm_trace.is_some() {
Expand All @@ -95,10 +102,14 @@ fn build_cairo_execution_info(
fn get_source_sierra_path(
contract_name: &str,
contracts_data: &ContractsData,
test_artifacts_path: &Utf8Path,
maybe_versioned_program_path: &Option<VersionedProgramPath>,
) -> Option<Utf8PathBuf> {
if contract_name == TEST_CODE_CONTRACT_NAME {
Some(Utf8PathBuf::from(test_artifacts_path))
Some(
maybe_versioned_program_path
.clone()
.map_or_else(Utf8PathBuf::new, Into::into),
)
} else {
contracts_data
.get_source_sierra_path(contract_name)
Expand All @@ -109,11 +120,11 @@ fn get_source_sierra_path(
fn build_profiler_call_trace_node(
value: &CallTraceNode,
contracts_data: &ContractsData,
test_artifacts_path: &Utf8PathBuf,
maybe_versioned_program_path: &Option<VersionedProgramPath>,
) -> ProfilerCallTraceNode {
match value {
CallTraceNode::EntryPointCall(trace) => ProfilerCallTraceNode::EntryPointCall(
build_profiler_call_trace(trace, contracts_data, test_artifacts_path),
build_profiler_call_trace(trace, contracts_data, maybe_versioned_program_path),
),
CallTraceNode::DeployWithoutConstructor => ProfilerCallTraceNode::DeployWithoutConstructor,
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
use crate::compiled_runnable::CrateLocation;
use anyhow::Context;
use anyhow::Result;
use cairo_lang_sierra::program::VersionedProgram;
use camino::{Utf8Path, Utf8PathBuf};
use fs4::FileExt;
use std::fs;
use std::fs::File;
use std::io::BufWriter;

pub const VERSIONED_PROGRAMS_DIR: &str = ".snfoundry_versioned_programs";

/// A path to a file with deserialized [`VersionedProgram`] that comes
/// from compiled test crate. Needed to provide path to source sierra in
/// [`trace_data::CairoExecutionInfo`].
#[derive(Clone)]
pub struct VersionedProgramPath(Utf8PathBuf);

impl VersionedProgramPath {
pub fn save_versioned_program(
versioned_program: &VersionedProgram,
crate_location: CrateLocation,
tests_programs_dir: &Utf8Path,
package_name: &str,
) -> Result<Self> {
// unique filename since pair (package_name, crate_location) is always unique
let test_sierra_program_path =
tests_programs_dir.join(format!("{package_name}_{crate_location:?}.sierra.json",));

fs::create_dir_all(test_sierra_program_path.parent().unwrap())
.context("Failed to create directory for tests sierra programs")?;
let output_file = File::options()
.create(true)
.write(true)
.truncate(true)
.open(&test_sierra_program_path)?;

output_file.lock_exclusive().with_context(|| {
format!("Couldn't lock the output file = {test_sierra_program_path}")
})?;
let file = BufWriter::new(&output_file);
serde_json::to_writer(file, &versioned_program)
.context("Failed to serialize VersionedProgram")?;
output_file.unlock().with_context(|| {
format!("Couldn't lock the output file = {test_sierra_program_path}")
})?;

Ok(Self(test_sierra_program_path))
}
}

impl From<VersionedProgramPath> for Utf8PathBuf {
fn from(value: VersionedProgramPath) -> Self {
value.0
}
}
14 changes: 12 additions & 2 deletions crates/forge-runner/src/compiled_runnable.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
use crate::expected_result::ExpectedTestResult;
use cairo_lang_sierra::{ids::GenericTypeId, program::Program};
use cairo_lang_sierra::ids::GenericTypeId;
use cairo_lang_sierra::program::ProgramArtifact;
use serde::Deserialize;
use starknet_api::block::BlockNumber;
use std::num::NonZeroU32;
use url::Url;

#[derive(Debug, Clone)]
pub struct CompiledTestCrateRunnable {
pub sierra_program: Program,
pub tests_location: CrateLocation,
pub sierra_program: ProgramArtifact,
pub test_cases: Vec<TestCaseRunnable>,
}

#[derive(Debug, PartialEq, Clone, Copy, Deserialize, Hash, Eq)]
pub enum CrateLocation {
/// Main crate in a package
Lib,
/// Crate in the `tests/` directory
Tests,
}

#[derive(Debug, Clone)]
pub struct TestCaseRunnable {
pub name: String,
Expand Down
12 changes: 1 addition & 11 deletions crates/forge-runner/src/forge_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,13 @@ pub struct TestRunnerConfig {
pub cache_dir: Utf8PathBuf,
pub contracts_data: ContractsData,
pub environment_variables: HashMap<String, String>,
pub test_artifacts_path: Utf8PathBuf,
}

#[derive(Debug, PartialEq)]
pub struct OutputConfig {
pub detailed_resources: bool,
pub execution_data_to_save: ExecutionDataToSave,
}

impl OutputConfig {
#[must_use]
pub fn new(detailed_resources: bool, save_trace_data: bool, build_profile: bool) -> Self {
Self {
detailed_resources,
execution_data_to_save: ExecutionDataToSave::from_flags(save_trace_data, build_profile),
}
}
pub versioned_programs_dir: Utf8PathBuf,
}

#[derive(Debug, PartialEq, Clone, Copy)]
Expand Down
Loading

0 comments on commit 709ad03

Please sign in to comment.