From 3bf283ff22224e7713cf0c1b9878e9137dc6dbf7 Mon Sep 17 00:00:00 2001
From: Andrei Eres <eresav@me.com>
Date: Tue, 28 May 2024 10:51:40 +0200
Subject: [PATCH] [subsytem-bench] Remove redundant banchmark_name param
 (#4540)

Fixes https://github.com/paritytech/polkadot-sdk/issues/3601

Since we print benchmark results manually, we don't need to save
benchmark_name anywhere, better just put the name inside `println!`.
---
 .../approval-voting-regression-bench.rs       |  2 +-
 ...ilability-distribution-regression-bench.rs |  6 +---
 .../availability-recovery-regression-bench.rs |  6 +---
 ...statement-distribution-regression-bench.rs |  6 +---
 .../src/cli/subsystem-bench.rs                | 29 +++++--------------
 .../subsystem-bench/src/lib/approval/mod.rs   |  6 ++--
 .../src/lib/availability/mod.rs               | 13 ++++-----
 .../subsystem-bench/src/lib/environment.rs    |  7 +----
 .../subsystem-bench/src/lib/statement/mod.rs  |  3 +-
 .../node/subsystem-bench/src/lib/usage.rs     | 23 ++++-----------
 10 files changed, 27 insertions(+), 74 deletions(-)

diff --git a/polkadot/node/core/approval-voting/benches/approval-voting-regression-bench.rs b/polkadot/node/core/approval-voting/benches/approval-voting-regression-bench.rs
index 280b8c53f7d..687063dd0eb 100644
--- a/polkadot/node/core/approval-voting/benches/approval-voting-regression-bench.rs
+++ b/polkadot/node/core/approval-voting/benches/approval-voting-regression-bench.rs
@@ -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));
diff --git a/polkadot/node/network/availability-distribution/benches/availability-distribution-regression-bench.rs b/polkadot/node/network/availability-distribution/benches/availability-distribution-regression-bench.rs
index 72278b5770b..6083a90e481 100644
--- a/polkadot/node/network/availability-distribution/benches/availability-distribution-regression-bench.rs
+++ b/polkadot/node/network/availability-distribution/benches/availability-distribution-regression-bench.rs
@@ -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));
diff --git a/polkadot/node/network/availability-recovery/benches/availability-recovery-regression-bench.rs b/polkadot/node/network/availability-recovery/benches/availability-recovery-regression-bench.rs
index e5a8f1eb7c9..c734ac99e87 100644
--- a/polkadot/node/network/availability-recovery/benches/availability-recovery-regression-bench.rs
+++ b/polkadot/node/network/availability-recovery/benches/availability-recovery-regression-bench.rs
@@ -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));
diff --git a/polkadot/node/network/statement-distribution/benches/statement-distribution-regression-bench.rs b/polkadot/node/network/statement-distribution/benches/statement-distribution-regression-bench.rs
index abcb1e6783f..9cbe385e3f4 100644
--- a/polkadot/node/network/statement-distribution/benches/statement-distribution-regression-bench.rs
+++ b/polkadot/node/network/statement-distribution/benches/statement-distribution-regression-bench.rs
@@ -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));
diff --git a/polkadot/node/subsystem-bench/src/cli/subsystem-bench.rs b/polkadot/node/subsystem-bench/src/cli/subsystem-bench.rs
index 1e921500a4d..346a058b979 100644
--- a/polkadot/node/subsystem-bench/src/cli/subsystem-bench.rs
+++ b/polkadot/node/subsystem-bench/src/cli/subsystem-bench.rs
@@ -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);
@@ -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 {
diff --git a/polkadot/node/subsystem-bench/src/lib/approval/mod.rs b/polkadot/node/subsystem-bench/src/lib/approval/mod.rs
index 4a479b6af29..2e5831276ad 100644
--- a/polkadot/node/subsystem-bench/src/lib/approval/mod.rs
+++ b/polkadot/node/subsystem-bench/src/lib/approval/mod.rs
@@ -888,7 +888,6 @@ fn prepare_test_inner(
 }
 
 pub async fn bench_approvals(
-	benchmark_name: &str,
 	env: &mut TestEnvironment,
 	mut state: ApprovalTestState,
 ) -> BenchmarkUsage {
@@ -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<()>,
@@ -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"])
 }
diff --git a/polkadot/node/subsystem-bench/src/lib/availability/mod.rs b/polkadot/node/subsystem-bench/src/lib/availability/mod.rs
index 955a8fbac2e..52944ffb08f 100644
--- a/polkadot/node/subsystem-bench/src/lib/availability/mod.rs
+++ b/polkadot/node/subsystem-bench/src/lib/availability/mod.rs
@@ -307,7 +307,6 @@ pub fn prepare_test(
 }
 
 pub async fn benchmark_availability_read(
-	benchmark_name: &str,
 	env: &mut TestEnvironment,
 	state: &TestState,
 ) -> BenchmarkUsage {
@@ -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 {
@@ -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",
+	])
 }
diff --git a/polkadot/node/subsystem-bench/src/lib/environment.rs b/polkadot/node/subsystem-bench/src/lib/environment.rs
index 42955d03022..a63f90da50b 100644
--- a/polkadot/node/subsystem-bench/src/lib/environment.rs
+++ b/polkadot/node/subsystem-bench/src/lib/environment.rs
@@ -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),
 		}
diff --git a/polkadot/node/subsystem-bench/src/lib/statement/mod.rs b/polkadot/node/subsystem-bench/src/lib/statement/mod.rs
index 508dd9179f7..bd47505f56a 100644
--- a/polkadot/node/subsystem-bench/src/lib/statement/mod.rs
+++ b/polkadot/node/subsystem-bench/src/lib/statement/mod.rs
@@ -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 {
@@ -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"])
 }
diff --git a/polkadot/node/subsystem-bench/src/lib/usage.rs b/polkadot/node/subsystem-bench/src/lib/usage.rs
index bfaac3265a2..883e9aa7ad0 100644
--- a/polkadot/node/subsystem-bench/src/lib/usage.rs
+++ b/polkadot/node/subsystem-bench/src/lib/usage.rs
@@ -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>,
 }
@@ -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()
@@ -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> {
@@ -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(
-- 
GitLab