Skip to content

Commit

Permalink
refactor: commit function (#297)
Browse files Browse the repository at this point in the history
* refactor: change commit function signature

* refactor: deserialize commiter input in test

* refactor: add deref for factmap
  • Loading branch information
aner-starkware authored Jul 11, 2024
1 parent 8b0b1e6 commit f9d223a
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 19 deletions.
18 changes: 13 additions & 5 deletions crates/committer_cli/benches/committer_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ use committer::{
types::NodeIndex,
},
};
use committer_cli::{commands::commit, tests::utils::parse_from_python::TreeFlowInput};
use committer_cli::{
commands::commit, parse_input::read::parse_input,
tests::utils::parse_from_python::TreeFlowInput,
};
use criterion::{criterion_group, criterion_main, Criterion};

const CONCURRENCY_MODE: bool = true;
Expand Down Expand Up @@ -58,12 +61,17 @@ pub fn full_committer_flow_benchmark(criterion: &mut Criterion) {

// TODO(Aner, 8/7/2024): use structs for deserialization.
let input: HashMap<String, String> = serde_json::from_str(FLOW_TEST_INPUT).unwrap();
let committer_input = input.get("committer_input").unwrap();

//TODO(Aner, 27/06/2024): output path should be a pipe (file on memory) to avoid disk IO in the benchmark.
let committer_input_string = input.get("committer_input").unwrap();
// TODO(Aner, 27/06/2024): output path should be a pipe (file on memory)
// to avoid disk IO in the benchmark.
// TODO(Aner, 11/7/24): consider moving function to production code.
async fn parse_and_commit(input_str: &str) {
let committer_input = parse_input(input_str).expect("Failed to parse the given input.");
commit(committer_input, OUTPUT_PATH.to_owned()).await;
}
criterion.bench_function("full_committer_flow", |benchmark| {
benchmark.iter(|| {
runtime.block_on(commit(committer_input, OUTPUT_PATH.to_owned()));
runtime.block_on(parse_and_commit(committer_input_string));
})
});
}
Expand Down
11 changes: 6 additions & 5 deletions crates/committer_cli/src/commands.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use committer::block_committer::commit::commit_block;
use committer::block_committer::{
commit::commit_block,
input::{ConfigImpl, Input},
};

use crate::{
filled_tree_output::filled_forest::SerializedForest,
parse_input::read::{parse_input, write_to_file},
filled_tree_output::filled_forest::SerializedForest, parse_input::read::write_to_file,
};

pub async fn commit(input_string: &str, output_path: String) {
let input = parse_input(input_string).expect("Failed to parse the given input.");
pub async fn commit(input: Input<ConfigImpl>, output_path: String) {
let serialized_filled_forest = SerializedForest(
commit_block(input)
.await
Expand Down
8 changes: 5 additions & 3 deletions crates/committer_cli/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use clap::{Args, Parser, Subcommand};
use committer_cli::block_hash::{BlockCommitmentsInput, BlockHashInput};
use committer_cli::commands::commit;
use committer_cli::parse_input::read::{load_from_stdin, read_from_stdin, write_to_file};
use committer_cli::parse_input::read::{
load_from_stdin, parse_input, read_from_stdin, write_to_file,
};
use committer_cli::tests::python_tests::PythonTest;
use simplelog::{ColorChoice, Config, LevelFilter, TermLogger, TerminalMode};
use starknet_api::block_hash::block_hash_calculator::{
Expand Down Expand Up @@ -71,8 +73,8 @@ async fn main() {

match args.command {
Command::Commit { output_path } => {
let input_string = read_from_stdin();
commit(&input_string, output_path).await;
let input = parse_input(&read_from_stdin()).expect("Failed to parse the given input.");
commit(input, output_path).await;
}

Command::PythonTest {
Expand Down
31 changes: 25 additions & 6 deletions crates/committer_cli/src/tests/regression_tests.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
use std::collections::HashMap;

use committer::patricia_merkle_tree::external_test_utils::single_tree_flow_test;
use committer::{
block_committer::input::{ConfigImpl, Input},
patricia_merkle_tree::external_test_utils::single_tree_flow_test,
};
use serde::{Deserialize, Deserializer};
use serde_json::{Map, Value};

use crate::{commands::commit, tests::utils::parse_from_python::TreeFlowInput};
use crate::{
commands::commit, parse_input::read::parse_input,
tests::utils::parse_from_python::TreeFlowInput,
};

use super::utils::parse_from_python::parse_input_single_storage_tree_flow_test;

Expand All @@ -17,6 +23,7 @@ const SINGLE_TREE_FLOW_INPUT: &str = include_str!("../../benches/tree_flow_input
const FLOW_TEST_INPUT: &str = include_str!("../../benches/committer_flow_inputs.json");
const OUTPUT_PATH: &str = "benchmark_output.txt";

#[derive(derive_more::Deref)]
struct FactMap(Map<String, Value>);

impl<'de> Deserialize<'de> for FactMap {
Expand All @@ -30,9 +37,22 @@ impl<'de> Deserialize<'de> for FactMap {
}
}

struct CommitterInput(Input<ConfigImpl>);

impl<'de> Deserialize<'de> for CommitterInput {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
Ok(Self(
parse_input(&String::deserialize(deserializer)?).unwrap(),
))
}
}

#[derive(Deserialize)]
struct CommitterRegressionInput {
committer_input: String,
committer_input: CommitterInput,
contract_states_root: String,
contract_classes_root: String,
expected_facts: FactMap,
Expand Down Expand Up @@ -127,8 +147,7 @@ pub async fn test_regression_committer_flow() {

let start = std::time::Instant::now();
// Benchmark the committer flow test.
// TODO(Aner, 9/7/2024): refactor committer_input to be a struct.
commit(&committer_input, OUTPUT_PATH.to_owned()).await;
commit(committer_input.0, OUTPUT_PATH.to_owned()).await;
let execution_time = std::time::Instant::now() - start;

// Assert correctness of the output of the committer flow test.
Expand All @@ -145,7 +164,7 @@ pub async fn test_regression_committer_flow() {

assert_eq!(contract_storage_root_hash, expected_contract_states_root);
assert_eq!(compiled_class_root_hash, expected_contract_classes_root);
assert_eq!(storage_changes, expected_facts.0);
assert_eq!(storage_changes, *expected_facts);

// Assert the execution time does not exceed the threshold.
assert!(execution_time.as_secs_f64() < MAX_TIME_FOR_COMMITTER_FLOW_BECHMARK_TEST);
Expand Down

0 comments on commit f9d223a

Please sign in to comment.