Skip to content

Commit

Permalink
[subsytem-bench] Remove redundant banchmark_name param (#4540)
Browse files Browse the repository at this point in the history
Fixes #3601

Since we print benchmark results manually, we don't need to save
benchmark_name anywhere, better just put the name inside `println!`.
  • Loading branch information
AndreiEres authored May 28, 2024
1 parent 523e625 commit 3bf283f
Show file tree
Hide file tree
Showing 10 changed files with 27 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fn main() -> Result<(), String> {
print!("\r[{}{}]", "#".repeat(n), "_".repeat(BENCH_COUNT - n));
std::io::stdout().flush().unwrap();
let (mut env, state) = prepare_test(config.clone(), options.clone(), false);
env.runtime().block_on(bench_approvals("approvals_throughput", &mut env, state))
env.runtime().block_on(bench_approvals(&mut env, state))
})
.collect();
println!("\rDone!{}", " ".repeat(BENCH_COUNT));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,7 @@ fn main() -> Result<(), String> {
polkadot_subsystem_bench::availability::TestDataAvailability::Write,
false,
);
env.runtime().block_on(benchmark_availability_write(
"data_availability_write",
&mut env,
&state,
))
env.runtime().block_on(benchmark_availability_write(&mut env, &state))
})
.collect();
println!("\rDone!{}", " ".repeat(BENCH_COUNT));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,7 @@ fn main() -> Result<(), String> {
std::io::stdout().flush().unwrap();
let (mut env, _cfgs) =
prepare_test(&state, TestDataAvailability::Read(options.clone()), false);
env.runtime().block_on(benchmark_availability_read(
"data_availability_read",
&mut env,
&state,
))
env.runtime().block_on(benchmark_availability_read(&mut env, &state))
})
.collect();
println!("\rDone!{}", " ".repeat(BENCH_COUNT));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@ fn main() -> Result<(), String> {
print!("\r[{}{}]", "#".repeat(n), "_".repeat(BENCH_COUNT - n));
std::io::stdout().flush().unwrap();
let (mut env, _cfgs) = prepare_test(&state, false);
env.runtime().block_on(benchmark_statement_distribution(
"statement-distribution",
&mut env,
&state,
))
env.runtime().block_on(benchmark_statement_distribution(&mut env, &state))
})
.collect();
println!("\rDone!{}", " ".repeat(BENCH_COUNT));
Expand Down
29 changes: 8 additions & 21 deletions polkadot/node/subsystem-bench/src/cli/subsystem-bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,8 @@ impl BenchCli {
availability::TestDataAvailability::Read(opts),
true,
);
env.runtime().block_on(availability::benchmark_availability_read(
&benchmark_name,
&mut env,
&state,
))
env.runtime()
.block_on(availability::benchmark_availability_read(&mut env, &state))
},
TestObjective::DataAvailabilityWrite => {
let state = availability::TestState::new(&test_config);
Expand All @@ -158,32 +155,22 @@ impl BenchCli {
availability::TestDataAvailability::Write,
true,
);
env.runtime().block_on(availability::benchmark_availability_write(
&benchmark_name,
&mut env,
&state,
))
env.runtime()
.block_on(availability::benchmark_availability_write(&mut env, &state))
},
TestObjective::ApprovalVoting(ref options) => {
let (mut env, state) =
approval::prepare_test(test_config.clone(), options.clone(), true);
env.runtime().block_on(approval::bench_approvals(
&benchmark_name,
&mut env,
state,
))
env.runtime().block_on(approval::bench_approvals(&mut env, state))
},
TestObjective::StatementDistribution => {
let state = statement::TestState::new(&test_config);
let (mut env, _protocol_config) = statement::prepare_test(&state, true);
env.runtime().block_on(statement::benchmark_statement_distribution(
&benchmark_name,
&mut env,
&state,
))
env.runtime()
.block_on(statement::benchmark_statement_distribution(&mut env, &state))
},
};
println!("{}", usage);
println!("\n{}\n{}", benchmark_name.purple(), usage);
}

if let Some(agent_running) = agent_running {
Expand Down
6 changes: 2 additions & 4 deletions polkadot/node/subsystem-bench/src/lib/approval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,6 @@ fn prepare_test_inner(
}

pub async fn bench_approvals(
benchmark_name: &str,
env: &mut TestEnvironment,
mut state: ApprovalTestState,
) -> BenchmarkUsage {
Expand All @@ -900,12 +899,11 @@ pub async fn bench_approvals(
env.registry().clone(),
)
.await;
bench_approvals_run(benchmark_name, env, state, producer_rx).await
bench_approvals_run(env, state, producer_rx).await
}

/// Runs the approval benchmark.
pub async fn bench_approvals_run(
benchmark_name: &str,
env: &mut TestEnvironment,
state: ApprovalTestState,
producer_rx: oneshot::Receiver<()>,
Expand Down Expand Up @@ -1072,5 +1070,5 @@ pub async fn bench_approvals_run(
state.total_unique_messages.load(std::sync::atomic::Ordering::SeqCst)
);

env.collect_resource_usage(benchmark_name, &["approval-distribution", "approval-voting"])
env.collect_resource_usage(&["approval-distribution", "approval-voting"])
}
13 changes: 6 additions & 7 deletions polkadot/node/subsystem-bench/src/lib/availability/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ pub fn prepare_test(
}

pub async fn benchmark_availability_read(
benchmark_name: &str,
env: &mut TestEnvironment,
state: &TestState,
) -> BenchmarkUsage {
Expand Down Expand Up @@ -373,11 +372,10 @@ pub async fn benchmark_availability_read(
);

env.stop().await;
env.collect_resource_usage(benchmark_name, &["availability-recovery"])
env.collect_resource_usage(&["availability-recovery"])
}

pub async fn benchmark_availability_write(
benchmark_name: &str,
env: &mut TestEnvironment,
state: &TestState,
) -> BenchmarkUsage {
Expand Down Expand Up @@ -508,8 +506,9 @@ pub async fn benchmark_availability_write(
);

env.stop().await;
env.collect_resource_usage(
benchmark_name,
&["availability-distribution", "bitfield-distribution", "availability-store"],
)
env.collect_resource_usage(&[
"availability-distribution",
"bitfield-distribution",
"availability-store",
])
}
7 changes: 1 addition & 6 deletions polkadot/node/subsystem-bench/src/lib/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,13 +351,8 @@ impl TestEnvironment {
}
}

pub fn collect_resource_usage(
&self,
benchmark_name: &str,
subsystems_under_test: &[&str],
) -> BenchmarkUsage {
pub fn collect_resource_usage(&self, subsystems_under_test: &[&str]) -> BenchmarkUsage {
BenchmarkUsage {
benchmark_name: benchmark_name.to_string(),
network_usage: self.network_usage(),
cpu_usage: self.cpu_usage(subsystems_under_test),
}
Expand Down
3 changes: 1 addition & 2 deletions polkadot/node/subsystem-bench/src/lib/statement/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ pub fn generate_topology(test_authorities: &TestAuthorities) -> SessionGridTopol
}

pub async fn benchmark_statement_distribution(
benchmark_name: &str,
env: &mut TestEnvironment,
state: &TestState,
) -> BenchmarkUsage {
Expand Down Expand Up @@ -446,5 +445,5 @@ pub async fn benchmark_statement_distribution(
);

env.stop().await;
env.collect_resource_usage(benchmark_name, &["statement-distribution"])
env.collect_resource_usage(&["statement-distribution"])
}
23 changes: 5 additions & 18 deletions polkadot/node/subsystem-bench/src/lib/usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use std::collections::HashMap;

#[derive(Debug, Serialize, Deserialize, Clone)]
pub struct BenchmarkUsage {
pub benchmark_name: String,
pub network_usage: Vec<ResourceUsage>,
pub cpu_usage: Vec<ResourceUsage>,
}
Expand All @@ -32,8 +31,7 @@ impl std::fmt::Display for BenchmarkUsage {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(
f,
"\n{}\n\n{}\n{}\n\n{}\n{}\n",
self.benchmark_name.purple(),
"\n{}\n{}\n\n{}\n{}\n",
format!("{:<32}{:>12}{:>12}", "Network usage, KiB", "total", "per block").blue(),
self.network_usage
.iter()
Expand All @@ -59,18 +57,17 @@ impl BenchmarkUsage {
let all_cpu_usage: Vec<&ResourceUsage> = usages.iter().flat_map(|v| &v.cpu_usage).collect();

Self {
benchmark_name: usages.first().map(|v| v.benchmark_name.clone()).unwrap_or_default(),
network_usage: ResourceUsage::average_by_resource_name(&all_network_usages),
cpu_usage: ResourceUsage::average_by_resource_name(&all_cpu_usage),
}
}

pub fn check_network_usage(&self, checks: &[ResourceUsageCheck]) -> Vec<String> {
check_usage(&self.benchmark_name, &self.network_usage, checks)
check_usage(&self.network_usage, checks)
}

pub fn check_cpu_usage(&self, checks: &[ResourceUsageCheck]) -> Vec<String> {
check_usage(&self.benchmark_name, &self.cpu_usage, checks)
check_usage(&self.cpu_usage, checks)
}

pub fn cpu_usage_diff(&self, other: &Self, resource_name: &str) -> Option<f64> {
Expand Down Expand Up @@ -105,18 +102,8 @@ impl BenchmarkUsage {
}
}

fn check_usage(
benchmark_name: &str,
usage: &[ResourceUsage],
checks: &[ResourceUsageCheck],
) -> Vec<String> {
checks
.iter()
.filter_map(|check| {
check_resource_usage(usage, check)
.map(|message| format!("{}: {}", benchmark_name, message))
})
.collect()
fn check_usage(usage: &[ResourceUsage], checks: &[ResourceUsageCheck]) -> Vec<String> {
checks.iter().filter_map(|check| check_resource_usage(usage, check)).collect()
}

fn check_resource_usage(
Expand Down

0 comments on commit 3bf283f

Please sign in to comment.