From 4a67c04a98ae77dfb434b0b1b71067b012cccd35 Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Thu, 3 Mar 2022 19:21:06 +0300 Subject: [PATCH 01/13] Fix typos --- utils/frame/benchmarking-cli/src/lib.rs | 2 +- utils/frame/benchmarking-cli/src/storage/template.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index 640b1770f5c3f..9815fe88a7f02 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -67,7 +67,7 @@ pub struct BenchmarkCmd { #[clap(long = "json")] pub json_output: bool, - /// Write the raw results in JSON format into the give file. + /// Write the raw results in JSON format into the given file. #[clap(long, conflicts_with = "json-output")] pub json_file: Option, diff --git a/utils/frame/benchmarking-cli/src/storage/template.rs b/utils/frame/benchmarking-cli/src/storage/template.rs index 56e0869a914a1..0f204e462e1c5 100644 --- a/utils/frame/benchmarking-cli/src/storage/template.rs +++ b/utils/frame/benchmarking-cli/src/storage/template.rs @@ -85,7 +85,7 @@ impl TemplateData { Ok(()) } - /// Filles out the `weights.hbs` HBS template with its own data. + /// Fills out the `weights.hbs` HBS template with its own data. /// Writes the result to `path` which can be a directory or file. pub fn write(&self, path: &str) -> Result<()> { let mut handlebars = handlebars::Handlebars::new(); From 8aec4c72b9071354983d62df2aa5ea60f702691f Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Fri, 4 Mar 2022 22:02:33 +0300 Subject: [PATCH 02/13] Enable overwriting handlebars template --- utils/frame/benchmarking-cli/src/storage/cmd.rs | 14 ++++++++++++-- .../frame/benchmarking-cli/src/storage/template.rs | 12 +++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index 4376b616286a4..2b7655b95a915 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -29,7 +29,7 @@ use clap::{Args, Parser}; use log::info; use rand::prelude::*; use serde::Serialize; -use std::{fmt::Debug, sync::Arc}; +use std::{fmt::Debug, path::PathBuf, sync::Arc}; use super::{record::StatSelect, template::TemplateData}; @@ -83,6 +83,10 @@ pub struct StorageParams { #[clap(long)] pub skip_write: bool, + /// Path to Handlebars template file used for outputting benchmark results. (Optional) + #[clap(long)] + pub template_path: Option, + /// Rounds of warmups before measuring. /// Only supported for `read` benchmarks. #[clap(long, default_value = "1")] @@ -113,6 +117,12 @@ impl StorageCmd { Block: BlockT, C: UsageProvider + StorageProvider + HeaderBackend, { + if let Some(hbs_template) = &self.params.template_path { + if !hbs_template.is_file() { + return Err("Handlebars template file is invalid!".into()) + }; + } + let mut template = TemplateData::new(&cfg, &self.params); if !self.params.skip_read { @@ -131,7 +141,7 @@ impl StorageCmd { template.set_stats(None, Some(stats))?; } - template.write(&self.params.weight_path) + template.write(&self.params.weight_path, &self.params.template_path) } /// Returns the specified state version. diff --git a/utils/frame/benchmarking-cli/src/storage/template.rs b/utils/frame/benchmarking-cli/src/storage/template.rs index 0f204e462e1c5..2bc414ac82f6a 100644 --- a/utils/frame/benchmarking-cli/src/storage/template.rs +++ b/utils/frame/benchmarking-cli/src/storage/template.rs @@ -85,9 +85,9 @@ impl TemplateData { Ok(()) } - /// Fills out the `weights.hbs` HBS template with its own data. + /// Fills out the `weights.hbs` or specified HBS template with its own data. /// Writes the result to `path` which can be a directory or file. - pub fn write(&self, path: &str) -> Result<()> { + pub fn write(&self, path: &str, hbs_template_path: &Option) -> Result<()> { let mut handlebars = handlebars::Handlebars::new(); // Format large integers with underscore. handlebars.register_helper("underscore", Box::new(crate::writer::UnderscoreHelper)); @@ -97,8 +97,14 @@ impl TemplateData { let out_path = self.build_path(path); let mut fd = fs::File::create(&out_path)?; info!("Writing weights to {:?}", fs::canonicalize(&out_path)?); + + // Use custom template if provided. + let template = match hbs_template_path { + Some(template) => fs::read_to_string(template)?, + None => TEMPLATE.to_string(), + }; handlebars - .render_template_to_write(&TEMPLATE, &self, &mut fd) + .render_template_to_write(&template, &self, &mut fd) .map_err(|e| format!("HBS template write: {:?}", e).into()) } From 2f54260d2b40ac403d9c1a83090ba0f3c5f1bf0e Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Mon, 7 Mar 2022 21:58:00 +0300 Subject: [PATCH 03/13] Optionally name json output or disable json altogether --- .../frame/benchmarking-cli/src/storage/cmd.rs | 20 ++++++++++++++-- .../benchmarking-cli/src/storage/record.rs | 24 +++++++++++++++---- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index 2b7655b95a915..76978b8c11b6a 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -87,6 +87,18 @@ pub struct StorageParams { #[clap(long)] pub template_path: Option, + /// Don't write benchmark output to a JSON file. + #[clap(long)] + pub no_json: bool, + + /// Optionally specify the filename where to write the raw 'read' results in JSON format. + #[clap(long, conflicts_with = "no-json")] + pub json_read_path: Option, + + /// Optionally specify the filename where to write the raw 'write' results in JSON format. + #[clap(long, conflicts_with = "no-json")] + pub json_write_path: Option, + /// Rounds of warmups before measuring. /// Only supported for `read` benchmarks. #[clap(long, default_value = "1")] @@ -127,7 +139,9 @@ impl StorageCmd { if !self.params.skip_read { let record = self.bench_read(client.clone())?; - record.save_json(&cfg, "read")?; + if !self.params.no_json { + record.save_json(&cfg, &self.params.json_read_path, "read")?; + } let stats = record.calculate_stats()?; info!("Time summary [ns]:\n{:?}\nValue size summary:\n{:?}", stats.0, stats.1); template.set_stats(Some(stats), None)?; @@ -135,7 +149,9 @@ impl StorageCmd { if !self.params.skip_write { let record = self.bench_write(client, db, storage)?; - record.save_json(&cfg, "write")?; + if !self.params.no_json { + record.save_json(&cfg, &self.params.json_write_path, "write")?; + } let stats = record.calculate_stats()?; info!("Time summary [ns]:\n{:?}\nValue size summary:\n{:?}", stats.0, stats.1); template.set_stats(None, Some(stats))?; diff --git a/utils/frame/benchmarking-cli/src/storage/record.rs b/utils/frame/benchmarking-cli/src/storage/record.rs index 00a613c713007..d19439ca39ae1 100644 --- a/utils/frame/benchmarking-cli/src/storage/record.rs +++ b/utils/frame/benchmarking-cli/src/storage/record.rs @@ -22,7 +22,8 @@ use sc_service::Configuration; use log::info; use serde::Serialize; -use std::{fmt, fs, result, str::FromStr, time::Duration}; +use sp_runtime::traits::Clear; +use std::{fmt, fs, path::PathBuf, result, str::FromStr, time::Duration}; /// Raw output of a Storage benchmark. #[derive(Debug, Default, Clone, Serialize)] @@ -95,12 +96,27 @@ impl BenchRecord { Ok((time, size)) // The swap of time/size here is intentional. } - /// Saves the raw results in a json file in the current directory. + /// Unless a path is specified, saves the raw results in a json file in the current directory. /// Prefixes it with the DB name and suffixed with `path_suffix`. - pub fn save_json(&self, cfg: &Configuration, path_suffix: &str) -> Result<()> { - let path = format!("{}_{}.json", cfg.database, path_suffix).to_lowercase(); + pub fn save_json( + &self, + cfg: &Configuration, + out_path: &Option, + path_suffix: &str, + ) -> Result<()> { + let mut path = PathBuf::new(); + if let Some(p) = out_path { + path = PathBuf::from(p); + } + + if path.is_clear() || path.is_dir() { + path.push(&format!("{}_{}.json", cfg.database, path_suffix).to_lowercase()); + path.set_extension("json"); + } + let json = serde_json::to_string_pretty(&self) .map_err(|e| format!("Serializing as JSON: {:?}", e))?; + fs::write(&path, json)?; info!("Raw data written to {:?}", fs::canonicalize(&path)?); Ok(()) From 63f87b45c984c4a141df122d78b29d071eac5a30 Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Wed, 9 Mar 2022 20:57:29 +0300 Subject: [PATCH 04/13] Don't write to json by default --- .../frame/benchmarking-cli/src/storage/cmd.rs | 18 +++++++----------- .../benchmarking-cli/src/storage/record.rs | 16 +++------------- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index 76978b8c11b6a..cd13fad80890e 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -87,16 +87,12 @@ pub struct StorageParams { #[clap(long)] pub template_path: Option, - /// Don't write benchmark output to a JSON file. + /// Write the raw 'read' results in JSON format to the specified file. #[clap(long)] - pub no_json: bool, - - /// Optionally specify the filename where to write the raw 'read' results in JSON format. - #[clap(long, conflicts_with = "no-json")] pub json_read_path: Option, - /// Optionally specify the filename where to write the raw 'write' results in JSON format. - #[clap(long, conflicts_with = "no-json")] + /// Write the raw 'write' results in JSON format to the specified file. + #[clap(long)] pub json_write_path: Option, /// Rounds of warmups before measuring. @@ -139,8 +135,8 @@ impl StorageCmd { if !self.params.skip_read { let record = self.bench_read(client.clone())?; - if !self.params.no_json { - record.save_json(&cfg, &self.params.json_read_path, "read")?; + if let Some(path) = &self.params.json_read_path { + record.save_json(&cfg, &path, "read")?; } let stats = record.calculate_stats()?; info!("Time summary [ns]:\n{:?}\nValue size summary:\n{:?}", stats.0, stats.1); @@ -149,8 +145,8 @@ impl StorageCmd { if !self.params.skip_write { let record = self.bench_write(client, db, storage)?; - if !self.params.no_json { - record.save_json(&cfg, &self.params.json_write_path, "write")?; + if let Some(path) = &self.params.json_write_path { + record.save_json(&cfg, &path, "write")?; } let stats = record.calculate_stats()?; info!("Time summary [ns]:\n{:?}\nValue size summary:\n{:?}", stats.0, stats.1); diff --git a/utils/frame/benchmarking-cli/src/storage/record.rs b/utils/frame/benchmarking-cli/src/storage/record.rs index d19439ca39ae1..d40bf0344cc8e 100644 --- a/utils/frame/benchmarking-cli/src/storage/record.rs +++ b/utils/frame/benchmarking-cli/src/storage/record.rs @@ -22,7 +22,6 @@ use sc_service::Configuration; use log::info; use serde::Serialize; -use sp_runtime::traits::Clear; use std::{fmt, fs, path::PathBuf, result, str::FromStr, time::Duration}; /// Raw output of a Storage benchmark. @@ -98,18 +97,9 @@ impl BenchRecord { /// Unless a path is specified, saves the raw results in a json file in the current directory. /// Prefixes it with the DB name and suffixed with `path_suffix`. - pub fn save_json( - &self, - cfg: &Configuration, - out_path: &Option, - path_suffix: &str, - ) -> Result<()> { - let mut path = PathBuf::new(); - if let Some(p) = out_path { - path = PathBuf::from(p); - } - - if path.is_clear() || path.is_dir() { + pub fn save_json(&self, cfg: &Configuration, out_path: &str, path_suffix: &str) -> Result<()> { + let mut path = PathBuf::from(out_path); + if path.is_dir() { path.push(&format!("{}_{}.json", cfg.database, path_suffix).to_lowercase()); path.set_extension("json"); } From 191d0691af16b03c564b517d66efeed708295d95 Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Wed, 9 Mar 2022 20:30:36 +0300 Subject: [PATCH 05/13] Include block id in handlebars output --- utils/frame/benchmarking-cli/src/storage/cmd.rs | 4 ++++ utils/frame/benchmarking-cli/src/storage/template.rs | 9 ++++++++- utils/frame/benchmarking-cli/src/storage/weights.hbs | 1 + 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index cd13fad80890e..a5463ccc341f0 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -29,6 +29,7 @@ use clap::{Args, Parser}; use log::info; use rand::prelude::*; use serde::Serialize; +use sp_runtime::generic::BlockId; use std::{fmt::Debug, path::PathBuf, sync::Arc}; use super::{record::StatSelect, template::TemplateData}; @@ -133,6 +134,9 @@ impl StorageCmd { let mut template = TemplateData::new(&cfg, &self.params); + let block_id = BlockId::::Number(client.usage_info().chain.best_number); + template.set_block_id(block_id.to_string()); + if !self.params.skip_read { let record = self.bench_read(client.clone())?; if let Some(path) = &self.params.json_read_path { diff --git a/utils/frame/benchmarking-cli/src/storage/template.rs b/utils/frame/benchmarking-cli/src/storage/template.rs index 2bc414ac82f6a..8f9185cb7381c 100644 --- a/utils/frame/benchmarking-cli/src/storage/template.rs +++ b/utils/frame/benchmarking-cli/src/storage/template.rs @@ -32,6 +32,8 @@ static TEMPLATE: &str = include_str!("./weights.hbs"); pub(crate) struct TemplateData { /// Name of the database used. db_name: String, + /// Block number that was used. + block_id: String, /// Name of the runtime. Taken from the chain spec. runtime_name: String, /// Version of the benchmarking CLI used. @@ -85,6 +87,11 @@ impl TemplateData { Ok(()) } + /// Sets the block id that was used. + pub fn set_block_id(&mut self, block_id: String) { + self.block_id = block_id + } + /// Fills out the `weights.hbs` or specified HBS template with its own data. /// Writes the result to `path` which can be a directory or file. pub fn write(&self, path: &str, hbs_template_path: &Option) -> Result<()> { @@ -100,7 +107,7 @@ impl TemplateData { // Use custom template if provided. let template = match hbs_template_path { - Some(template) => fs::read_to_string(template)?, + Some(template) => fs::read_to_string(template)?, None => TEMPLATE.to_string(), }; handlebars diff --git a/utils/frame/benchmarking-cli/src/storage/weights.hbs b/utils/frame/benchmarking-cli/src/storage/weights.hbs index ffeb1fe04d81c..477c39e22891c 100644 --- a/utils/frame/benchmarking-cli/src/storage/weights.hbs +++ b/utils/frame/benchmarking-cli/src/storage/weights.hbs @@ -19,6 +19,7 @@ //! DATE: {{date}} //! //! DATABASE: `{{db_name}}`, RUNTIME: `{{runtime_name}}` +//! BLOCK-ID: `{{block_id}}` //! SKIP-WRITE: `{{params.skip_write}}`, SKIP-READ: `{{params.skip_read}}`, WARMUPS: `{{params.warmups}}` //! STATE-VERSION: `V{{params.state_version}}`, STATE-CACHE-SIZE: `{{params.state_cache_size}}` //! WEIGHT-PATH: `{{params.weight_path}}` From 7ca13b6f2c14636660533e4fbae0540a82689cc4 Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Wed, 9 Mar 2022 21:21:40 +0300 Subject: [PATCH 06/13] Include warmups for write benchmarks --- utils/frame/benchmarking-cli/src/storage/cmd.rs | 1 - .../frame/benchmarking-cli/src/storage/write.rs | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index a5463ccc341f0..941208da1fccd 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -97,7 +97,6 @@ pub struct StorageParams { pub json_write_path: Option, /// Rounds of warmups before measuring. - /// Only supported for `read` benchmarks. #[clap(long, default_value = "1")] pub warmups: u32, diff --git a/utils/frame/benchmarking-cli/src/storage/write.rs b/utils/frame/benchmarking-cli/src/storage/write.rs index eb9ba11f30696..9facea020dc1a 100644 --- a/utils/frame/benchmarking-cli/src/storage/write.rs +++ b/utils/frame/benchmarking-cli/src/storage/write.rs @@ -62,6 +62,22 @@ impl StorageCmd { let mut rng = Self::setup_rng(); kvs.shuffle(&mut rng); + // Run some rounds of the benchmark as warmup. + for i in 0..self.params.warmups { + info!("Warmup round {}/{}", i + 1, self.params.warmups); + for (k, original_v) in kvs.clone().iter() { + let mut new_v = vec![0; original_v.len()]; + rng.fill_bytes(&mut new_v[..]); + + let replace = vec![(k.as_ref(), Some(new_v.as_ref()))]; + let (_, stx) = trie.storage_root(replace.iter().cloned(), self.state_version()); + let tx = convert_tx::(stx.clone(), true, state_col, supports_rc); + db.commit(tx).map_err(|e| format!("Writing to the Database: {}", e))?; + let tx = convert_tx::(stx.clone(), false, state_col, supports_rc); + db.commit(tx).map_err(|e| format!("Writing to the Database: {}", e))?; + } + } + info!("Writing {} keys", kvs.len()); // Write each value in one commit. for (k, original_v) in kvs.iter() { From 52390f9f2d704a475142a2308cb35bbff289f56a Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Thu, 10 Mar 2022 17:56:25 +0300 Subject: [PATCH 07/13] PR comments --- utils/frame/benchmarking-cli/src/storage/cmd.rs | 10 ++-------- .../benchmarking-cli/src/storage/record.rs | 2 +- .../benchmarking-cli/src/storage/template.rs | 17 +++++++++-------- .../benchmarking-cli/src/storage/weights.hbs | 2 +- .../frame/benchmarking-cli/src/storage/write.rs | 1 + 5 files changed, 14 insertions(+), 18 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index 941208da1fccd..8f8ff7b1c82e1 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -84,7 +84,7 @@ pub struct StorageParams { #[clap(long)] pub skip_write: bool, - /// Path to Handlebars template file used for outputting benchmark results. (Optional) + /// Specify the Handlebars template to use for outputting benchmark results. #[clap(long)] pub template_path: Option, @@ -125,16 +125,10 @@ impl StorageCmd { Block: BlockT, C: UsageProvider + StorageProvider + HeaderBackend, { - if let Some(hbs_template) = &self.params.template_path { - if !hbs_template.is_file() { - return Err("Handlebars template file is invalid!".into()) - }; - } - let mut template = TemplateData::new(&cfg, &self.params); let block_id = BlockId::::Number(client.usage_info().chain.best_number); - template.set_block_id(block_id.to_string()); + template.set_block_number(block_id.to_string()); if !self.params.skip_read { let record = self.bench_read(client.clone())?; diff --git a/utils/frame/benchmarking-cli/src/storage/record.rs b/utils/frame/benchmarking-cli/src/storage/record.rs index d40bf0344cc8e..d932c41163a78 100644 --- a/utils/frame/benchmarking-cli/src/storage/record.rs +++ b/utils/frame/benchmarking-cli/src/storage/record.rs @@ -100,7 +100,7 @@ impl BenchRecord { pub fn save_json(&self, cfg: &Configuration, out_path: &str, path_suffix: &str) -> Result<()> { let mut path = PathBuf::from(out_path); if path.is_dir() { - path.push(&format!("{}_{}.json", cfg.database, path_suffix).to_lowercase()); + path.push(&format!("{}_{}", cfg.database, path_suffix).to_lowercase()); path.set_extension("json"); } diff --git a/utils/frame/benchmarking-cli/src/storage/template.rs b/utils/frame/benchmarking-cli/src/storage/template.rs index 8f9185cb7381c..57f958339158d 100644 --- a/utils/frame/benchmarking-cli/src/storage/template.rs +++ b/utils/frame/benchmarking-cli/src/storage/template.rs @@ -33,7 +33,7 @@ pub(crate) struct TemplateData { /// Name of the database used. db_name: String, /// Block number that was used. - block_id: String, + block_number: String, /// Name of the runtime. Taken from the chain spec. runtime_name: String, /// Version of the benchmarking CLI used. @@ -88,8 +88,8 @@ impl TemplateData { } /// Sets the block id that was used. - pub fn set_block_id(&mut self, block_id: String) { - self.block_id = block_id + pub fn set_block_number(&mut self, block_number: String) { + self.block_number = block_number } /// Fills out the `weights.hbs` or specified HBS template with its own data. @@ -100,16 +100,17 @@ impl TemplateData { handlebars.register_helper("underscore", Box::new(crate::writer::UnderscoreHelper)); // Don't HTML escape any characters. handlebars.register_escape_fn(|s| -> String { s.to_string() }); + // Use custom template if provided. + let template = match hbs_template_path { + Some(template) if template.is_file() => fs::read_to_string(template)?, + Some(_) => return Err("Handlebars template file is invalid!".into()), + None => TEMPLATE.to_string(), + }; let out_path = self.build_path(path); let mut fd = fs::File::create(&out_path)?; info!("Writing weights to {:?}", fs::canonicalize(&out_path)?); - // Use custom template if provided. - let template = match hbs_template_path { - Some(template) => fs::read_to_string(template)?, - None => TEMPLATE.to_string(), - }; handlebars .render_template_to_write(&template, &self, &mut fd) .map_err(|e| format!("HBS template write: {:?}", e).into()) diff --git a/utils/frame/benchmarking-cli/src/storage/weights.hbs b/utils/frame/benchmarking-cli/src/storage/weights.hbs index 477c39e22891c..bfb832cb847f9 100644 --- a/utils/frame/benchmarking-cli/src/storage/weights.hbs +++ b/utils/frame/benchmarking-cli/src/storage/weights.hbs @@ -19,7 +19,7 @@ //! DATE: {{date}} //! //! DATABASE: `{{db_name}}`, RUNTIME: `{{runtime_name}}` -//! BLOCK-ID: `{{block_id}}` +//! BLOCK-NUM: `{{block_number}}` //! SKIP-WRITE: `{{params.skip_write}}`, SKIP-READ: `{{params.skip_read}}`, WARMUPS: `{{params.warmups}}` //! STATE-VERSION: `V{{params.state_version}}`, STATE-CACHE-SIZE: `{{params.state_cache_size}}` //! WEIGHT-PATH: `{{params.weight_path}}` diff --git a/utils/frame/benchmarking-cli/src/storage/write.rs b/utils/frame/benchmarking-cli/src/storage/write.rs index 9facea020dc1a..a059ca8519ddb 100644 --- a/utils/frame/benchmarking-cli/src/storage/write.rs +++ b/utils/frame/benchmarking-cli/src/storage/write.rs @@ -63,6 +63,7 @@ impl StorageCmd { kvs.shuffle(&mut rng); // Run some rounds of the benchmark as warmup. + // The code here is a copy of the instrumentation run below, look there for comments. for i in 0..self.params.warmups { info!("Warmup round {}/{}", i + 1, self.params.warmups); for (k, original_v) in kvs.clone().iter() { From 47fc65c7a62afe66d0053eec97881761d5b1bac2 Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Thu, 10 Mar 2022 21:20:42 +0300 Subject: [PATCH 08/13] Drop unnecessary file extension --- utils/frame/benchmarking-cli/src/storage/template.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/storage/template.rs b/utils/frame/benchmarking-cli/src/storage/template.rs index 57f958339158d..66ad931b03984 100644 --- a/utils/frame/benchmarking-cli/src/storage/template.rs +++ b/utils/frame/benchmarking-cli/src/storage/template.rs @@ -120,7 +120,7 @@ impl TemplateData { fn build_path(&self, weight_out: &str) -> PathBuf { let mut path = PathBuf::from(weight_out); if path.is_dir() { - path.push(format!("{}_weights.rs", self.db_name.to_lowercase())); + path.push(format!("{}_weights", self.db_name.to_lowercase())); path.set_extension("rs"); } path From dc005de700b811d5c9bdae13f07a0c499ab14c60 Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Fri, 11 Mar 2022 15:26:43 +0300 Subject: [PATCH 09/13] Use more appropriate types --- utils/frame/benchmarking-cli/src/storage/cmd.rs | 12 ++++++------ utils/frame/benchmarking-cli/src/storage/record.rs | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index 8f8ff7b1c82e1..b6096afff0495 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -88,13 +88,13 @@ pub struct StorageParams { #[clap(long)] pub template_path: Option, - /// Write the raw 'read' results in JSON format to the specified file. + /// Path to write the raw 'read' results in JSON format to. Can be a file or directory. #[clap(long)] - pub json_read_path: Option, + pub json_read_path: Option, - /// Write the raw 'write' results in JSON format to the specified file. + /// Path to write the raw 'write' results in JSON format to. Can be a file or directory. #[clap(long)] - pub json_write_path: Option, + pub json_write_path: Option, /// Rounds of warmups before measuring. #[clap(long, default_value = "1")] @@ -133,7 +133,7 @@ impl StorageCmd { if !self.params.skip_read { let record = self.bench_read(client.clone())?; if let Some(path) = &self.params.json_read_path { - record.save_json(&cfg, &path, "read")?; + record.save_json(&cfg, path, "read")?; } let stats = record.calculate_stats()?; info!("Time summary [ns]:\n{:?}\nValue size summary:\n{:?}", stats.0, stats.1); @@ -143,7 +143,7 @@ impl StorageCmd { if !self.params.skip_write { let record = self.bench_write(client, db, storage)?; if let Some(path) = &self.params.json_write_path { - record.save_json(&cfg, &path, "write")?; + record.save_json(&cfg, path, "write")?; } let stats = record.calculate_stats()?; info!("Time summary [ns]:\n{:?}\nValue size summary:\n{:?}", stats.0, stats.1); diff --git a/utils/frame/benchmarking-cli/src/storage/record.rs b/utils/frame/benchmarking-cli/src/storage/record.rs index d932c41163a78..667274bef0dd5 100644 --- a/utils/frame/benchmarking-cli/src/storage/record.rs +++ b/utils/frame/benchmarking-cli/src/storage/record.rs @@ -97,10 +97,10 @@ impl BenchRecord { /// Unless a path is specified, saves the raw results in a json file in the current directory. /// Prefixes it with the DB name and suffixed with `path_suffix`. - pub fn save_json(&self, cfg: &Configuration, out_path: &str, path_suffix: &str) -> Result<()> { + pub fn save_json(&self, cfg: &Configuration, out_path: &PathBuf, suffix: &str) -> Result<()> { let mut path = PathBuf::from(out_path); - if path.is_dir() { - path.push(&format!("{}_{}", cfg.database, path_suffix).to_lowercase()); + if path.is_dir() || path.as_os_str().is_empty() { + path.push(&format!("{}_{}", cfg.database, suffix).to_lowercase()); path.set_extension("json"); } From fa2b98f8fea26b2f71bd13d64f151f45c767b74e Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Fri, 11 Mar 2022 15:28:26 +0300 Subject: [PATCH 10/13] Use more appropriate error message --- utils/frame/benchmarking-cli/src/storage/template.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/storage/template.rs b/utils/frame/benchmarking-cli/src/storage/template.rs index 66ad931b03984..7e929567e1167 100644 --- a/utils/frame/benchmarking-cli/src/storage/template.rs +++ b/utils/frame/benchmarking-cli/src/storage/template.rs @@ -103,7 +103,7 @@ impl TemplateData { // Use custom template if provided. let template = match hbs_template_path { Some(template) if template.is_file() => fs::read_to_string(template)?, - Some(_) => return Err("Handlebars template file is invalid!".into()), + Some(_) => return Err("Handlebars template is not a valid file!".into()), None => TEMPLATE.to_string(), }; From fe2cd6958da1b2276ecb549f8549196751f5a8c7 Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Fri, 11 Mar 2022 15:28:32 +0300 Subject: [PATCH 11/13] More use of more appropriate types --- utils/frame/benchmarking-cli/src/storage/cmd.rs | 4 ++-- .../frame/benchmarking-cli/src/storage/template.rs | 14 +++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index b6096afff0495..6d04f975e54b8 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -59,8 +59,8 @@ pub struct StorageCmd { pub struct StorageParams { /// Path to write the *weight* file to. Can be a file or directory. /// For substrate this should be `frame/support/src/weights`. - #[clap(long, default_value = ".")] - pub weight_path: String, + #[clap(long)] + pub weight_path: Option, /// Select a specific metric to calculate the final weight output. #[clap(long = "metric", default_value = "average")] diff --git a/utils/frame/benchmarking-cli/src/storage/template.rs b/utils/frame/benchmarking-cli/src/storage/template.rs index 7e929567e1167..13b825d891a51 100644 --- a/utils/frame/benchmarking-cli/src/storage/template.rs +++ b/utils/frame/benchmarking-cli/src/storage/template.rs @@ -94,14 +94,14 @@ impl TemplateData { /// Fills out the `weights.hbs` or specified HBS template with its own data. /// Writes the result to `path` which can be a directory or file. - pub fn write(&self, path: &str, hbs_template_path: &Option) -> Result<()> { + pub fn write(&self, path: &Option, hbs_template: &Option) -> Result<()> { let mut handlebars = handlebars::Handlebars::new(); // Format large integers with underscore. handlebars.register_helper("underscore", Box::new(crate::writer::UnderscoreHelper)); // Don't HTML escape any characters. handlebars.register_escape_fn(|s| -> String { s.to_string() }); // Use custom template if provided. - let template = match hbs_template_path { + let template = match hbs_template { Some(template) if template.is_file() => fs::read_to_string(template)?, Some(_) => return Err("Handlebars template is not a valid file!".into()), None => TEMPLATE.to_string(), @@ -117,9 +117,13 @@ impl TemplateData { } /// Builds a path for the weight file. - fn build_path(&self, weight_out: &str) -> PathBuf { - let mut path = PathBuf::from(weight_out); - if path.is_dir() { + fn build_path(&self, weight_out: &Option) -> PathBuf { + let mut path = match weight_out { + Some(p) => PathBuf::from(p), + None => PathBuf::new(), + }; + + if path.is_dir() || path.as_os_str().is_empty() { path.push(format!("{}_weights", self.db_name.to_lowercase())); path.set_extension("rs"); } From 89bf4e2c222af794fadfe043e0df73499eb2e505 Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Fri, 11 Mar 2022 21:47:20 +0300 Subject: [PATCH 12/13] Rework write benchmark warmups --- utils/frame/benchmarking-cli/src/storage/write.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/storage/write.rs b/utils/frame/benchmarking-cli/src/storage/write.rs index a059ca8519ddb..e7f0a50ecd0b9 100644 --- a/utils/frame/benchmarking-cli/src/storage/write.rs +++ b/utils/frame/benchmarking-cli/src/storage/write.rs @@ -62,20 +62,15 @@ impl StorageCmd { let mut rng = Self::setup_rng(); kvs.shuffle(&mut rng); - // Run some rounds of the benchmark as warmup. - // The code here is a copy of the instrumentation run below, look there for comments. + // Run some rounds of the benchmark as warmup, excluding db transactions. + // The code here is derived from the instrumentation run below, look there for comments. for i in 0..self.params.warmups { info!("Warmup round {}/{}", i + 1, self.params.warmups); - for (k, original_v) in kvs.clone().iter() { + for (k, original_v) in kvs.clone() { let mut new_v = vec![0; original_v.len()]; rng.fill_bytes(&mut new_v[..]); - let replace = vec![(k.as_ref(), Some(new_v.as_ref()))]; - let (_, stx) = trie.storage_root(replace.iter().cloned(), self.state_version()); - let tx = convert_tx::(stx.clone(), true, state_col, supports_rc); - db.commit(tx).map_err(|e| format!("Writing to the Database: {}", e))?; - let tx = convert_tx::(stx.clone(), false, state_col, supports_rc); - db.commit(tx).map_err(|e| format!("Writing to the Database: {}", e))?; + let _ = trie.storage_root(replace.iter().cloned(), self.state_version()); } } From db20064b6b04bc280e40259bd95c609582c9937b Mon Sep 17 00:00:00 2001 From: Daan Schutte Date: Mon, 14 Mar 2022 21:05:20 +0300 Subject: [PATCH 13/13] Run same benchmark for both read and write --- .../frame/benchmarking-cli/src/storage/cmd.rs | 30 +++++++++++++++++++ .../benchmarking-cli/src/storage/read.rs | 11 ------- .../benchmarking-cli/src/storage/write.rs | 12 -------- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index 6d04f975e54b8..ad7d13a2022e4 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -20,6 +20,7 @@ use sc_client_api::{Backend as ClientBackend, StorageProvider, UsageProvider}; use sc_client_db::DbHash; use sc_service::Configuration; use sp_blockchain::HeaderBackend; +use sp_core::storage::StorageKey; use sp_database::{ColumnId, Database}; use sp_runtime::traits::{Block as BlockT, HashFor}; use sp_state_machine::Storage; @@ -131,6 +132,7 @@ impl StorageCmd { template.set_block_number(block_id.to_string()); if !self.params.skip_read { + self.bench_warmup(&client)?; let record = self.bench_read(client.clone())?; if let Some(path) = &self.params.json_read_path { record.save_json(&cfg, path, "read")?; @@ -141,6 +143,7 @@ impl StorageCmd { } if !self.params.skip_write { + self.bench_warmup(&client)?; let record = self.bench_write(client, db, storage)?; if let Some(path) = &self.params.json_write_path { record.save_json(&cfg, path, "write")?; @@ -168,6 +171,33 @@ impl StorageCmd { info!("Using seed {}", seed); StdRng::seed_from_u64(seed) } + + /// Run some rounds of the (read) benchmark as warmup. + /// See `frame_benchmarking_cli::storage::read::bench_read` for detailed comments. + fn bench_warmup(&self, client: &Arc) -> Result<()> + where + C: UsageProvider + StorageProvider, + B: BlockT + Debug, + BA: ClientBackend, + { + let block = BlockId::Number(client.usage_info().chain.best_number); + let empty_prefix = StorageKey(Vec::new()); + let mut keys = client.storage_keys(&block, &empty_prefix)?; + let mut rng = Self::setup_rng(); + keys.shuffle(&mut rng); + + for i in 0..self.params.warmups { + info!("Warmup round {}/{}", i + 1, self.params.warmups); + for key in keys.clone() { + let _ = client + .storage(&block, &key) + .expect("Checked above to exist") + .ok_or("Value unexpectedly empty"); + } + } + + Ok(()) + } } // Boilerplate diff --git a/utils/frame/benchmarking-cli/src/storage/read.rs b/utils/frame/benchmarking-cli/src/storage/read.rs index 3974c4010f632..ca506202e1067 100644 --- a/utils/frame/benchmarking-cli/src/storage/read.rs +++ b/utils/frame/benchmarking-cli/src/storage/read.rs @@ -49,17 +49,6 @@ impl StorageCmd { let mut rng = Self::setup_rng(); keys.shuffle(&mut rng); - // Run some rounds of the benchmark as warmup. - for i in 0..self.params.warmups { - info!("Warmup round {}/{}", i + 1, self.params.warmups); - for key in keys.clone() { - let _ = client - .storage(&block, &key) - .expect("Checked above to exist") - .ok_or("Value unexpectedly empty")?; - } - } - // Interesting part here: // Read all the keys in the database and measure the time it takes to access each. info!("Reading {} keys", keys.len()); diff --git a/utils/frame/benchmarking-cli/src/storage/write.rs b/utils/frame/benchmarking-cli/src/storage/write.rs index e7f0a50ecd0b9..eb9ba11f30696 100644 --- a/utils/frame/benchmarking-cli/src/storage/write.rs +++ b/utils/frame/benchmarking-cli/src/storage/write.rs @@ -62,18 +62,6 @@ impl StorageCmd { let mut rng = Self::setup_rng(); kvs.shuffle(&mut rng); - // Run some rounds of the benchmark as warmup, excluding db transactions. - // The code here is derived from the instrumentation run below, look there for comments. - for i in 0..self.params.warmups { - info!("Warmup round {}/{}", i + 1, self.params.warmups); - for (k, original_v) in kvs.clone() { - let mut new_v = vec![0; original_v.len()]; - rng.fill_bytes(&mut new_v[..]); - let replace = vec![(k.as_ref(), Some(new_v.as_ref()))]; - let _ = trie.storage_root(replace.iter().cloned(), self.state_version()); - } - } - info!("Writing {} keys", kvs.len()); // Write each value in one commit. for (k, original_v) in kvs.iter() {