From ada0ee071f63538d6ebdfaee7ebc6950f31bc487 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 27 Sep 2022 17:58:01 +0200 Subject: [PATCH 01/37] move Throughput to sc-sysinfo --- client/sysinfo/src/lib.rs | 2 +- client/sysinfo/src/sysinfo.rs | 74 ++++++++++++++++++- .../benchmarking-cli/src/machine/hardware.rs | 73 +----------------- .../frame/benchmarking-cli/src/machine/mod.rs | 5 +- 4 files changed, 78 insertions(+), 76 deletions(-) diff --git a/client/sysinfo/src/lib.rs b/client/sysinfo/src/lib.rs index be63fefe9ecd1..6c2f2bb8cab9e 100644 --- a/client/sysinfo/src/lib.rs +++ b/client/sysinfo/src/lib.rs @@ -28,7 +28,7 @@ mod sysinfo_linux; pub use sysinfo::{ benchmark_cpu, benchmark_disk_random_writes, benchmark_disk_sequential_writes, - benchmark_memory, benchmark_sr25519_verify, gather_hwbench, gather_sysinfo, + benchmark_memory, benchmark_sr25519_verify, gather_hwbench, gather_sysinfo, Throughput, }; /// The operating system part of the current target triplet. diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index fc347c1cc2eb3..e89944c1cec79 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -21,9 +21,10 @@ use crate::{ExecutionLimit, HwBench}; use sc_telemetry::SysInfo; use sp_core::{sr25519, Pair}; use sp_io::crypto::sr25519_verify; -use sp_std::prelude::*; +use sp_std::{fmt, prelude::*}; use rand::{seq::SliceRandom, Rng, RngCore}; +use serde::{Deserialize, Serialize}; use std::{ fs::File, io::{Seek, SeekFrom, Write}, @@ -32,6 +33,77 @@ use std::{ time::{Duration, Instant}, }; +/// Throughput as measured in bytes per second. +#[derive(Deserialize, Serialize, Debug, Clone, Copy, PartialEq)] +pub enum Throughput { + /// KiB/s + KiBs(f64), + /// MiB/s + MiBs(f64), + /// GiB/s + GiBs(f64), +} + +const KIBIBYTE: f64 = 1024.0; + +impl Throughput { + /// The unit of the metric. + pub fn unit(&self) -> &'static str { + match self { + Self::KiBs(_) => "KiB/s", + Self::MiBs(_) => "MiB/s", + Self::GiBs(_) => "GiB/s", + } + } + + /// [`Self`] as number of byte/s. + pub fn to_bs(&self) -> f64 { + self.to_kibs() * KIBIBYTE + } + + /// [`Self`] as number of kibibyte/s. + pub fn to_kibs(&self) -> f64 { + self.to_mibs() * KIBIBYTE + } + + /// [`Self`] as number of mebibyte/s. + pub fn to_mibs(&self) -> f64 { + self.to_gibs() * KIBIBYTE + } + + /// [`Self`] as number of gibibyte/s. + pub fn to_gibs(&self) -> f64 { + match self { + Self::KiBs(k) => *k / (KIBIBYTE * KIBIBYTE), + Self::MiBs(m) => *m / KIBIBYTE, + Self::GiBs(g) => *g, + } + } + + /// Normalizes [`Self`] to use the larges unit possible. + pub fn normalize(&self) -> Self { + let bs = self.to_bs(); + + if bs >= KIBIBYTE * KIBIBYTE * KIBIBYTE { + Self::GiBs(self.to_gibs()) + } else if bs >= KIBIBYTE * KIBIBYTE { + Self::MiBs(self.to_mibs()) + } else { + Self::KiBs(self.to_kibs()) + } + } +} + +impl fmt::Display for Throughput { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let normalized = self.normalize(); + match normalized { + Self::KiBs(s) | Self::MiBs(s) | Self::GiBs(s) => + write!(f, "{:.2?} {}", s, normalized.unit()), + } + } +} + #[inline(always)] pub(crate) fn benchmark( name: &str, diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 97960de99c4bf..e9d4f32be048c 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -18,8 +18,8 @@ //! Contains types to define hardware requirements. use lazy_static::lazy_static; +use sc_sysinfo::Throughput; use serde::{Deserialize, Serialize}; -use std::fmt; lazy_static! { /// The hardware requirements as measured on reference hardware. @@ -65,17 +65,6 @@ pub enum Metric { DiskRndWrite, } -/// Throughput as measured in bytes per second. -#[derive(Deserialize, Serialize, Debug, Clone, Copy, PartialEq)] -pub enum Throughput { - /// KiB/s - KiBs(f64), - /// MiB/s - MiBs(f64), - /// GiB/s - GiBs(f64), -} - impl Metric { /// The category of the metric. pub fn category(&self) -> &'static str { @@ -98,66 +87,6 @@ impl Metric { } } -const KIBIBYTE: f64 = 1024.0; - -impl Throughput { - /// The unit of the metric. - pub fn unit(&self) -> &'static str { - match self { - Self::KiBs(_) => "KiB/s", - Self::MiBs(_) => "MiB/s", - Self::GiBs(_) => "GiB/s", - } - } - - /// [`Self`] as number of byte/s. - pub fn to_bs(&self) -> f64 { - self.to_kibs() * KIBIBYTE - } - - /// [`Self`] as number of kibibyte/s. - pub fn to_kibs(&self) -> f64 { - self.to_mibs() * KIBIBYTE - } - - /// [`Self`] as number of mebibyte/s. - pub fn to_mibs(&self) -> f64 { - self.to_gibs() * KIBIBYTE - } - - /// [`Self`] as number of gibibyte/s. - pub fn to_gibs(&self) -> f64 { - match self { - Self::KiBs(k) => *k / (KIBIBYTE * KIBIBYTE), - Self::MiBs(m) => *m / KIBIBYTE, - Self::GiBs(g) => *g, - } - } - - /// Normalizes [`Self`] to use the larges unit possible. - pub fn normalize(&self) -> Self { - let bs = self.to_bs(); - - if bs >= KIBIBYTE * KIBIBYTE * KIBIBYTE { - Self::GiBs(self.to_gibs()) - } else if bs >= KIBIBYTE * KIBIBYTE { - Self::MiBs(self.to_mibs()) - } else { - Self::KiBs(self.to_kibs()) - } - } -} - -impl fmt::Display for Throughput { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let normalized = self.normalize(); - match normalized { - Self::KiBs(s) | Self::MiBs(s) | Self::GiBs(s) => - write!(f, "{:.2?} {}", s, normalized.unit()), - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/utils/frame/benchmarking-cli/src/machine/mod.rs b/utils/frame/benchmarking-cli/src/machine/mod.rs index 5f27c71983905..b16cf853a1aca 100644 --- a/utils/frame/benchmarking-cli/src/machine/mod.rs +++ b/utils/frame/benchmarking-cli/src/machine/mod.rs @@ -30,11 +30,11 @@ use sc_cli::{CliConfiguration, Result, SharedParams}; use sc_service::Configuration; use sc_sysinfo::{ benchmark_cpu, benchmark_disk_random_writes, benchmark_disk_sequential_writes, - benchmark_memory, benchmark_sr25519_verify, ExecutionLimit, + benchmark_memory, benchmark_sr25519_verify, ExecutionLimit, Throughput, }; use crate::shared::check_build_profile; -pub use hardware::{Metric, Requirement, Requirements, Throughput, SUBSTRATE_REFERENCE_HARDWARE}; +pub use hardware::{Metric, Requirement, Requirements, SUBSTRATE_REFERENCE_HARDWARE}; /// Command to benchmark the hardware. /// @@ -128,6 +128,7 @@ impl MachineCmd { /// Benchmarks a specific metric of the hardware and judges the resulting score. fn run_benchmark(&self, requirement: &Requirement, dir: &Path) -> Result { // Dispatch the concrete function from `sc-sysinfo`. + let score = self.measure(&requirement.metric, dir)?; let rel_score = score.to_bs() / requirement.minimum.to_bs(); From d25f110ef936a5e4a133f1debe76d1f93fbb97a4 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 27 Sep 2022 21:50:07 +0200 Subject: [PATCH 02/37] replace u64 --- client/sysinfo/src/lib.rs | 8 +++---- client/sysinfo/src/sysinfo.rs | 24 +++++++++++-------- .../frame/benchmarking-cli/src/machine/mod.rs | 4 ++-- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/client/sysinfo/src/lib.rs b/client/sysinfo/src/lib.rs index 6c2f2bb8cab9e..aa0c2960d609b 100644 --- a/client/sysinfo/src/lib.rs +++ b/client/sysinfo/src/lib.rs @@ -44,13 +44,13 @@ pub const TARGET_ENV: &str = include_str!(concat!(env!("OUT_DIR"), "/target_env. #[derive(Clone, Debug, serde::Serialize)] pub struct HwBench { /// The CPU speed, as measured in how many MB/s it can hash using the BLAKE2b-256 hash. - pub cpu_hashrate_score: u64, + pub cpu_hashrate_score: Throughput, /// Memory bandwidth in MB/s, calculated by measuring the throughput of `memcpy`. - pub memory_memcpy_score: u64, + pub memory_memcpy_score: Throughput, /// Sequential disk write speed in MB/s. - pub disk_sequential_write_score: Option, + pub disk_sequential_write_score: Option, /// Random disk write speed in MB/s. - pub disk_random_write_score: Option, + pub disk_random_write_score: Option, } /// Limit the execution time of a benchmark. diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index e89944c1cec79..519b2c27dbfb2 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -193,7 +193,7 @@ pub const DEFAULT_CPU_EXECUTION_LIMIT: ExecutionLimit = ExecutionLimit::Both { max_iterations: 4 * 1024, max_duration: Duration::from_millis(100) }; // This benchmarks the CPU speed as measured by calculating BLAKE2b-256 hashes, in MB/s. -pub fn benchmark_cpu(limit: ExecutionLimit) -> f64 { +pub fn benchmark_cpu(limit: ExecutionLimit) -> Throughput { // In general the results of this benchmark are somewhat sensitive to how much // data we hash at the time. The smaller this is the *less* MB/s we can hash, // the bigger this is the *more* MB/s we can hash, up until a certain point @@ -220,8 +220,10 @@ pub fn benchmark_cpu(limit: ExecutionLimit) -> f64 { Ok(()) }; - benchmark("CPU score", SIZE, limit.max_iterations(), limit.max_duration(), run) - .expect("benchmark cannot fail; qed") + Throughput::MiBs( + benchmark("CPU score", SIZE, limit.max_iterations(), limit.max_duration(), run) + .expect("benchmark cannot fail; qed"), + ) } /// A default [`ExecutionLimit`] that can be used to call [`benchmark_memory`]. @@ -233,7 +235,7 @@ pub const DEFAULT_MEMORY_EXECUTION_LIMIT: ExecutionLimit = // It doesn't technically measure the absolute maximum memory bandwidth available, // but that's fine, because real code most of the time isn't optimized to take // advantage of the full memory bandwidth either. -pub fn benchmark_memory(limit: ExecutionLimit) -> f64 { +pub fn benchmark_memory(limit: ExecutionLimit) -> Throughput { // Ideally this should be at least as big as the CPU's L3 cache, // and it should be big enough so that the `memcpy` takes enough // time to be actually measurable. @@ -268,8 +270,10 @@ pub fn benchmark_memory(limit: ExecutionLimit) -> f64 { Ok(()) }; - benchmark("memory score", SIZE, limit.max_iterations(), limit.max_duration(), run) - .expect("benchmark cannot fail; qed") + Throughput::MiBs( + benchmark("memory score", SIZE, limit.max_iterations(), limit.max_duration(), run) + .expect("benchmark cannot fail; qed"), + ) } struct TemporaryFile { @@ -474,8 +478,8 @@ pub fn benchmark_sr25519_verify(limit: ExecutionLimit) -> f64 { pub fn gather_hwbench(scratch_directory: Option<&Path>) -> HwBench { #[allow(unused_mut)] let mut hwbench = HwBench { - cpu_hashrate_score: benchmark_cpu(DEFAULT_CPU_EXECUTION_LIMIT) as u64, - memory_memcpy_score: benchmark_memory(DEFAULT_MEMORY_EXECUTION_LIMIT) as u64, + cpu_hashrate_score: benchmark_cpu(DEFAULT_CPU_EXECUTION_LIMIT), + memory_memcpy_score: benchmark_memory(DEFAULT_MEMORY_EXECUTION_LIMIT), disk_sequential_write_score: None, disk_random_write_score: None, }; @@ -484,7 +488,7 @@ pub fn gather_hwbench(scratch_directory: Option<&Path>) -> HwBench { hwbench.disk_sequential_write_score = match benchmark_disk_sequential_writes(DEFAULT_DISK_EXECUTION_LIMIT, scratch_directory) { - Ok(score) => Some(score as u64), + Ok(score) => Some(Throughput::MiBs(score)), Err(error) => { log::warn!("Failed to run the sequential write disk benchmark: {}", error); None @@ -493,7 +497,7 @@ pub fn gather_hwbench(scratch_directory: Option<&Path>) -> HwBench { hwbench.disk_random_write_score = match benchmark_disk_random_writes(DEFAULT_DISK_EXECUTION_LIMIT, scratch_directory) { - Ok(score) => Some(score as u64), + Ok(score) => Some(Throughput::MiBs(score)), Err(error) => { log::warn!("Failed to run the random write disk benchmark: {}", error); None diff --git a/utils/frame/benchmarking-cli/src/machine/mod.rs b/utils/frame/benchmarking-cli/src/machine/mod.rs index b16cf853a1aca..84f52ebd92b74 100644 --- a/utils/frame/benchmarking-cli/src/machine/mod.rs +++ b/utils/frame/benchmarking-cli/src/machine/mod.rs @@ -148,9 +148,9 @@ impl MachineCmd { let memory_limit = ExecutionLimit::from_secs_f32(self.memory_duration); let score = match metric { - Metric::Blake2256 => Throughput::MiBs(benchmark_cpu(hash_limit) as f64), + Metric::Blake2256 => benchmark_cpu(hash_limit), Metric::Sr25519Verify => Throughput::MiBs(benchmark_sr25519_verify(verify_limit)), - Metric::MemCopy => Throughput::MiBs(benchmark_memory(memory_limit) as f64), + Metric::MemCopy => benchmark_memory(memory_limit), Metric::DiskSeqWrite => Throughput::MiBs(benchmark_disk_sequential_writes(disk_limit, dir)? as f64), Metric::DiskRndWrite => From a564d4f3144e1078dbd3aa573571be40cf2d1e47 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 29 Sep 2022 07:33:23 +0200 Subject: [PATCH 03/37] fix in tests --- client/sysinfo/src/sysinfo.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 519b2c27dbfb2..24c3910efed15 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -526,12 +526,12 @@ mod tests { #[test] fn test_benchmark_cpu() { - assert!(benchmark_cpu(DEFAULT_CPU_EXECUTION_LIMIT) > 0.0); + assert!(benchmark_cpu(DEFAULT_CPU_EXECUTION_LIMIT).to_bs() > 0.0); } #[test] fn test_benchmark_memory() { - assert!(benchmark_memory(DEFAULT_MEMORY_EXECUTION_LIMIT) > 0.0); + assert!(benchmark_memory(DEFAULT_MEMORY_EXECUTION_LIMIT).to_bs() > 0.0); } #[test] From 7c30f8d163fbf6c2d5b2cf09d5d2c03ebae7c75c Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 29 Sep 2022 17:20:28 +0200 Subject: [PATCH 04/37] change Throughput --- client/sysinfo/src/sysinfo.rs | 71 ++++++++++++++--------------------- 1 file changed, 29 insertions(+), 42 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 24c3910efed15..2acb1f907302d 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -35,72 +35,59 @@ use std::{ /// Throughput as measured in bytes per second. #[derive(Deserialize, Serialize, Debug, Clone, Copy, PartialEq)] -pub enum Throughput { - /// KiB/s - KiBs(f64), - /// MiB/s - MiBs(f64), - /// GiB/s - GiBs(f64), -} +pub struct Throughput(f64); const KIBIBYTE: f64 = 1024.0; impl Throughput { - /// The unit of the metric. - pub fn unit(&self) -> &'static str { - match self { - Self::KiBs(_) => "KiB/s", - Self::MiBs(_) => "MiB/s", - Self::GiBs(_) => "GiB/s", - } + /// `f64` kibibyte/s to byte/s. + pub fn from_kibs(mibs: f64) -> Throughput { + Throughput(mibs * KIBIBYTE) + } + + /// `f64` mebibyte/s to byte/s. + pub fn from_mibs(mibs: f64) -> Throughput { + Throughput(mibs * KIBIBYTE * KIBIBYTE) } - /// [`Self`] as number of byte/s. - pub fn to_bs(&self) -> f64 { - self.to_kibs() * KIBIBYTE + /// `f64` gibibyte/s to byte/s. + pub fn from_gibs(mibs: f64) -> Throughput { + Throughput(mibs * KIBIBYTE * KIBIBYTE * KIBIBYTE) } /// [`Self`] as number of kibibyte/s. - pub fn to_kibs(&self) -> f64 { - self.to_mibs() * KIBIBYTE + fn to_kibs(&self) -> Throughput { + Throughput(self.0 / KIBIBYTE) } /// [`Self`] as number of mebibyte/s. - pub fn to_mibs(&self) -> f64 { - self.to_gibs() * KIBIBYTE + fn to_mibs(&self) -> Throughput { + Throughput(self.0 / (KIBIBYTE * KIBIBYTE)) } /// [`Self`] as number of gibibyte/s. - pub fn to_gibs(&self) -> f64 { - match self { - Self::KiBs(k) => *k / (KIBIBYTE * KIBIBYTE), - Self::MiBs(m) => *m / KIBIBYTE, - Self::GiBs(g) => *g, - } + fn to_gibs(&self) -> Throughput { + Throughput(self.0 / (KIBIBYTE * KIBIBYTE * KIBIBYTE)) } /// Normalizes [`Self`] to use the larges unit possible. - pub fn normalize(&self) -> Self { - let bs = self.to_bs(); + pub fn normalize(&self) -> (Throughput, &'static str) { + let bs = self.0; if bs >= KIBIBYTE * KIBIBYTE * KIBIBYTE { - Self::GiBs(self.to_gibs()) + (self.to_gibs(), "GiB/s") } else if bs >= KIBIBYTE * KIBIBYTE { - Self::MiBs(self.to_mibs()) + (self.to_mibs(), "MiB/s") } else { - Self::KiBs(self.to_kibs()) + (self.to_kibs(), "KiB/s") } } } impl fmt::Display for Throughput { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let normalized = self.normalize(); - match normalized { - Self::KiBs(s) | Self::MiBs(s) | Self::GiBs(s) => - write!(f, "{:.2?} {}", s, normalized.unit()), - } + let (value, unit) = self.normalize(); + write!(f, "{:.2?} {}", value, unit) } } @@ -220,7 +207,7 @@ pub fn benchmark_cpu(limit: ExecutionLimit) -> Throughput { Ok(()) }; - Throughput::MiBs( + Throughput::from_mibs( benchmark("CPU score", SIZE, limit.max_iterations(), limit.max_duration(), run) .expect("benchmark cannot fail; qed"), ) @@ -270,7 +257,7 @@ pub fn benchmark_memory(limit: ExecutionLimit) -> Throughput { Ok(()) }; - Throughput::MiBs( + Throughput::from_mibs( benchmark("memory score", SIZE, limit.max_iterations(), limit.max_duration(), run) .expect("benchmark cannot fail; qed"), ) @@ -488,7 +475,7 @@ pub fn gather_hwbench(scratch_directory: Option<&Path>) -> HwBench { hwbench.disk_sequential_write_score = match benchmark_disk_sequential_writes(DEFAULT_DISK_EXECUTION_LIMIT, scratch_directory) { - Ok(score) => Some(Throughput::MiBs(score)), + Ok(score) => Some(Throughput::from_mibs(score)), Err(error) => { log::warn!("Failed to run the sequential write disk benchmark: {}", error); None @@ -497,7 +484,7 @@ pub fn gather_hwbench(scratch_directory: Option<&Path>) -> HwBench { hwbench.disk_random_write_score = match benchmark_disk_random_writes(DEFAULT_DISK_EXECUTION_LIMIT, scratch_directory) { - Ok(score) => Some(Throughput::MiBs(score)), + Ok(score) => Some(Throughput::from_mibs(score)), Err(error) => { log::warn!("Failed to run the random write disk benchmark: {}", error); None From d276670f5cd91d911ccb8551ac005466ffdde52d Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 29 Sep 2022 19:20:16 +0200 Subject: [PATCH 05/37] refactored Throughput --- client/sysinfo/src/sysinfo.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 2acb1f907302d..55cfd4383240b 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -56,30 +56,30 @@ impl Throughput { } /// [`Self`] as number of kibibyte/s. - fn to_kibs(&self) -> Throughput { - Throughput(self.0 / KIBIBYTE) + fn as_kibs(&self) -> f64 { + self.0 / KIBIBYTE } /// [`Self`] as number of mebibyte/s. - fn to_mibs(&self) -> Throughput { - Throughput(self.0 / (KIBIBYTE * KIBIBYTE)) + fn as_mibs(&self) -> f64 { + self.0 / (KIBIBYTE * KIBIBYTE) } /// [`Self`] as number of gibibyte/s. - fn to_gibs(&self) -> Throughput { - Throughput(self.0 / (KIBIBYTE * KIBIBYTE * KIBIBYTE)) + fn as_gibs(&self) -> f64 { + self.0 / (KIBIBYTE * KIBIBYTE * KIBIBYTE) } /// Normalizes [`Self`] to use the larges unit possible. - pub fn normalize(&self) -> (Throughput, &'static str) { + pub fn normalize(&self) -> (f64, &'static str) { let bs = self.0; if bs >= KIBIBYTE * KIBIBYTE * KIBIBYTE { - (self.to_gibs(), "GiB/s") + (self.as_gibs(), "GiB/s") } else if bs >= KIBIBYTE * KIBIBYTE { - (self.to_mibs(), "MiB/s") + (self.as_mibs(), "MiB/s") } else { - (self.to_kibs(), "KiB/s") + (self.as_kibs(), "KiB/s") } } } @@ -513,12 +513,12 @@ mod tests { #[test] fn test_benchmark_cpu() { - assert!(benchmark_cpu(DEFAULT_CPU_EXECUTION_LIMIT).to_bs() > 0.0); + assert!(benchmark_cpu(DEFAULT_CPU_EXECUTION_LIMIT).as_bs() > 0.0); } #[test] fn test_benchmark_memory() { - assert!(benchmark_memory(DEFAULT_MEMORY_EXECUTION_LIMIT).to_bs() > 0.0); + assert!(benchmark_memory(DEFAULT_MEMORY_EXECUTION_LIMIT).as_bs() > 0.0); } #[test] From 96ec3f0c34777796c62578fc3a80865136d57d1c Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 30 Sep 2022 07:55:29 +0200 Subject: [PATCH 06/37] fixes --- client/sysinfo/src/sysinfo.rs | 60 +++++++++---------- .../benchmarking-cli/src/machine/hardware.rs | 12 ++-- .../frame/benchmarking-cli/src/machine/mod.rs | 8 +-- 3 files changed, 40 insertions(+), 40 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 55cfd4383240b..88c1bc7a0da98 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -34,15 +34,15 @@ use std::{ }; /// Throughput as measured in bytes per second. -#[derive(Deserialize, Serialize, Debug, Clone, Copy, PartialEq)] +#[derive(Deserialize, Serialize, Debug, Clone, Copy, PartialEq, PartialOrd)] pub struct Throughput(f64); const KIBIBYTE: f64 = 1024.0; impl Throughput { /// `f64` kibibyte/s to byte/s. - pub fn from_kibs(mibs: f64) -> Throughput { - Throughput(mibs * KIBIBYTE) + pub fn from_kibs(kibs: f64) -> Throughput { + Throughput(kibs * KIBIBYTE) } /// `f64` mebibyte/s to byte/s. @@ -51,22 +51,26 @@ impl Throughput { } /// `f64` gibibyte/s to byte/s. - pub fn from_gibs(mibs: f64) -> Throughput { - Throughput(mibs * KIBIBYTE * KIBIBYTE * KIBIBYTE) + pub fn from_gibs(gibs: f64) -> Throughput { + Throughput(gibs * KIBIBYTE * KIBIBYTE * KIBIBYTE) + } + + pub fn as_f64(&self) -> f64 { + self.0 } /// [`Self`] as number of kibibyte/s. - fn as_kibs(&self) -> f64 { + pub fn as_kibs(&self) -> f64 { self.0 / KIBIBYTE } /// [`Self`] as number of mebibyte/s. - fn as_mibs(&self) -> f64 { + pub fn as_mibs(&self) -> f64 { self.0 / (KIBIBYTE * KIBIBYTE) } /// [`Self`] as number of gibibyte/s. - fn as_gibs(&self) -> f64 { + pub fn as_gibs(&self) -> f64 { self.0 / (KIBIBYTE * KIBIBYTE * KIBIBYTE) } @@ -98,7 +102,7 @@ pub(crate) fn benchmark( max_iterations: usize, max_duration: Duration, mut run: impl FnMut() -> Result<(), E>, -) -> Result { +) -> Result { // Run the benchmark once as a warmup to get the code into the L1 cache. run()?; @@ -117,9 +121,9 @@ pub(crate) fn benchmark( } } - let score = ((size * count) as f64 / elapsed.as_secs_f64()) / (1024.0 * 1024.0); + let score = Throughput((size * count) as f64 / elapsed.as_secs_f64()); log::trace!( - "Calculated {} of {:.2}MB/s in {} iterations in {}ms", + "Calculated {} of {:.2}B/s in {} iterations in {}ms", name, score, count, @@ -207,10 +211,8 @@ pub fn benchmark_cpu(limit: ExecutionLimit) -> Throughput { Ok(()) }; - Throughput::from_mibs( - benchmark("CPU score", SIZE, limit.max_iterations(), limit.max_duration(), run) - .expect("benchmark cannot fail; qed"), - ) + benchmark("CPU score", SIZE, limit.max_iterations(), limit.max_duration(), run) + .expect("benchmark cannot fail; qed") } /// A default [`ExecutionLimit`] that can be used to call [`benchmark_memory`]. @@ -257,10 +259,8 @@ pub fn benchmark_memory(limit: ExecutionLimit) -> Throughput { Ok(()) }; - Throughput::from_mibs( - benchmark("memory score", SIZE, limit.max_iterations(), limit.max_duration(), run) - .expect("benchmark cannot fail; qed"), - ) + benchmark("memory score", SIZE, limit.max_iterations(), limit.max_duration(), run) + .expect("benchmark cannot fail; qed") } struct TemporaryFile { @@ -316,7 +316,7 @@ pub const DEFAULT_DISK_EXECUTION_LIMIT: ExecutionLimit = pub fn benchmark_disk_sequential_writes( limit: ExecutionLimit, directory: &Path, -) -> Result { +) -> Result { const SIZE: usize = 64 * 1024 * 1024; let buffer = random_data(SIZE); @@ -358,7 +358,7 @@ pub fn benchmark_disk_sequential_writes( pub fn benchmark_disk_random_writes( limit: ExecutionLimit, directory: &Path, -) -> Result { +) -> Result { const SIZE: usize = 64 * 1024 * 1024; let buffer = random_data(SIZE); @@ -423,9 +423,9 @@ pub fn benchmark_disk_random_writes( /// Benchmarks the verification speed of sr25519 signatures. /// -/// Returns the throughput in MB/s by convention. +/// Returns the throughput in B/s by convention. /// The values are rather small (0.4-0.8) so it is advised to convert them into KB/s. -pub fn benchmark_sr25519_verify(limit: ExecutionLimit) -> f64 { +pub fn benchmark_sr25519_verify(limit: ExecutionLimit) -> Throughput { const INPUT_SIZE: usize = 32; const ITERATION_SIZE: usize = 2048; let pair = sr25519::Pair::from_string("//Alice", None).unwrap(); @@ -475,7 +475,7 @@ pub fn gather_hwbench(scratch_directory: Option<&Path>) -> HwBench { hwbench.disk_sequential_write_score = match benchmark_disk_sequential_writes(DEFAULT_DISK_EXECUTION_LIMIT, scratch_directory) { - Ok(score) => Some(Throughput::from_mibs(score)), + Ok(score) => Some(score), Err(error) => { log::warn!("Failed to run the sequential write disk benchmark: {}", error); None @@ -484,7 +484,7 @@ pub fn gather_hwbench(scratch_directory: Option<&Path>) -> HwBench { hwbench.disk_random_write_score = match benchmark_disk_random_writes(DEFAULT_DISK_EXECUTION_LIMIT, scratch_directory) { - Ok(score) => Some(Throughput::from_mibs(score)), + Ok(score) => Some(score), Err(error) => { log::warn!("Failed to run the random write disk benchmark: {}", error); None @@ -513,19 +513,19 @@ mod tests { #[test] fn test_benchmark_cpu() { - assert!(benchmark_cpu(DEFAULT_CPU_EXECUTION_LIMIT).as_bs() > 0.0); + assert!(benchmark_cpu(DEFAULT_CPU_EXECUTION_LIMIT) > Throughput(0.0)); } #[test] fn test_benchmark_memory() { - assert!(benchmark_memory(DEFAULT_MEMORY_EXECUTION_LIMIT).as_bs() > 0.0); + assert!(benchmark_memory(DEFAULT_MEMORY_EXECUTION_LIMIT) > Throughput(0.0)); } #[test] fn test_benchmark_disk_sequential_writes() { assert!( benchmark_disk_sequential_writes(DEFAULT_DISK_EXECUTION_LIMIT, "./".as_ref()).unwrap() > - 0.0 + Throughput(0.0) ); } @@ -533,12 +533,12 @@ mod tests { fn test_benchmark_disk_random_writes() { assert!( benchmark_disk_random_writes(DEFAULT_DISK_EXECUTION_LIMIT, "./".as_ref()).unwrap() > - 0.0 + Throughput(0.0) ); } #[test] fn test_benchmark_sr25519_verify() { - assert!(benchmark_sr25519_verify(ExecutionLimit::MaxIterations(1)) > 0.0); + assert!(benchmark_sr25519_verify(ExecutionLimit::MaxIterations(1)) > Throughput(0.0)); } } diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index e9d4f32be048c..5e21b0e0b6a3f 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -106,15 +106,15 @@ mod tests { fn throughput_works() { /// Float precision. const EPS: f64 = 0.1; - let gib = Throughput::GiBs(14.324); + let gib = Throughput::from_gibs(14.324); - assert_eq_error_rate_float!(14.324, gib.to_gibs(), EPS); - assert_eq_error_rate_float!(14667.776, gib.to_mibs(), EPS); - assert_eq_error_rate_float!(14667.776 * 1024.0, gib.to_kibs(), EPS); + assert_eq_error_rate_float!(14.324, gib.as_gibs(), EPS); + assert_eq_error_rate_float!(14667.776, gib.as_mibs(), EPS); + assert_eq_error_rate_float!(14667.776 * 1024.0, gib.as_kibs(), EPS); assert_eq!("14.32 GiB/s", gib.to_string()); - assert_eq!("14.32 GiB/s", gib.normalize().to_string()); + assert_eq!("14.32 GiB/s", gib.normalize().0.to_string()); - let mib = Throughput::MiBs(1029.0); + let mib = Throughput::from_mibs(1029.0); assert_eq!("1.00 GiB/s", mib.to_string()); } } diff --git a/utils/frame/benchmarking-cli/src/machine/mod.rs b/utils/frame/benchmarking-cli/src/machine/mod.rs index 84f52ebd92b74..56e9caa69c9e5 100644 --- a/utils/frame/benchmarking-cli/src/machine/mod.rs +++ b/utils/frame/benchmarking-cli/src/machine/mod.rs @@ -130,7 +130,7 @@ impl MachineCmd { // Dispatch the concrete function from `sc-sysinfo`. let score = self.measure(&requirement.metric, dir)?; - let rel_score = score.to_bs() / requirement.minimum.to_bs(); + let rel_score = score.as_f64() / requirement.minimum.as_f64(); // Sanity check if the result is off by factor >100x. if rel_score >= 100.0 || rel_score <= 0.01 { @@ -149,12 +149,12 @@ impl MachineCmd { let score = match metric { Metric::Blake2256 => benchmark_cpu(hash_limit), - Metric::Sr25519Verify => Throughput::MiBs(benchmark_sr25519_verify(verify_limit)), + Metric::Sr25519Verify => benchmark_sr25519_verify(verify_limit), Metric::MemCopy => benchmark_memory(memory_limit), Metric::DiskSeqWrite => - Throughput::MiBs(benchmark_disk_sequential_writes(disk_limit, dir)? as f64), + benchmark_disk_sequential_writes(disk_limit, dir)?, Metric::DiskRndWrite => - Throughput::MiBs(benchmark_disk_random_writes(disk_limit, dir)? as f64), + benchmark_disk_random_writes(disk_limit, dir)?, }; Ok(score) } From 7226a57445849f12faf46a8ccb37c382fbcfd388 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 30 Sep 2022 14:56:08 +0200 Subject: [PATCH 07/37] moved tests & fixes --- Cargo.lock | 1 + client/sysinfo/Cargo.toml | 1 + client/sysinfo/src/sysinfo.rs | 31 ++++++++++++++----- .../benchmarking-cli/src/machine/hardware.rs | 18 ----------- .../frame/benchmarking-cli/src/machine/mod.rs | 6 ++-- 5 files changed, 28 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index de50d4ec27105..bdf811a0617a8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9019,6 +9019,7 @@ dependencies = [ "serde_json", "sp-core", "sp-io", + "sp-runtime", "sp-std", ] diff --git a/client/sysinfo/Cargo.toml b/client/sysinfo/Cargo.toml index 1e96f69a92dfe..6917b45eb9799 100644 --- a/client/sysinfo/Cargo.toml +++ b/client/sysinfo/Cargo.toml @@ -26,3 +26,4 @@ sc-telemetry = { version = "4.0.0-dev", path = "../telemetry" } sp-core = { version = "6.0.0", path = "../../primitives/core" } sp-io = { version = "6.0.0", path = "../../primitives/io" } sp-std = { version = "4.0.0", path = "../../primitives/std" } +sp-runtime = { version = "6.0.0", path = "../../primitives/runtime" } diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 88c1bc7a0da98..f42fb84403116 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -75,7 +75,7 @@ impl Throughput { } /// Normalizes [`Self`] to use the larges unit possible. - pub fn normalize(&self) -> (f64, &'static str) { + fn normalize(&self) -> (f64, &'static str) { let bs = self.0; if bs >= KIBIBYTE * KIBIBYTE * KIBIBYTE { @@ -123,7 +123,7 @@ pub(crate) fn benchmark( let score = Throughput((size * count) as f64 / elapsed.as_secs_f64()); log::trace!( - "Calculated {} of {:.2}B/s in {} iterations in {}ms", + "Calculated {} of {:.2} in {} iterations in {}ms", name, score, count, @@ -183,14 +183,14 @@ fn clobber_value(input: &mut T) { pub const DEFAULT_CPU_EXECUTION_LIMIT: ExecutionLimit = ExecutionLimit::Both { max_iterations: 4 * 1024, max_duration: Duration::from_millis(100) }; -// This benchmarks the CPU speed as measured by calculating BLAKE2b-256 hashes, in MB/s. +// This benchmarks the CPU speed as measured by calculating BLAKE2b-256 hashes, in bytes per second. pub fn benchmark_cpu(limit: ExecutionLimit) -> Throughput { // In general the results of this benchmark are somewhat sensitive to how much - // data we hash at the time. The smaller this is the *less* MB/s we can hash, - // the bigger this is the *more* MB/s we can hash, up until a certain point + // data we hash at the time. The smaller this is the *less* B/s we can hash, + // the bigger this is the *more* B/s we can hash, up until a certain point // where we can achieve roughly ~100% of what the hasher can do. If we'd plot // this on a graph with the number of bytes we want to hash on the X axis - // and the speed in MB/s on the Y axis then we'd essentially see it grow + // and the speed in B/s on the Y axis then we'd essentially see it grow // logarithmically. // // In practice however we might not always have enough data to hit the maximum @@ -219,7 +219,7 @@ pub fn benchmark_cpu(limit: ExecutionLimit) -> Throughput { pub const DEFAULT_MEMORY_EXECUTION_LIMIT: ExecutionLimit = ExecutionLimit::Both { max_iterations: 32, max_duration: Duration::from_millis(100) }; -// This benchmarks the effective `memcpy` memory bandwidth available in MB/s. +// This benchmarks the effective `memcpy` memory bandwidth available in bytes per second. // // It doesn't technically measure the absolute maximum memory bandwidth available, // but that's fine, because real code most of the time isn't optimized to take @@ -498,6 +498,7 @@ pub fn gather_hwbench(scratch_directory: Option<&Path>) -> HwBench { #[cfg(test)] mod tests { use super::*; + use sp_runtime::assert_eq_error_rate_float; #[cfg(target_os = "linux")] #[test] @@ -541,4 +542,20 @@ mod tests { fn test_benchmark_sr25519_verify() { assert!(benchmark_sr25519_verify(ExecutionLimit::MaxIterations(1)) > Throughput(0.0)); } + + /// Test the [`Throughput`]. + #[test] + fn throughput_works() { + /// Float precision. + const EPS: f64 = 0.1; + let gib = Throughput::from_gibs(14.324); + + assert_eq_error_rate_float!(14.324, gib.as_gibs(), EPS); + assert_eq_error_rate_float!(14667.776, gib.as_mibs(), EPS); + assert_eq_error_rate_float!(14667.776 * 1024.0, gib.as_kibs(), EPS); + assert_eq!("14.32 GiB/s", gib.to_string()); + + let mib = Throughput::from_mibs(1029.0); + assert_eq!("1.00 GiB/s", mib.to_string()); + } } diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 5e21b0e0b6a3f..298e95f226a0a 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -90,7 +90,6 @@ impl Metric { #[cfg(test)] mod tests { use super::*; - use sp_runtime::assert_eq_error_rate_float; /// `SUBSTRATE_REFERENCE_HARDWARE` can be en- and decoded. #[test] @@ -100,21 +99,4 @@ mod tests { assert_eq!(decoded, SUBSTRATE_REFERENCE_HARDWARE.clone()); } - - /// Test the [`Throughput`]. - #[test] - fn throughput_works() { - /// Float precision. - const EPS: f64 = 0.1; - let gib = Throughput::from_gibs(14.324); - - assert_eq_error_rate_float!(14.324, gib.as_gibs(), EPS); - assert_eq_error_rate_float!(14667.776, gib.as_mibs(), EPS); - assert_eq_error_rate_float!(14667.776 * 1024.0, gib.as_kibs(), EPS); - assert_eq!("14.32 GiB/s", gib.to_string()); - assert_eq!("14.32 GiB/s", gib.normalize().0.to_string()); - - let mib = Throughput::from_mibs(1029.0); - assert_eq!("1.00 GiB/s", mib.to_string()); - } } diff --git a/utils/frame/benchmarking-cli/src/machine/mod.rs b/utils/frame/benchmarking-cli/src/machine/mod.rs index 56e9caa69c9e5..9604349513d9a 100644 --- a/utils/frame/benchmarking-cli/src/machine/mod.rs +++ b/utils/frame/benchmarking-cli/src/machine/mod.rs @@ -151,10 +151,8 @@ impl MachineCmd { Metric::Blake2256 => benchmark_cpu(hash_limit), Metric::Sr25519Verify => benchmark_sr25519_verify(verify_limit), Metric::MemCopy => benchmark_memory(memory_limit), - Metric::DiskSeqWrite => - benchmark_disk_sequential_writes(disk_limit, dir)?, - Metric::DiskRndWrite => - benchmark_disk_random_writes(disk_limit, dir)?, + Metric::DiskSeqWrite => benchmark_disk_sequential_writes(disk_limit, dir)?, + Metric::DiskRndWrite => benchmark_disk_random_writes(disk_limit, dir)?, }; Ok(score) } From f5530ace3e2521e4b9a89c9e05119adc40054c50 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 30 Sep 2022 17:32:20 +0200 Subject: [PATCH 08/37] custom serializer --- client/sysinfo/src/sysinfo.rs | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index f42fb84403116..b12361ecdded8 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -24,7 +24,7 @@ use sp_io::crypto::sr25519_verify; use sp_std::{fmt, prelude::*}; use rand::{seq::SliceRandom, Rng, RngCore}; -use serde::{Deserialize, Serialize}; +use serde::{ser::SerializeTuple, Deserialize, Serialize, Serializer}; use std::{ fs::File, io::{Seek, SeekFrom, Write}, @@ -34,7 +34,7 @@ use std::{ }; /// Throughput as measured in bytes per second. -#[derive(Deserialize, Serialize, Debug, Clone, Copy, PartialEq, PartialOrd)] +#[derive(Deserialize, Debug, Clone, Copy, PartialEq, PartialOrd)] pub struct Throughput(f64); const KIBIBYTE: f64 = 1024.0; @@ -79,11 +79,11 @@ impl Throughput { let bs = self.0; if bs >= KIBIBYTE * KIBIBYTE * KIBIBYTE { - (self.as_gibs(), "GiB/s") + (self.as_gibs(), "GiBs") } else if bs >= KIBIBYTE * KIBIBYTE { - (self.as_mibs(), "MiB/s") + (self.as_mibs(), "MiBs") } else { - (self.as_kibs(), "KiB/s") + (self.as_kibs(), "KiBs") } } } @@ -95,6 +95,18 @@ impl fmt::Display for Throughput { } } +impl Serialize for Throughput { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let mut state = serializer.serialize_tuple(4)?; + let (value, unit) = self.normalize(); + state.serialize_element(&(unit, value))?; + state.end() + } +} + #[inline(always)] pub(crate) fn benchmark( name: &str, @@ -553,9 +565,9 @@ mod tests { assert_eq_error_rate_float!(14.324, gib.as_gibs(), EPS); assert_eq_error_rate_float!(14667.776, gib.as_mibs(), EPS); assert_eq_error_rate_float!(14667.776 * 1024.0, gib.as_kibs(), EPS); - assert_eq!("14.32 GiB/s", gib.to_string()); + assert_eq!("14.32 GiBs", gib.to_string()); let mib = Throughput::from_mibs(1029.0); - assert_eq!("1.00 GiB/s", mib.to_string()); + assert_eq!("1.00 GiBs", mib.to_string()); } } From 99b25a7efda775437bfe3114cd7ab1fe358be57c Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 30 Sep 2022 17:41:25 +0200 Subject: [PATCH 09/37] note --- client/sysinfo/src/sysinfo.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index b12361ecdded8..89ed356bddd5c 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -100,6 +100,7 @@ impl Serialize for Throughput { where S: Serializer, { + // NOTE I will replace 4 with the actual length. let mut state = serializer.serialize_tuple(4)?; let (value, unit) = self.normalize(); state.serialize_element(&(unit, value))?; From cdd84d230f62424f9630a6630537e4696aa0db36 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 1 Oct 2022 08:40:12 +0200 Subject: [PATCH 10/37] fix serializer --- client/sysinfo/src/sysinfo.rs | 8 ++++---- utils/frame/benchmarking-cli/src/machine/hardware.rs | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 89ed356bddd5c..a712ff5a85f4c 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -24,7 +24,7 @@ use sp_io::crypto::sr25519_verify; use sp_std::{fmt, prelude::*}; use rand::{seq::SliceRandom, Rng, RngCore}; -use serde::{ser::SerializeTuple, Deserialize, Serialize, Serializer}; +use serde::{ser::SerializeMap, Deserialize, Serialize, Serializer}; use std::{ fs::File, io::{Seek, SeekFrom, Write}, @@ -101,10 +101,10 @@ impl Serialize for Throughput { S: Serializer, { // NOTE I will replace 4 with the actual length. - let mut state = serializer.serialize_tuple(4)?; + let mut seq = serializer.serialize_map(Some(4))?; let (value, unit) = self.normalize(); - state.serialize_element(&(unit, value))?; - state.end() + seq.serialize_entry(unit, &value)?; + seq.end() } } diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 298e95f226a0a..4970440f2f69d 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -30,6 +30,8 @@ lazy_static! { /// The reference hardware is describe here: /// pub static ref SUBSTRATE_REFERENCE_HARDWARE: Requirements = { + let res = serde_json::to_string(&Throughput::from_kibs(3 as f64)).unwrap(); + println!("REZULTAT: {}", res); let raw = include_bytes!("reference_hardware.json").as_slice(); serde_json::from_slice(raw).expect("Hardcoded data is known good; qed") }; From 9092ba0d0f77ffef35a07bd21561674b3c07b27e Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 1 Oct 2022 12:29:30 +0200 Subject: [PATCH 11/37] forgot to remove --- client/sysinfo/src/sysinfo.rs | 15 +++++++++++++-- .../benchmarking-cli/src/machine/hardware.rs | 2 -- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index a712ff5a85f4c..e0db61dc3c0f1 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -24,7 +24,7 @@ use sp_io::crypto::sr25519_verify; use sp_std::{fmt, prelude::*}; use rand::{seq::SliceRandom, Rng, RngCore}; -use serde::{ser::SerializeMap, Deserialize, Serialize, Serializer}; +use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer}; use std::{ fs::File, io::{Seek, SeekFrom, Write}, @@ -34,7 +34,7 @@ use std::{ }; /// Throughput as measured in bytes per second. -#[derive(Deserialize, Debug, Clone, Copy, PartialEq, PartialOrd)] +#[derive(Debug, Clone, Copy, PartialEq, PartialOrd)] pub struct Throughput(f64); const KIBIBYTE: f64 = 1024.0; @@ -108,6 +108,17 @@ impl Serialize for Throughput { } } +impl<'de> Deserialize<'de> for Throughput { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + //let s: &str = serde::de::Deserialize::deserialize(deserializer)?; + //println!("ALLLOO: {}", s); + Ok(Throughput(1 as f64)) + } +} + #[inline(always)] pub(crate) fn benchmark( name: &str, diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 4970440f2f69d..298e95f226a0a 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -30,8 +30,6 @@ lazy_static! { /// The reference hardware is describe here: /// pub static ref SUBSTRATE_REFERENCE_HARDWARE: Requirements = { - let res = serde_json::to_string(&Throughput::from_kibs(3 as f64)).unwrap(); - println!("REZULTAT: {}", res); let raw = include_bytes!("reference_hardware.json").as_slice(); serde_json::from_slice(raw).expect("Hardcoded data is known good; qed") }; From 621b56c9863ad70508510c7a814c4d6f341ef893 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 1 Oct 2022 12:34:47 +0200 Subject: [PATCH 12/37] deserialize --- client/sysinfo/src/sysinfo.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index e0db61dc3c0f1..a87be56a55dfd 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -113,8 +113,6 @@ impl<'de> Deserialize<'de> for Throughput { where D: Deserializer<'de>, { - //let s: &str = serde::de::Deserialize::deserialize(deserializer)?; - //println!("ALLLOO: {}", s); Ok(Throughput(1 as f64)) } } From 5f256b0929557905044fc6d5df918a041882381b Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 1 Oct 2022 13:18:53 +0200 Subject: [PATCH 13/37] functioning deserialization :) --- client/sysinfo/src/sysinfo.rs | 51 +++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index a87be56a55dfd..e642884f3a2b6 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -21,10 +21,14 @@ use crate::{ExecutionLimit, HwBench}; use sc_telemetry::SysInfo; use sp_core::{sr25519, Pair}; use sp_io::crypto::sr25519_verify; -use sp_std::{fmt, prelude::*}; +use sp_std::{fmt, fmt::Formatter, prelude::*}; use rand::{seq::SliceRandom, Rng, RngCore}; -use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer}; +use serde::{ + de::{MapAccess, Visitor}, + ser::SerializeMap, + Deserialize, Deserializer, Serialize, Serializer, +}; use std::{ fs::File, io::{Seek, SeekFrom, Write}, @@ -101,20 +105,45 @@ impl Serialize for Throughput { S: Serializer, { // NOTE I will replace 4 with the actual length. - let mut seq = serializer.serialize_map(Some(4))?; + let mut map = serializer.serialize_map(Some(4))?; let (value, unit) = self.normalize(); - seq.serialize_entry(unit, &value)?; - seq.end() + map.serialize_entry(unit, &value)?; + map.end() + } +} + +struct ThroughputVisitor; + +impl<'de> Visitor<'de> for ThroughputVisitor { + type Value = Throughput; + + fn expecting(&self, formatter: &mut Formatter) -> fmt::Result { + formatter.write_str("A very usefull message") + } + + fn visit_map(self, mut access: M) -> Result + where + M: MapAccess<'de>, + { + let (key, value): (&str, f64) = + if let Some((key, value)) = access.next_entry()? { (key, value) } else { todo!() }; + if key == "KiBs" { + return Ok(Throughput(value * KIBIBYTE)) + } else if key == "MiBs" { + return Ok(Throughput(value * KIBIBYTE * KIBIBYTE)) + } else { + return Ok(Throughput(value * KIBIBYTE * KIBIBYTE * KIBIBYTE)) + } } } impl<'de> Deserialize<'de> for Throughput { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - Ok(Throughput(1 as f64)) - } + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + Ok(deserializer.deserialize_map(ThroughputVisitor)?) + } } #[inline(always)] From 57315069ede12998d77667a54ad6617127641642 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 1 Oct 2022 13:25:59 +0200 Subject: [PATCH 14/37] try to make clipply happy --- client/sysinfo/src/sysinfo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index e642884f3a2b6..fecdc2c949a90 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -142,7 +142,7 @@ impl<'de> Deserialize<'de> for Throughput { where D: Deserializer<'de>, { - Ok(deserializer.deserialize_map(ThroughputVisitor)?) + Ok(deserializer.deserialize_map(ThroughputVisitor))? } } From 8c61d1ba0b3839828e06524583f4565dd7c8e2e6 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sun, 2 Oct 2022 11:04:07 +0200 Subject: [PATCH 15/37] Serialize as function --- client/sysinfo/src/lib.rs | 8 +++- client/sysinfo/src/sysinfo.rs | 45 ++++++++++++++----- .../benchmarking-cli/src/machine/hardware.rs | 3 +- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/client/sysinfo/src/lib.rs b/client/sysinfo/src/lib.rs index aa0c2960d609b..18c188c7f90c1 100644 --- a/client/sysinfo/src/lib.rs +++ b/client/sysinfo/src/lib.rs @@ -28,7 +28,9 @@ mod sysinfo_linux; pub use sysinfo::{ benchmark_cpu, benchmark_disk_random_writes, benchmark_disk_sequential_writes, - benchmark_memory, benchmark_sr25519_verify, gather_hwbench, gather_sysinfo, Throughput, + benchmark_memory, benchmark_sr25519_verify, gather_hwbench, gather_sysinfo, + serialize_throughput, serialize_throughput_as_mibs, serialize_throughput_option_as_mibs, + Throughput, }; /// The operating system part of the current target triplet. @@ -44,12 +46,16 @@ pub const TARGET_ENV: &str = include_str!(concat!(env!("OUT_DIR"), "/target_env. #[derive(Clone, Debug, serde::Serialize)] pub struct HwBench { /// The CPU speed, as measured in how many MB/s it can hash using the BLAKE2b-256 hash. + #[serde(serialize_with = "serialize_throughput_as_mibs")] pub cpu_hashrate_score: Throughput, /// Memory bandwidth in MB/s, calculated by measuring the throughput of `memcpy`. + #[serde(serialize_with = "serialize_throughput_as_mibs")] pub memory_memcpy_score: Throughput, /// Sequential disk write speed in MB/s. + #[serde(serialize_with = "serialize_throughput_option_as_mibs")] pub disk_sequential_write_score: Option, /// Random disk write speed in MB/s. + #[serde(serialize_with = "serialize_throughput_option_as_mibs")] pub disk_random_write_score: Option, } diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index fecdc2c949a90..1b9099972dfee 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -27,7 +27,7 @@ use rand::{seq::SliceRandom, Rng, RngCore}; use serde::{ de::{MapAccess, Visitor}, ser::SerializeMap, - Deserialize, Deserializer, Serialize, Serializer, + Deserialize, Deserializer, Serializer, }; use std::{ fs::File, @@ -99,17 +99,38 @@ impl fmt::Display for Throughput { } } -impl Serialize for Throughput { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - // NOTE I will replace 4 with the actual length. - let mut map = serializer.serialize_map(Some(4))?; - let (value, unit) = self.normalize(); - map.serialize_entry(unit, &value)?; - map.end() +pub fn serialize_throughput(t: &Throughput, serializer: S) -> Result +where + S: Serializer, +{ + // NOTE I will replace 4 with the actual length. + let mut map = serializer.serialize_map(Some(4))?; + let (value, unit) = t.normalize(); + map.serialize_entry(unit, &value)?; + map.end() +} + +pub fn serialize_throughput_as_mibs(t: &Throughput, serializer: S) -> Result +where + S: Serializer, +{ + let mut map = serializer.serialize_map(Some(4))?; + map.serialize_entry("MiBs", &t.as_mibs())?; + map.end() +} + +pub fn serialize_throughput_option_as_mibs( + maybe_t: &Option, + serializer: S, +) -> Result +where + S: Serializer, +{ + let mut map = serializer.serialize_map(Some(4))?; + if let Some(t) = maybe_t { + map.serialize_entry("MiBs", &t.as_mibs())?; } + map.end() } struct ThroughputVisitor; @@ -118,7 +139,7 @@ impl<'de> Visitor<'de> for ThroughputVisitor { type Value = Throughput; fn expecting(&self, formatter: &mut Formatter) -> fmt::Result { - formatter.write_str("A very usefull message") + formatter.write_str("A f64") } fn visit_map(self, mut access: M) -> Result diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 298e95f226a0a..4b43dbb4ce6d2 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -18,7 +18,7 @@ //! Contains types to define hardware requirements. use lazy_static::lazy_static; -use sc_sysinfo::Throughput; +use sc_sysinfo::{serialize_throughput, Throughput}; use serde::{Deserialize, Serialize}; lazy_static! { @@ -45,6 +45,7 @@ pub struct Requirement { /// The metric to measure. pub metric: Metric, /// The minimal throughput that needs to be archived for this requirement. + #[serde(serialize_with = "serialize_throughput")] pub minimum: Throughput, } From ed10eaefc762f71420d3947167c62ac964308353 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sun, 2 Oct 2022 15:45:43 +0200 Subject: [PATCH 16/37] test HwBench --- client/sysinfo/src/sysinfo.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 1b9099972dfee..46b083589d0ac 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -630,4 +630,19 @@ mod tests { let mib = Throughput::from_mibs(1029.0); assert_eq!("1.00 GiBs", mib.to_string()); } + + /// Test the [`HwBench`]. + #[test] + fn hw_serialize_works() { + let hwbench = HwBench { + cpu_hashrate_score: Throughput::from_gibs(1.32), + memory_memcpy_score: Throughput::from_kibs(1342.432), + disk_sequential_write_score: Some(Throughput::from_kibs(432.12)), + disk_random_write_score: None, + }; + + let serialized = serde_json::to_string(&hwbench).unwrap(); + // All the throughput should be converted to MiBs. + assert_eq!(serialized, "{\"cpu_hashrate_score\":{\"MiBs\":1351.68},\"memory_memcpy_score\":{\"MiBs\":1.31096875},\"disk_sequential_write_score\":{\"MiBs\":0.4219921875},\"disk_random_write_score\":{}}"); + } } From 099713175c75230a56b1f88f1dd2005a1d1dbad3 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sun, 2 Oct 2022 15:49:44 +0200 Subject: [PATCH 17/37] rename --- client/sysinfo/src/sysinfo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 46b083589d0ac..7ce6288a61e59 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -633,7 +633,7 @@ mod tests { /// Test the [`HwBench`]. #[test] - fn hw_serialize_works() { + fn hwbench_serialize_works() { let hwbench = HwBench { cpu_hashrate_score: Throughput::from_gibs(1.32), memory_memcpy_score: Throughput::from_kibs(1342.432), From 3785a62242253ecb1c4ed677eb202fb6af97347c Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 3 Oct 2022 11:29:04 +0200 Subject: [PATCH 18/37] fix serialization --- client/sysinfo/src/sysinfo.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 7ce6288a61e59..29829455830fa 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -114,9 +114,7 @@ pub fn serialize_throughput_as_mibs(t: &Throughput, serializer: S) -> Result< where S: Serializer, { - let mut map = serializer.serialize_map(Some(4))?; - map.serialize_entry("MiBs", &t.as_mibs())?; - map.end() + serializer.serialize_u64(t.as_mibs() as u64) } pub fn serialize_throughput_option_as_mibs( @@ -126,11 +124,10 @@ pub fn serialize_throughput_option_as_mibs( where S: Serializer, { - let mut map = serializer.serialize_map(Some(4))?; if let Some(t) = maybe_t { - map.serialize_entry("MiBs", &t.as_mibs())?; + return serializer.serialize_u64(t.as_mibs() as u64) } - map.end() + serializer.serialize_u64(0) } struct ThroughputVisitor; @@ -636,13 +633,13 @@ mod tests { fn hwbench_serialize_works() { let hwbench = HwBench { cpu_hashrate_score: Throughput::from_gibs(1.32), - memory_memcpy_score: Throughput::from_kibs(1342.432), - disk_sequential_write_score: Some(Throughput::from_kibs(432.12)), + memory_memcpy_score: Throughput::from_kibs(9342.432), + disk_sequential_write_score: Some(Throughput::from_kibs(4332.12)), disk_random_write_score: None, }; let serialized = serde_json::to_string(&hwbench).unwrap(); // All the throughput should be converted to MiBs. - assert_eq!(serialized, "{\"cpu_hashrate_score\":{\"MiBs\":1351.68},\"memory_memcpy_score\":{\"MiBs\":1.31096875},\"disk_sequential_write_score\":{\"MiBs\":0.4219921875},\"disk_random_write_score\":{}}"); + assert_eq!(serialized, "{\"cpu_hashrate_score\":1351,\"memory_memcpy_score\":9,\"disk_sequential_write_score\":4,\"disk_random_write_score\":0}"); } } From 7354ccb294ab1f8734c0043bec68ab820349bf35 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 3 Oct 2022 11:47:28 +0200 Subject: [PATCH 19/37] deserialize as function --- client/sysinfo/src/lib.rs | 6 +++--- client/sysinfo/src/sysinfo.rs | 12 +++++------- utils/frame/benchmarking-cli/src/machine/hardware.rs | 4 ++-- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/client/sysinfo/src/lib.rs b/client/sysinfo/src/lib.rs index 18c188c7f90c1..3084b407a2aed 100644 --- a/client/sysinfo/src/lib.rs +++ b/client/sysinfo/src/lib.rs @@ -28,9 +28,9 @@ mod sysinfo_linux; pub use sysinfo::{ benchmark_cpu, benchmark_disk_random_writes, benchmark_disk_sequential_writes, - benchmark_memory, benchmark_sr25519_verify, gather_hwbench, gather_sysinfo, - serialize_throughput, serialize_throughput_as_mibs, serialize_throughput_option_as_mibs, - Throughput, + benchmark_memory, benchmark_sr25519_verify, deserialize_throughput, gather_hwbench, + gather_sysinfo, serialize_throughput, serialize_throughput_as_mibs, + serialize_throughput_option_as_mibs, Throughput, }; /// The operating system part of the current target triplet. diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 29829455830fa..ed4b703a08609 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -155,13 +155,11 @@ impl<'de> Visitor<'de> for ThroughputVisitor { } } -impl<'de> Deserialize<'de> for Throughput { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - Ok(deserializer.deserialize_map(ThroughputVisitor))? - } +pub fn deserialize_throughput<'de, D>(deserializer: D) -> Result +where + D: Deserializer<'de>, +{ + Ok(deserializer.deserialize_map(ThroughputVisitor))? } #[inline(always)] diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 4b43dbb4ce6d2..6b987bfe6a821 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -18,7 +18,7 @@ //! Contains types to define hardware requirements. use lazy_static::lazy_static; -use sc_sysinfo::{serialize_throughput, Throughput}; +use sc_sysinfo::{deserialize_throughput, serialize_throughput, Throughput}; use serde::{Deserialize, Serialize}; lazy_static! { @@ -45,7 +45,7 @@ pub struct Requirement { /// The metric to measure. pub metric: Metric, /// The minimal throughput that needs to be archived for this requirement. - #[serde(serialize_with = "serialize_throughput")] + #[serde(serialize_with = "serialize_throughput", deserialize_with = "deserialize_throughput")] pub minimum: Throughput, } From c3d0995c8778e95cc58e62595f79c3316c7d6523 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 3 Oct 2022 11:58:25 +0200 Subject: [PATCH 20/37] unused import --- client/sysinfo/src/sysinfo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index ed4b703a08609..73d2216560f2d 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -27,7 +27,7 @@ use rand::{seq::SliceRandom, Rng, RngCore}; use serde::{ de::{MapAccess, Visitor}, ser::SerializeMap, - Deserialize, Deserializer, Serializer, + Deserializer, Serializer, }; use std::{ fs::File, From bab6f75a2f60d60a1dfd79bee7b5e350040933c3 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 3 Oct 2022 18:12:02 +0200 Subject: [PATCH 21/37] move serialize/deserialize --- Cargo.lock | 1 + client/sysinfo/Cargo.toml | 2 + client/sysinfo/src/lib.rs | 5 +- client/sysinfo/src/sysinfo.rs | 53 ++----------------- utils/frame/benchmarking-cli/Cargo.toml | 1 + .../benchmarking-cli/src/machine/hardware.rs | 52 +++++++++++++++++- 6 files changed, 59 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bdf811a0617a8..1a746b245e097 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2183,6 +2183,7 @@ dependencies = [ "sp-keystore", "sp-runtime", "sp-state-machine", + "sp-std", "sp-storage", "sp-trie", "tempfile", diff --git a/client/sysinfo/Cargo.toml b/client/sysinfo/Cargo.toml index 6917b45eb9799..882cbd96c1c5f 100644 --- a/client/sysinfo/Cargo.toml +++ b/client/sysinfo/Cargo.toml @@ -26,4 +26,6 @@ sc-telemetry = { version = "4.0.0-dev", path = "../telemetry" } sp-core = { version = "6.0.0", path = "../../primitives/core" } sp-io = { version = "6.0.0", path = "../../primitives/io" } sp-std = { version = "4.0.0", path = "../../primitives/std" } + +[dev-dependencies] sp-runtime = { version = "6.0.0", path = "../../primitives/runtime" } diff --git a/client/sysinfo/src/lib.rs b/client/sysinfo/src/lib.rs index 3084b407a2aed..2cd679fafeecb 100644 --- a/client/sysinfo/src/lib.rs +++ b/client/sysinfo/src/lib.rs @@ -28,9 +28,8 @@ mod sysinfo_linux; pub use sysinfo::{ benchmark_cpu, benchmark_disk_random_writes, benchmark_disk_sequential_writes, - benchmark_memory, benchmark_sr25519_verify, deserialize_throughput, gather_hwbench, - gather_sysinfo, serialize_throughput, serialize_throughput_as_mibs, - serialize_throughput_option_as_mibs, Throughput, + benchmark_memory, benchmark_sr25519_verify, gather_hwbench, gather_sysinfo, + serialize_throughput_as_mibs, serialize_throughput_option_as_mibs, Throughput, }; /// The operating system part of the current target triplet. diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 73d2216560f2d..00766eead70a8 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -21,14 +21,10 @@ use crate::{ExecutionLimit, HwBench}; use sc_telemetry::SysInfo; use sp_core::{sr25519, Pair}; use sp_io::crypto::sr25519_verify; -use sp_std::{fmt, fmt::Formatter, prelude::*}; +use sp_std::{fmt, prelude::*}; use rand::{seq::SliceRandom, Rng, RngCore}; -use serde::{ - de::{MapAccess, Visitor}, - ser::SerializeMap, - Deserializer, Serializer, -}; +use serde::Serializer; use std::{ fs::File, io::{Seek, SeekFrom, Write}, @@ -79,7 +75,7 @@ impl Throughput { } /// Normalizes [`Self`] to use the larges unit possible. - fn normalize(&self) -> (f64, &'static str) { + pub fn normalize(&self) -> (f64, &'static str) { let bs = self.0; if bs >= KIBIBYTE * KIBIBYTE * KIBIBYTE { @@ -99,17 +95,6 @@ impl fmt::Display for Throughput { } } -pub fn serialize_throughput(t: &Throughput, serializer: S) -> Result -where - S: Serializer, -{ - // NOTE I will replace 4 with the actual length. - let mut map = serializer.serialize_map(Some(4))?; - let (value, unit) = t.normalize(); - map.serialize_entry(unit, &value)?; - map.end() -} - pub fn serialize_throughput_as_mibs(t: &Throughput, serializer: S) -> Result where S: Serializer, @@ -130,38 +115,6 @@ where serializer.serialize_u64(0) } -struct ThroughputVisitor; - -impl<'de> Visitor<'de> for ThroughputVisitor { - type Value = Throughput; - - fn expecting(&self, formatter: &mut Formatter) -> fmt::Result { - formatter.write_str("A f64") - } - - fn visit_map(self, mut access: M) -> Result - where - M: MapAccess<'de>, - { - let (key, value): (&str, f64) = - if let Some((key, value)) = access.next_entry()? { (key, value) } else { todo!() }; - if key == "KiBs" { - return Ok(Throughput(value * KIBIBYTE)) - } else if key == "MiBs" { - return Ok(Throughput(value * KIBIBYTE * KIBIBYTE)) - } else { - return Ok(Throughput(value * KIBIBYTE * KIBIBYTE * KIBIBYTE)) - } - } -} - -pub fn deserialize_throughput<'de, D>(deserializer: D) -> Result -where - D: Deserializer<'de>, -{ - Ok(deserializer.deserialize_map(ThroughputVisitor))? -} - #[inline(always)] pub(crate) fn benchmark( name: &str, diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index 108d0d338c2b3..552e373ee95f9 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -54,6 +54,7 @@ sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } sp-keystore = { version = "0.12.0", path = "../../../primitives/keystore" } sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } sp-state-machine = { version = "0.12.0", path = "../../../primitives/state-machine" } +sp-std = { version = "4.0.0", path = "../../../primitives/std" } sp-storage = { version = "6.0.0", path = "../../../primitives/storage" } sp-trie = { version = "6.0.0", path = "../../../primitives/trie" } gethostname = "0.2.3" diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 6b987bfe6a821..b6fef7163ed47 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -18,8 +18,56 @@ //! Contains types to define hardware requirements. use lazy_static::lazy_static; -use sc_sysinfo::{deserialize_throughput, serialize_throughput, Throughput}; -use serde::{Deserialize, Serialize}; +use sc_sysinfo::Throughput; +use serde::{ + de::{MapAccess, Visitor}, + ser::SerializeMap, + Deserialize, Deserializer, Serialize, Serializer, +}; +use sp_std::{fmt, fmt::Formatter}; + +pub fn serialize_throughput(t: &Throughput, serializer: S) -> Result +where + S: Serializer, +{ + // NOTE I will replace 4 with the actual length. + let mut map = serializer.serialize_map(Some(4))?; + let (value, unit) = t.normalize(); + map.serialize_entry(unit, &value)?; + map.end() +} + +struct ThroughputVisitor; + +impl<'de> Visitor<'de> for ThroughputVisitor { + type Value = Throughput; + + fn expecting(&self, formatter: &mut Formatter) -> fmt::Result { + formatter.write_str("A f64") + } + + fn visit_map(self, mut access: M) -> Result + where + M: MapAccess<'de>, + { + let (key, value): (&str, f64) = + if let Some((key, value)) = access.next_entry()? { (key, value) } else { todo!() }; + if key == "KiBs" { + return Ok(Throughput::from_kibs(value)) + } else if key == "MiBs" { + return Ok(Throughput::from_mibs(value)) + } else { + return Ok(Throughput::from_gibs(value)) + } + } +} + +pub fn deserialize_throughput<'de, D>(deserializer: D) -> Result +where + D: Deserializer<'de>, +{ + Ok(deserializer.deserialize_map(ThroughputVisitor))? +} lazy_static! { /// The hardware requirements as measured on reference hardware. From 51a71acaeb3369cec976d2c42329a83a58c64f1d Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 3 Oct 2022 19:19:01 +0200 Subject: [PATCH 22/37] don't serialize none --- client/sysinfo/src/lib.rs | 10 ++++++++-- client/sysinfo/src/sysinfo.rs | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/client/sysinfo/src/lib.rs b/client/sysinfo/src/lib.rs index 2cd679fafeecb..ddfddcbb816dd 100644 --- a/client/sysinfo/src/lib.rs +++ b/client/sysinfo/src/lib.rs @@ -51,10 +51,16 @@ pub struct HwBench { #[serde(serialize_with = "serialize_throughput_as_mibs")] pub memory_memcpy_score: Throughput, /// Sequential disk write speed in MB/s. - #[serde(serialize_with = "serialize_throughput_option_as_mibs")] + #[serde( + serialize_with = "serialize_throughput_option_as_mibs", + skip_serializing_if = "Option::is_none" + )] pub disk_sequential_write_score: Option, /// Random disk write speed in MB/s. - #[serde(serialize_with = "serialize_throughput_option_as_mibs")] + #[serde( + serialize_with = "serialize_throughput_option_as_mibs", + skip_serializing_if = "Option::is_none" + )] pub disk_random_write_score: Option, } diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 00766eead70a8..26bf9ac4547a7 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -591,6 +591,6 @@ mod tests { let serialized = serde_json::to_string(&hwbench).unwrap(); // All the throughput should be converted to MiBs. - assert_eq!(serialized, "{\"cpu_hashrate_score\":1351,\"memory_memcpy_score\":9,\"disk_sequential_write_score\":4,\"disk_random_write_score\":0}"); + assert_eq!(serialized, "{\"cpu_hashrate_score\":1351,\"memory_memcpy_score\":9,\"disk_sequential_write_score\":4}"); } } From a62ecb89c41c245c5763c4182b9d5e31a17a3163 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 3 Oct 2022 19:20:45 +0200 Subject: [PATCH 23/37] remove nonsense --- utils/frame/benchmarking-cli/src/machine/hardware.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index b6fef7163ed47..642613a79ea05 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -31,7 +31,7 @@ where S: Serializer, { // NOTE I will replace 4 with the actual length. - let mut map = serializer.serialize_map(Some(4))?; + let mut map = serializer.serialize_map(Some(1))?; let (value, unit) = t.normalize(); map.serialize_entry(unit, &value)?; map.end() From 0ed33204405d58f65d3c66fe5baacfa8001bebbf Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 3 Oct 2022 19:21:29 +0200 Subject: [PATCH 24/37] remove nonsense comment :P --- utils/frame/benchmarking-cli/src/machine/hardware.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 642613a79ea05..6feffab64965c 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -30,7 +30,6 @@ pub fn serialize_throughput(t: &Throughput, serializer: S) -> Result Date: Tue, 4 Oct 2022 20:27:15 +0200 Subject: [PATCH 25/37] fixes --- client/sysinfo/src/sysinfo.rs | 4 ++-- utils/frame/benchmarking-cli/src/machine/hardware.rs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 26bf9ac4547a7..14c58c302a6c8 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -112,7 +112,7 @@ where if let Some(t) = maybe_t { return serializer.serialize_u64(t.as_mibs() as u64) } - serializer.serialize_u64(0) + serializer.serialize_none() } #[inline(always)] @@ -143,7 +143,7 @@ pub(crate) fn benchmark( let score = Throughput((size * count) as f64 / elapsed.as_secs_f64()); log::trace!( - "Calculated {} of {:.2} in {} iterations in {}ms", + "Calculated {} of {} in {} iterations in {}ms", name, score, count, diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 6feffab64965c..2936744b5ecee 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -51,12 +51,12 @@ impl<'de> Visitor<'de> for ThroughputVisitor { { let (key, value): (&str, f64) = if let Some((key, value)) = access.next_entry()? { (key, value) } else { todo!() }; - if key == "KiBs" { - return Ok(Throughput::from_kibs(value)) - } else if key == "MiBs" { - return Ok(Throughput::from_mibs(value)) - } else { - return Ok(Throughput::from_gibs(value)) + + match key { + "KiBs" => Ok(Throughput::from_kibs(value)), + "MiBs" => Ok(Throughput::from_mibs(value)), + "GiBs" => Ok(Throughput::from_gibs(value)), + _ => todo!(), } } } From cd73e342af8fd555e40b1263ce617ebd8ff9e253 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 5 Oct 2022 09:22:12 +0200 Subject: [PATCH 26/37] remove all the todos --- .../frame/benchmarking-cli/src/machine/hardware.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 2936744b5ecee..4d0d969ef751e 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -20,7 +20,7 @@ use lazy_static::lazy_static; use sc_sysinfo::Throughput; use serde::{ - de::{MapAccess, Visitor}, + de::{Error, MapAccess, Visitor}, ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer, }; @@ -42,21 +42,24 @@ impl<'de> Visitor<'de> for ThroughputVisitor { type Value = Throughput; fn expecting(&self, formatter: &mut Formatter) -> fmt::Result { - formatter.write_str("A f64") + formatter.write_str("A map where the key is the unit and value is a f64.") } fn visit_map(self, mut access: M) -> Result where M: MapAccess<'de>, { - let (key, value): (&str, f64) = - if let Some((key, value)) = access.next_entry()? { (key, value) } else { todo!() }; + let (key, value): (&str, f64) = if let Some((key, value)) = access.next_entry()? { + (key, value) + } else { + return Err(M::Error::custom("Expected an entry.")) + }; match key { "KiBs" => Ok(Throughput::from_kibs(value)), "MiBs" => Ok(Throughput::from_mibs(value)), "GiBs" => Ok(Throughput::from_gibs(value)), - _ => todo!(), + _ => Err(M::Error::custom("Invalid unit.")), } } } From 5028b9a67abde3318cec5dffda261e0675b0c66f Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 5 Oct 2022 09:48:05 +0200 Subject: [PATCH 27/37] return enum --- client/sysinfo/src/sysinfo.rs | 27 +++++++++++++++---- .../benchmarking-cli/src/machine/hardware.rs | 2 +- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 14c58c302a6c8..3a4c359ae1752 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -33,6 +33,23 @@ use std::{ time::{Duration, Instant}, }; +pub enum Unit { + GiBs, + MiBs, + KiBs, +} + +impl fmt::Display for Unit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let unit = match self { + Unit::GiBs => "GiBs", + Unit::MiBs => "MiBs", + Unit::KiBs => "KiBs", + }; + write!(f, "{}", unit) + } +} + /// Throughput as measured in bytes per second. #[derive(Debug, Clone, Copy, PartialEq, PartialOrd)] pub struct Throughput(f64); @@ -75,15 +92,15 @@ impl Throughput { } /// Normalizes [`Self`] to use the larges unit possible. - pub fn normalize(&self) -> (f64, &'static str) { + pub fn normalize(&self) -> (f64, Unit) { let bs = self.0; if bs >= KIBIBYTE * KIBIBYTE * KIBIBYTE { - (self.as_gibs(), "GiBs") + (self.as_gibs(), Unit::GiBs) } else if bs >= KIBIBYTE * KIBIBYTE { - (self.as_mibs(), "MiBs") + (self.as_mibs(), Unit::MiBs) } else { - (self.as_kibs(), "KiBs") + (self.as_kibs(), Unit::KiBs) } } } @@ -91,7 +108,7 @@ impl Throughput { impl fmt::Display for Throughput { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let (value, unit) = self.normalize(); - write!(f, "{:.2?} {}", value, unit) + write!(f, "{:.2?} {}", value, unit.to_string()) } } diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 4d0d969ef751e..cab5d0e5f45c7 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -32,7 +32,7 @@ where { let mut map = serializer.serialize_map(Some(1))?; let (value, unit) = t.normalize(); - map.serialize_entry(unit, &value)?; + map.serialize_entry(&unit.to_string(), &value)?; map.end() } From 1f08df953345033b7cbe8a198cbc84b31534cc40 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 6 Oct 2022 18:19:40 +0200 Subject: [PATCH 28/37] fixes --- client/sysinfo/src/lib.rs | 2 +- client/sysinfo/src/sysinfo.rs | 7 +++---- utils/frame/benchmarking-cli/src/machine/hardware.rs | 10 ++++++++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/client/sysinfo/src/lib.rs b/client/sysinfo/src/lib.rs index ddfddcbb816dd..3ded63c2e2ac7 100644 --- a/client/sysinfo/src/lib.rs +++ b/client/sysinfo/src/lib.rs @@ -29,7 +29,7 @@ mod sysinfo_linux; pub use sysinfo::{ benchmark_cpu, benchmark_disk_random_writes, benchmark_disk_sequential_writes, benchmark_memory, benchmark_sr25519_verify, gather_hwbench, gather_sysinfo, - serialize_throughput_as_mibs, serialize_throughput_option_as_mibs, Throughput, + serialize_throughput_as_mibs, serialize_throughput_option_as_mibs, Throughput, Unit, }; /// The operating system part of the current target triplet. diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 3a4c359ae1752..a5ea14fdcef54 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -41,12 +41,11 @@ pub enum Unit { impl fmt::Display for Unit { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let unit = match self { + f.write_str(match self { Unit::GiBs => "GiBs", Unit::MiBs => "MiBs", Unit::KiBs => "KiBs", - }; - write!(f, "{}", unit) + }) } } @@ -127,7 +126,7 @@ where S: Serializer, { if let Some(t) = maybe_t { - return serializer.serialize_u64(t.as_mibs() as u64) + return serializer.serialize_some(&(t.as_mibs() as u64)) } serializer.serialize_none() } diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index cab5d0e5f45c7..f961662d04311 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -18,7 +18,7 @@ //! Contains types to define hardware requirements. use lazy_static::lazy_static; -use sc_sysinfo::Throughput; +use sc_sysinfo::{Throughput, Unit}; use serde::{ de::{Error, MapAccess, Visitor}, ser::SerializeMap, @@ -32,7 +32,13 @@ where { let mut map = serializer.serialize_map(Some(1))?; let (value, unit) = t.normalize(); - map.serialize_entry(&unit.to_string(), &value)?; + let unit_as_str = match unit { + Unit::GiBs => "GiBs", + Unit::MiBs => "MiBs", + Unit::KiBs => "KiBs", + }; + + map.serialize_entry(&unit_as_str, &value)?; map.end() } From f59280a6b5a93e0b7f3959b3b54becf313d2e40b Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 7 Oct 2022 17:23:20 +0200 Subject: [PATCH 29/37] fix nit --- client/sysinfo/src/sysinfo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index a5ea14fdcef54..0e359c8866eb2 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -107,7 +107,7 @@ impl Throughput { impl fmt::Display for Throughput { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let (value, unit) = self.normalize(); - write!(f, "{:.2?} {}", value, unit.to_string()) + write!(f, "{:.2?} {}", value, unit) } } From 3173864e1de9a0859aed86f5d5cc8a4c9bdc544a Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 7 Oct 2022 17:37:35 +0200 Subject: [PATCH 30/37] improve docs & readability --- client/sysinfo/src/sysinfo.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 0e359c8866eb2..fcf79d8831e70 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -33,6 +33,7 @@ use std::{ time::{Duration, Instant}, }; +/// Used to represent the unit used in the benchmarks. pub enum Unit { GiBs, MiBs, @@ -71,6 +72,7 @@ impl Throughput { Throughput(gibs * KIBIBYTE * KIBIBYTE * KIBIBYTE) } + /// [`Self`] as f64. pub fn as_f64(&self) -> f64 { self.0 } @@ -90,7 +92,7 @@ impl Throughput { self.0 / (KIBIBYTE * KIBIBYTE * KIBIBYTE) } - /// Normalizes [`Self`] to use the larges unit possible. + /// Normalizes [`Self`] to use the largest unit possible. pub fn normalize(&self) -> (f64, Unit) { let bs = self.0; @@ -111,22 +113,25 @@ impl fmt::Display for Throughput { } } -pub fn serialize_throughput_as_mibs(t: &Throughput, serializer: S) -> Result +pub fn serialize_throughput_as_mibs( + throughput: &Throughput, + serializer: S, +) -> Result where S: Serializer, { - serializer.serialize_u64(t.as_mibs() as u64) + serializer.serialize_u64(throughput.as_mibs() as u64) } pub fn serialize_throughput_option_as_mibs( - maybe_t: &Option, + maybe_throughput: &Option, serializer: S, ) -> Result where S: Serializer, { - if let Some(t) = maybe_t { - return serializer.serialize_some(&(t.as_mibs() as u64)) + if let Some(throughput) = maybe_throughput { + return serializer.serialize_some(&(throughput.as_mibs() as u64)) } serializer.serialize_none() } @@ -595,7 +600,7 @@ mod tests { assert_eq!("1.00 GiBs", mib.to_string()); } - /// Test the [`HwBench`]. + /// Test the [`HwBench`] serialization. #[test] fn hwbench_serialize_works() { let hwbench = HwBench { @@ -606,7 +611,7 @@ mod tests { }; let serialized = serde_json::to_string(&hwbench).unwrap(); - // All the throughput should be converted to MiBs. + // Throughput from all of the benchmarks should be converted to MiBs. assert_eq!(serialized, "{\"cpu_hashrate_score\":1351,\"memory_memcpy_score\":9,\"disk_sequential_write_score\":4}"); } } From e63e6bd67f967941200b34504b68e3dddaa24332 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Wed, 12 Oct 2022 14:35:45 +0200 Subject: [PATCH 31/37] Update client/sysinfo/src/sysinfo.rs Co-authored-by: Oliver Tale-Yazdi --- client/sysinfo/src/sysinfo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index fcf79d8831e70..6c6af0b0fc892 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -33,7 +33,7 @@ use std::{ time::{Duration, Instant}, }; -/// Used to represent the unit used in the benchmarks. +/// The unit in which the [`Throughput`] (bytes per second) is denoted. pub enum Unit { GiBs, MiBs, From 1ee877200ddce644ea3704ae07b9a8596739eb0a Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 12 Oct 2022 18:35:09 +0200 Subject: [PATCH 32/37] fix all the nits --- client/sysinfo/src/sysinfo.rs | 40 ++++++++++--------- .../benchmarking-cli/src/machine/hardware.rs | 4 +- .../frame/benchmarking-cli/src/machine/mod.rs | 2 +- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 6c6af0b0fc892..8bee787d86547 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -33,7 +33,7 @@ use std::{ time::{Duration, Instant}, }; -/// The unit in which the [`Throughput`] (bytes per second) is denoted. +/// The unit in which the [`Throughput`] (bytes per second) is denoted. pub enum Unit { GiBs, MiBs, @@ -54,26 +54,28 @@ impl fmt::Display for Unit { #[derive(Debug, Clone, Copy, PartialEq, PartialOrd)] pub struct Throughput(f64); -const KIBIBYTE: f64 = 1024.0; +const KIBIBYTE: f64 = (1 << 10) as f64; +const MEBIBYTE: f64 = (1 << 20) as f64; +const GIBIBYTE: f64 = (1 << 30) as f64; impl Throughput { - /// `f64` kibibyte/s to byte/s. + /// Construct [`Self`] from kibibyte/s. pub fn from_kibs(kibs: f64) -> Throughput { Throughput(kibs * KIBIBYTE) } - /// `f64` mebibyte/s to byte/s. + /// Construct [`Self`] from mebibyte/s. pub fn from_mibs(mibs: f64) -> Throughput { - Throughput(mibs * KIBIBYTE * KIBIBYTE) + Throughput(mibs * MEBIBYTE) } - /// `f64` gibibyte/s to byte/s. + /// Construct [`Self`] from gibibyte/s. pub fn from_gibs(gibs: f64) -> Throughput { - Throughput(gibs * KIBIBYTE * KIBIBYTE * KIBIBYTE) + Throughput(gibs * GIBIBYTE) } - /// [`Self`] as f64. - pub fn as_f64(&self) -> f64 { + /// [`Self`] as number of byte/s. + pub fn as_byte(&self) -> f64 { self.0 } @@ -84,21 +86,21 @@ impl Throughput { /// [`Self`] as number of mebibyte/s. pub fn as_mibs(&self) -> f64 { - self.0 / (KIBIBYTE * KIBIBYTE) + self.0 / MEBIBYTE } /// [`Self`] as number of gibibyte/s. pub fn as_gibs(&self) -> f64 { - self.0 / (KIBIBYTE * KIBIBYTE * KIBIBYTE) + self.0 / GIBIBYTE } /// Normalizes [`Self`] to use the largest unit possible. pub fn normalize(&self) -> (f64, Unit) { let bs = self.0; - if bs >= KIBIBYTE * KIBIBYTE * KIBIBYTE { + if bs >= GIBIBYTE { (self.as_gibs(), Unit::GiBs) - } else if bs >= KIBIBYTE * KIBIBYTE { + } else if bs >= MEBIBYTE { (self.as_mibs(), Unit::MiBs) } else { (self.as_kibs(), Unit::KiBs) @@ -555,19 +557,19 @@ mod tests { #[test] fn test_benchmark_cpu() { - assert!(benchmark_cpu(DEFAULT_CPU_EXECUTION_LIMIT) > Throughput(0.0)); + assert!(benchmark_cpu(DEFAULT_CPU_EXECUTION_LIMIT) > Throughput::from_mibs(0.0)); } #[test] fn test_benchmark_memory() { - assert!(benchmark_memory(DEFAULT_MEMORY_EXECUTION_LIMIT) > Throughput(0.0)); + assert!(benchmark_memory(DEFAULT_MEMORY_EXECUTION_LIMIT) > Throughput::from_mibs(0.0)); } #[test] fn test_benchmark_disk_sequential_writes() { assert!( benchmark_disk_sequential_writes(DEFAULT_DISK_EXECUTION_LIMIT, "./".as_ref()).unwrap() > - Throughput(0.0) + Throughput::from_mibs(0.0) ); } @@ -575,13 +577,15 @@ mod tests { fn test_benchmark_disk_random_writes() { assert!( benchmark_disk_random_writes(DEFAULT_DISK_EXECUTION_LIMIT, "./".as_ref()).unwrap() > - Throughput(0.0) + Throughput::from_mibs(0.0) ); } #[test] fn test_benchmark_sr25519_verify() { - assert!(benchmark_sr25519_verify(ExecutionLimit::MaxIterations(1)) > Throughput(0.0)); + assert!( + benchmark_sr25519_verify(ExecutionLimit::MaxIterations(1)) > Throughput::from_mibs(0.0) + ); } /// Test the [`Throughput`]. diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index f961662d04311..2b41391d0504d 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -26,7 +26,7 @@ use serde::{ }; use sp_std::{fmt, fmt::Formatter}; -pub fn serialize_throughput(t: &Throughput, serializer: S) -> Result +fn serialize_throughput(t: &Throughput, serializer: S) -> Result where S: Serializer, { @@ -70,7 +70,7 @@ impl<'de> Visitor<'de> for ThroughputVisitor { } } -pub fn deserialize_throughput<'de, D>(deserializer: D) -> Result +fn deserialize_throughput<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, { diff --git a/utils/frame/benchmarking-cli/src/machine/mod.rs b/utils/frame/benchmarking-cli/src/machine/mod.rs index 9604349513d9a..597846c01d670 100644 --- a/utils/frame/benchmarking-cli/src/machine/mod.rs +++ b/utils/frame/benchmarking-cli/src/machine/mod.rs @@ -130,7 +130,7 @@ impl MachineCmd { // Dispatch the concrete function from `sc-sysinfo`. let score = self.measure(&requirement.metric, dir)?; - let rel_score = score.as_f64() / requirement.minimum.as_f64(); + let rel_score = score.as_byte() / requirement.minimum.as_byte(); // Sanity check if the result is off by factor >100x. if rel_score >= 100.0 || rel_score <= 0.01 { From 02f38627523d2f4576b93ca587ea5ccd7d2b78d2 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 12 Oct 2022 21:30:18 +0200 Subject: [PATCH 33/37] rename --- client/sysinfo/src/sysinfo.rs | 2 +- utils/frame/benchmarking-cli/src/machine/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 8bee787d86547..47de7a18bd310 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -75,7 +75,7 @@ impl Throughput { } /// [`Self`] as number of byte/s. - pub fn as_byte(&self) -> f64 { + pub fn as_bytes(&self) -> f64 { self.0 } diff --git a/utils/frame/benchmarking-cli/src/machine/mod.rs b/utils/frame/benchmarking-cli/src/machine/mod.rs index 597846c01d670..9be48a3b82da8 100644 --- a/utils/frame/benchmarking-cli/src/machine/mod.rs +++ b/utils/frame/benchmarking-cli/src/machine/mod.rs @@ -130,7 +130,7 @@ impl MachineCmd { // Dispatch the concrete function from `sc-sysinfo`. let score = self.measure(&requirement.metric, dir)?; - let rel_score = score.as_byte() / requirement.minimum.as_byte(); + let rel_score = score.as_bytes() / requirement.minimum.as_bytes(); // Sanity check if the result is off by factor >100x. if rel_score >= 100.0 || rel_score <= 0.01 { From cbca2c2c1e05288de38c741db25f8a72c4c44664 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 15 Oct 2022 10:03:08 +0200 Subject: [PATCH 34/37] fix --- client/sysinfo/src/sysinfo.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 47de7a18bd310..98d529c71f1dd 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -115,6 +115,7 @@ impl fmt::Display for Throughput { } } +/// Serializes `Throughput` and uses MiBs as the unit. pub fn serialize_throughput_as_mibs( throughput: &Throughput, serializer: S, @@ -125,6 +126,7 @@ where serializer.serialize_u64(throughput.as_mibs() as u64) } +/// Serializes `Option` and uses MiBs as the unit. pub fn serialize_throughput_option_as_mibs( maybe_throughput: &Option, serializer: S, @@ -164,7 +166,7 @@ pub(crate) fn benchmark( } } - let score = Throughput((size * count) as f64 / elapsed.as_secs_f64()); + let score = Throughput::from_mibs((size * count) as f64 / elapsed.as_secs_f64()); log::trace!( "Calculated {} of {} in {} iterations in {}ms", name, From 30fe5eef8476a344640a4736dde753a5f56ed1fa Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Thu, 20 Oct 2022 17:44:00 +0200 Subject: [PATCH 35/37] Update client/sysinfo/src/sysinfo.rs Co-authored-by: Oliver Tale-Yazdi --- client/sysinfo/src/sysinfo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index 98d529c71f1dd..a91d64682f8be 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -166,7 +166,7 @@ pub(crate) fn benchmark( } } - let score = Throughput::from_mibs((size * count) as f64 / elapsed.as_secs_f64()); + let score = Throughput::from_kibs((size * count) as f64 / (elapsed.as_secs_f64() * 1024.0)); log::trace!( "Calculated {} of {} in {} iterations in {}ms", name, From 7a3fe3e866888040af10032bc8036b47278aea4b Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 29 Oct 2022 15:04:42 +0200 Subject: [PATCH 36/37] remove unit from serialization --- client/sysinfo/src/lib.rs | 10 ++-- client/sysinfo/src/sysinfo.rs | 7 +-- .../benchmarking-cli/src/machine/hardware.rs | 50 ++++++------------- .../src/machine/reference_hardware.json | 20 ++------ 4 files changed, 27 insertions(+), 60 deletions(-) diff --git a/client/sysinfo/src/lib.rs b/client/sysinfo/src/lib.rs index 3ded63c2e2ac7..372c7c1a6d219 100644 --- a/client/sysinfo/src/lib.rs +++ b/client/sysinfo/src/lib.rs @@ -29,7 +29,7 @@ mod sysinfo_linux; pub use sysinfo::{ benchmark_cpu, benchmark_disk_random_writes, benchmark_disk_sequential_writes, benchmark_memory, benchmark_sr25519_verify, gather_hwbench, gather_sysinfo, - serialize_throughput_as_mibs, serialize_throughput_option_as_mibs, Throughput, Unit, + serialize_throughput, serialize_throughput_option, Throughput, }; /// The operating system part of the current target triplet. @@ -45,20 +45,20 @@ pub const TARGET_ENV: &str = include_str!(concat!(env!("OUT_DIR"), "/target_env. #[derive(Clone, Debug, serde::Serialize)] pub struct HwBench { /// The CPU speed, as measured in how many MB/s it can hash using the BLAKE2b-256 hash. - #[serde(serialize_with = "serialize_throughput_as_mibs")] + #[serde(serialize_with = "serialize_throughput")] pub cpu_hashrate_score: Throughput, /// Memory bandwidth in MB/s, calculated by measuring the throughput of `memcpy`. - #[serde(serialize_with = "serialize_throughput_as_mibs")] + #[serde(serialize_with = "serialize_throughput")] pub memory_memcpy_score: Throughput, /// Sequential disk write speed in MB/s. #[serde( - serialize_with = "serialize_throughput_option_as_mibs", + serialize_with = "serialize_throughput_option", skip_serializing_if = "Option::is_none" )] pub disk_sequential_write_score: Option, /// Random disk write speed in MB/s. #[serde( - serialize_with = "serialize_throughput_option_as_mibs", + serialize_with = "serialize_throughput_option", skip_serializing_if = "Option::is_none" )] pub disk_random_write_score: Option, diff --git a/client/sysinfo/src/sysinfo.rs b/client/sysinfo/src/sysinfo.rs index a91d64682f8be..c66a6f6a62aed 100644 --- a/client/sysinfo/src/sysinfo.rs +++ b/client/sysinfo/src/sysinfo.rs @@ -116,10 +116,7 @@ impl fmt::Display for Throughput { } /// Serializes `Throughput` and uses MiBs as the unit. -pub fn serialize_throughput_as_mibs( - throughput: &Throughput, - serializer: S, -) -> Result +pub fn serialize_throughput(throughput: &Throughput, serializer: S) -> Result where S: Serializer, { @@ -127,7 +124,7 @@ where } /// Serializes `Option` and uses MiBs as the unit. -pub fn serialize_throughput_option_as_mibs( +pub fn serialize_throughput_option( maybe_throughput: &Option, serializer: S, ) -> Result diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index 2b41391d0504d..ea56bfee0430c 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -18,55 +18,31 @@ //! Contains types to define hardware requirements. use lazy_static::lazy_static; -use sc_sysinfo::{Throughput, Unit}; -use serde::{ - de::{Error, MapAccess, Visitor}, - ser::SerializeMap, - Deserialize, Deserializer, Serialize, Serializer, -}; +use sc_sysinfo::Throughput; +use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer}; use sp_std::{fmt, fmt::Formatter}; -fn serialize_throughput(t: &Throughput, serializer: S) -> Result +/// Serializes throughput into MiBs and represents it as `f64`. +fn serialize_throughput_as_f64(throughput: &Throughput, serializer: S) -> Result where S: Serializer, { - let mut map = serializer.serialize_map(Some(1))?; - let (value, unit) = t.normalize(); - let unit_as_str = match unit { - Unit::GiBs => "GiBs", - Unit::MiBs => "MiBs", - Unit::KiBs => "KiBs", - }; - - map.serialize_entry(&unit_as_str, &value)?; - map.end() + serializer.serialize_f64(throughput.as_mibs()) } struct ThroughputVisitor; - impl<'de> Visitor<'de> for ThroughputVisitor { type Value = Throughput; fn expecting(&self, formatter: &mut Formatter) -> fmt::Result { - formatter.write_str("A map where the key is the unit and value is a f64.") + formatter.write_str("A value that is a f64.") } - fn visit_map(self, mut access: M) -> Result + fn visit_f64(self, value: f64) -> Result where - M: MapAccess<'de>, + E: serde::de::Error, { - let (key, value): (&str, f64) = if let Some((key, value)) = access.next_entry()? { - (key, value) - } else { - return Err(M::Error::custom("Expected an entry.")) - }; - - match key { - "KiBs" => Ok(Throughput::from_kibs(value)), - "MiBs" => Ok(Throughput::from_mibs(value)), - "GiBs" => Ok(Throughput::from_gibs(value)), - _ => Err(M::Error::custom("Invalid unit.")), - } + Ok(Throughput::from_mibs(value)) } } @@ -74,7 +50,7 @@ fn deserialize_throughput<'de, D>(deserializer: D) -> Result, { - Ok(deserializer.deserialize_map(ThroughputVisitor))? + Ok(deserializer.deserialize_f64(ThroughputVisitor))? } lazy_static! { @@ -87,6 +63,7 @@ lazy_static! { /// pub static ref SUBSTRATE_REFERENCE_HARDWARE: Requirements = { let raw = include_bytes!("reference_hardware.json").as_slice(); + println!("{:?}", raw); serde_json::from_slice(raw).expect("Hardcoded data is known good; qed") }; } @@ -101,7 +78,10 @@ pub struct Requirement { /// The metric to measure. pub metric: Metric, /// The minimal throughput that needs to be archived for this requirement. - #[serde(serialize_with = "serialize_throughput", deserialize_with = "deserialize_throughput")] + #[serde( + serialize_with = "serialize_throughput_as_f64", + deserialize_with = "deserialize_throughput" + )] pub minimum: Throughput, } diff --git a/utils/frame/benchmarking-cli/src/machine/reference_hardware.json b/utils/frame/benchmarking-cli/src/machine/reference_hardware.json index 12645df8391e7..2a451d31403f1 100644 --- a/utils/frame/benchmarking-cli/src/machine/reference_hardware.json +++ b/utils/frame/benchmarking-cli/src/machine/reference_hardware.json @@ -1,32 +1,22 @@ [ { "metric": "Blake2256", - "minimum": { - "MiBs": 1029.0 - } + "minimum": 1029.0 }, { "metric": "Sr25519Verify", - "minimum": { - "KiBs": 666.0 - } + "minimum": 0.650391 }, { "metric": "MemCopy", - "minimum": { - "GiBs": 14.323 - } + "minimum": 14666.752 }, { "metric": "DiskSeqWrite", - "minimum": { - "MiBs": 450.0 - } + "minimum": 450.0 }, { "metric": "DiskRndWrite", - "minimum": { - "MiBs": 200.0 - } + "minimum": 200.0 } ] From 9211e1450fec7184c1d96e182d3863c7a5a41ce3 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 4 Nov 2022 17:43:17 +0100 Subject: [PATCH 37/37] Update utils/frame/benchmarking-cli/src/machine/hardware.rs --- utils/frame/benchmarking-cli/src/machine/hardware.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/machine/hardware.rs b/utils/frame/benchmarking-cli/src/machine/hardware.rs index ea56bfee0430c..50c88ec74646c 100644 --- a/utils/frame/benchmarking-cli/src/machine/hardware.rs +++ b/utils/frame/benchmarking-cli/src/machine/hardware.rs @@ -63,7 +63,6 @@ lazy_static! { /// pub static ref SUBSTRATE_REFERENCE_HARDWARE: Requirements = { let raw = include_bytes!("reference_hardware.json").as_slice(); - println!("{:?}", raw); serde_json::from_slice(raw).expect("Hardcoded data is known good; qed") }; }