From 2b16af69d4402106ef2c7ac88cf42b1a2a8f3c13 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 23 Sep 2024 00:28:38 +0200 Subject: [PATCH] [benchmarking] Reset to genesis storage after each run (#5655) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The genesis state is currently partially provided via `OverlayedChanges`, but these changes are reset by the runtime after the first repetition, causing the second repitition to use an invalid genesis state. Changes: - Provide the genesis state as a `Storage` without any `OverlayedChanges` to make it work correctly with repetitions. - Add `--genesis-builder-preset` option to use different genesis preset names. - Improve error messages. --------- Signed-off-by: Oliver Tale-Yazdi Co-authored-by: ggwpez Co-authored-by: Bastian Köcher Co-authored-by: Branislav Kontur (cherry picked from commit 2e4e5bf2fd0ae19fa38951c7e5f495dd1453b2bb) --- prdoc/pr_5655.prdoc | 15 ++ .../node/cli/tests/benchmark_pallet_works.rs | 35 +++ .../benchmarking/pov/src/benchmarking.rs | 31 +++ .../benchmarking-cli/src/pallet/command.rs | 228 +++++++----------- .../frame/benchmarking-cli/src/pallet/mod.rs | 16 +- .../benchmarking-cli/src/pallet/types.rs | 22 +- .../benchmarking-cli/src/pallet/writer.rs | 7 +- 7 files changed, 197 insertions(+), 157 deletions(-) create mode 100644 prdoc/pr_5655.prdoc diff --git a/prdoc/pr_5655.prdoc b/prdoc/pr_5655.prdoc new file mode 100644 index 000000000000..bfa2e295d157 --- /dev/null +++ b/prdoc/pr_5655.prdoc @@ -0,0 +1,15 @@ +title: '[benchmarking] Reset to genesis storage after each run' +doc: +- audience: Runtime Dev + description: |- + The genesis state is currently partially provided via `OverlayedChanges`, but these changes are reset by the runtime after the first repetition, causing the second repitition to use an invalid genesis state. + + Changes: + - Provide the genesis state as a `Storage` without any `OverlayedChanges` to make it work correctly with repetitions. + - Add `--genesis-builder-preset` option to use different genesis preset names. + - Improve error messages. +crates: +- name: frame-benchmarking-cli + bump: major +- name: frame-benchmarking-pallet-pov + bump: patch diff --git a/substrate/bin/node/cli/tests/benchmark_pallet_works.rs b/substrate/bin/node/cli/tests/benchmark_pallet_works.rs index 8441333429be..d913228881a4 100644 --- a/substrate/bin/node/cli/tests/benchmark_pallet_works.rs +++ b/substrate/bin/node/cli/tests/benchmark_pallet_works.rs @@ -33,6 +33,31 @@ fn benchmark_pallet_works() { benchmark_pallet(20, 50, true); } +#[test] +fn benchmark_pallet_args_work() { + benchmark_pallet_args(&["--list", "--pallet=pallet_balances"], true); + benchmark_pallet_args(&["--list", "--pallet=pallet_balances"], true); + benchmark_pallet_args( + &["--list", "--pallet=pallet_balances", "--genesis-builder=spec-genesis"], + true, + ); + benchmark_pallet_args( + &["--list", "--pallet=pallet_balances", "--chain=dev", "--genesis-builder=spec-genesis"], + true, + ); + + // Error because the genesis runtime does not have any presets in it: + benchmark_pallet_args( + &["--list", "--pallet=pallet_balances", "--chain=dev", "--genesis-builder=spec-runtime"], + false, + ); + // Error because no runtime is provided: + benchmark_pallet_args( + &["--list", "--pallet=pallet_balances", "--chain=dev", "--genesis-builder=runtime"], + false, + ); +} + fn benchmark_pallet(steps: u32, repeat: u32, should_work: bool) { let status = Command::new(cargo_bin("substrate-node")) .args(["benchmark", "pallet", "--dev"]) @@ -51,3 +76,13 @@ fn benchmark_pallet(steps: u32, repeat: u32, should_work: bool) { assert_eq!(status.success(), should_work); } + +fn benchmark_pallet_args(args: &[&str], should_work: bool) { + let status = Command::new(cargo_bin("substrate-node")) + .args(["benchmark", "pallet"]) + .args(args) + .status() + .unwrap(); + + assert_eq!(status.success(), should_work); +} diff --git a/substrate/frame/benchmarking/pov/src/benchmarking.rs b/substrate/frame/benchmarking/pov/src/benchmarking.rs index bf3d406d0b2b..d52fcc2689c4 100644 --- a/substrate/frame/benchmarking/pov/src/benchmarking.rs +++ b/substrate/frame/benchmarking/pov/src/benchmarking.rs @@ -26,6 +26,11 @@ use frame_support::traits::UnfilteredDispatchable; use frame_system::{Pallet as System, RawOrigin}; use sp_runtime::traits::Hash; +#[cfg(feature = "std")] +frame_support::parameter_types! { + pub static StorageRootHash: Option> = None; +} + #[benchmarks] mod benchmarks { use super::*; @@ -392,6 +397,32 @@ mod benchmarks { } } + #[benchmark] + fn storage_root_is_the_same_every_time(i: Linear<0, 10>) { + #[cfg(feature = "std")] + let root = sp_io::storage::root(sp_runtime::StateVersion::V1); + + #[cfg(feature = "std")] + match (i, StorageRootHash::get()) { + (0, Some(_)) => panic!("StorageRootHash should be None initially"), + (0, None) => StorageRootHash::set(Some(root)), + (_, Some(r)) if r == root => {}, + (_, Some(r)) => + panic!("StorageRootHash should be the same every time: {:?} vs {:?}", r, root), + (_, None) => panic!("StorageRootHash should be Some after the first iteration"), + } + + // Also test that everything is reset correctly: + sp_io::storage::set(b"key1", b"value"); + + #[block] + { + sp_io::storage::set(b"key2", b"value"); + } + + sp_io::storage::set(b"key3", b"value"); + } + impl_benchmark_test_suite!(Pallet, super::mock::new_test_ext(), super::mock::Test,); } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 471919815206..f1499f41c2ea 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -19,7 +19,7 @@ use super::{ types::{ComponentRange, ComponentRangeMap}, writer, ListOutput, PalletCmd, }; -use crate::pallet::{types::FetchedCode, GenesisBuilder}; +use crate::pallet::{types::FetchedCode, GenesisBuilderPolicy}; use codec::{Decode, Encode}; use frame_benchmarking::{ Analysis, BenchmarkBatch, BenchmarkBatchSplitResults, BenchmarkList, BenchmarkParameter, @@ -27,7 +27,7 @@ use frame_benchmarking::{ }; use frame_support::traits::StorageInfo; use linked_hash_map::LinkedHashMap; -use sc_chain_spec::json_patch::merge as json_merge; +use sc_chain_spec::GenesisConfigBuilderRuntimeCaller; use sc_cli::{execution_method_from_cli, ChainSpec, CliConfiguration, Result, SharedParams}; use sc_client_db::BenchmarkingState; use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY}; @@ -36,18 +36,17 @@ use sp_core::{ testing::{TestOffchainExt, TestTransactionPoolExt}, OffchainDbExt, OffchainWorkerExt, TransactionPoolExt, }, - traits::{CallContext, CodeExecutor, ReadRuntimeVersionExt, WrappedRuntimeCode}, + traits::{CallContext, CodeExecutor, ReadRuntimeVersionExt}, Hasher, }; use sp_externalities::Extensions; -use sp_genesis_builder::{PresetId, Result as GenesisBuildResult}; use sp_keystore::{testing::MemoryKeystore, KeystoreExt}; use sp_runtime::traits::Hash; -use sp_state_machine::{OverlayedChanges, StateMachine}; +use sp_state_machine::StateMachine; +use sp_storage::{well_known_keys::CODE, Storage}; use sp_trie::{proof_size_extension::ProofSizeExt, recorder::Recorder}; use sp_wasm_interface::HostFunctions; use std::{ - borrow::Cow, collections::{BTreeMap, BTreeSet, HashMap}, fmt::Debug, fs, @@ -162,9 +161,6 @@ generate the genesis state is deprecated. Please remove the `--chain`/`--dev`/`- point `--runtime` to your runtime blob and set `--genesis-builder=runtime`. This warning may \ become a hard error any time after December 2024."; -/// The preset that we expect to find in the GenesisBuilder runtime API. -const GENESIS_PRESET: &str = "development"; - impl PalletCmd { /// Runs the command and benchmarks a pallet. #[deprecated( @@ -214,9 +210,7 @@ impl PalletCmd { return self.output_from_results(&batches) } - let (genesis_storage, genesis_changes) = - self.genesis_storage::(&chain_spec)?; - let mut changes = genesis_changes.clone(); + let genesis_storage = self.genesis_storage::(&chain_spec)?; let cache_size = Some(self.database_cache_size as usize); let state_with_tracking = BenchmarkingState::::new( @@ -262,7 +256,7 @@ impl PalletCmd { Self::exec_state_machine( StateMachine::new( state, - &mut changes, + &mut Default::default(), &executor, "Benchmark_benchmark_metadata", &(self.extra).encode(), @@ -347,7 +341,6 @@ impl PalletCmd { for (s, selected_components) in all_components.iter().enumerate() { // First we run a verification if !self.no_verify { - let mut changes = genesis_changes.clone(); let state = &state_without_tracking; // Don't use these results since verification code will add overhead. let _batch: Vec = match Self::exec_state_machine::< @@ -357,7 +350,7 @@ impl PalletCmd { >( StateMachine::new( state, - &mut changes, + &mut Default::default(), &executor, "Benchmark_dispatch_benchmark", &( @@ -389,7 +382,6 @@ impl PalletCmd { } // Do one loop of DB tracking. { - let mut changes = genesis_changes.clone(); let state = &state_with_tracking; let batch: Vec = match Self::exec_state_machine::< std::result::Result, String>, @@ -398,7 +390,7 @@ impl PalletCmd { >( StateMachine::new( state, // todo remove tracking - &mut changes, + &mut Default::default(), &executor, "Benchmark_dispatch_benchmark", &( @@ -432,7 +424,6 @@ impl PalletCmd { } // Finally run a bunch of loops to get extrinsic timing information. for r in 0..self.external_repeat { - let mut changes = genesis_changes.clone(); let state = &state_without_tracking; let batch = match Self::exec_state_machine::< std::result::Result, String>, @@ -441,7 +432,7 @@ impl PalletCmd { >( StateMachine::new( state, // todo remove tracking - &mut changes, + &mut Default::default(), &executor, "Benchmark_dispatch_benchmark", &( @@ -567,19 +558,19 @@ impl PalletCmd { Ok(benchmarks_to_run) } - /// Produce a genesis storage and genesis changes. + /// Build the genesis storage by either the Genesis Builder API, chain spec or nothing. /// - /// It would be easier to only return one type, but there is no easy way to convert them. - // TODO: Re-write `BenchmarkingState` to not be such a clusterfuck and only accept - // `OverlayedChanges` instead of a mix between `OverlayedChanges` and `State`. But this can only - // be done once we deprecated and removed the legacy interface :( - fn genesis_storage( + /// Behaviour can be controlled by the `--genesis-builder` flag. + fn genesis_storage( &self, chain_spec: &Option>, - ) -> Result<(sp_storage::Storage, OverlayedChanges)> { - Ok(match (self.genesis_builder, self.runtime.is_some()) { - (Some(GenesisBuilder::None), _) => Default::default(), - (Some(GenesisBuilder::Spec), _) | (None, false) => { + ) -> Result { + Ok(match (self.genesis_builder, self.runtime.as_ref()) { + (Some(GenesisBuilderPolicy::None), Some(_)) => return Err("Cannot use `--genesis-builder=none` with `--runtime` since the runtime would be ignored.".into()), + (Some(GenesisBuilderPolicy::None), None) => Storage::default(), + (Some(GenesisBuilderPolicy::SpecGenesis | GenesisBuilderPolicy::Spec), Some(_)) => + return Err("Cannot use `--genesis-builder=spec-genesis` with `--runtime` since the runtime would be ignored.".into()), + (Some(GenesisBuilderPolicy::SpecGenesis | GenesisBuilderPolicy::Spec), None) | (None, None) => { log::warn!("{WARN_SPEC_GENESIS_CTOR}"); let Some(chain_spec) = chain_spec else { return Err("No chain spec specified to generate the genesis state".into()); @@ -589,111 +580,73 @@ impl PalletCmd { .build_storage() .map_err(|e| format!("{ERROR_CANNOT_BUILD_GENESIS}\nError: {e}"))?; - (storage, Default::default()) + storage }, - (Some(GenesisBuilder::Runtime), _) | (None, true) => - (Default::default(), self.genesis_from_runtime::()?), - }) - } + (Some(GenesisBuilderPolicy::SpecRuntime), Some(_)) => + return Err("Cannot use `--genesis-builder=spec` with `--runtime` since the runtime would be ignored.".into()), + (Some(GenesisBuilderPolicy::SpecRuntime), None) => { + let Some(chain_spec) = chain_spec else { + return Err("No chain spec specified to generate the genesis state".into()); + }; - /// Generate the genesis changeset by the runtime API. - fn genesis_from_runtime(&self) -> Result> { - let state = BenchmarkingState::::new( - Default::default(), - Some(self.database_cache_size as usize), - false, - false, - )?; + self.genesis_from_spec_runtime::(chain_spec.as_ref())? + }, + (Some(GenesisBuilderPolicy::Runtime), None) => return Err("Cannot use `--genesis-builder=runtime` without `--runtime`".into()), + (Some(GenesisBuilderPolicy::Runtime), Some(runtime)) | (None, Some(runtime)) => { + log::info!("Loading WASM from {}", runtime.display()); + + let code = fs::read(&runtime).map_err(|e| { + format!( + "Could not load runtime file from path: {}, error: {}", + runtime.display(), + e + ) + })?; - // Create a dummy WasmExecutor just to build the genesis storage. - let method = - execution_method_from_cli(self.wasm_method, self.wasmtime_instantiation_strategy); - let executor = WasmExecutor::<( - sp_io::SubstrateHostFunctions, - frame_benchmarking::benchmarking::HostFunctions, - F, - )>::builder() - .with_execution_method(method) - .with_allow_missing_host_functions(self.allow_missing_host_functions) - .build(); + self.genesis_from_code::(&code)? + } + }) + } - let runtime = self.runtime_blob(&state)?; - let runtime_code = runtime.code()?; + /// Setup the genesis state by calling the runtime APIs of the chain-specs genesis runtime. + fn genesis_from_spec_runtime( + &self, + chain_spec: &dyn ChainSpec, + ) -> Result { + log::info!("Building genesis state from chain spec runtime"); + let storage = chain_spec + .build_storage() + .map_err(|e| format!("{ERROR_CANNOT_BUILD_GENESIS}\nError: {e}"))?; - // We cannot use the `GenesisConfigBuilderRuntimeCaller` here since it returns the changes - // as `Storage` item, but we need it as `OverlayedChanges`. - let genesis_json: Option> = Self::exec_state_machine( - StateMachine::new( - &state, - &mut Default::default(), - &executor, - "GenesisBuilder_get_preset", - &None::.encode(), // Use the default preset - &mut Self::build_extensions(executor.clone(), state.recorder()), - &runtime_code, - CallContext::Offchain, - ), - "build the genesis spec", - )?; + let code: &Vec = + storage.top.get(CODE).ok_or("No runtime code in the genesis storage")?; - let Some(base_genesis_json) = genesis_json else { - return Err("GenesisBuilder::get_preset returned no data".into()) - }; - - let base_genesis_json = serde_json::from_slice::(&base_genesis_json) - .map_err(|e| format!("GenesisBuilder::get_preset returned invalid JSON: {:?}", e))?; - - let dev_genesis_json: Option> = Self::exec_state_machine( - StateMachine::new( - &state, - &mut Default::default(), - &executor, - "GenesisBuilder_get_preset", - &Some::(GENESIS_PRESET.into()).encode(), // Use the default preset - &mut Self::build_extensions(executor.clone(), state.recorder()), - &runtime_code, - CallContext::Offchain, - ), - "build the genesis spec", - )?; + self.genesis_from_code::(code) + } - let mut genesis_json = serde_json::Value::default(); - json_merge(&mut genesis_json, base_genesis_json); + fn genesis_from_code(&self, code: &[u8]) -> Result { + let genesis_config_caller = GenesisConfigBuilderRuntimeCaller::<( + sp_io::SubstrateHostFunctions, + frame_benchmarking::benchmarking::HostFunctions, + EHF, + )>::new(code); + let preset = Some(&self.genesis_builder_preset); - if let Some(dev) = dev_genesis_json { - let dev: serde_json::Value = serde_json::from_slice(&dev).map_err(|e| { - format!("GenesisBuilder::get_preset returned invalid JSON: {:?}", e) + let mut storage = + genesis_config_caller.get_storage_for_named_preset(preset).inspect_err(|e| { + let presets = genesis_config_caller.preset_names().unwrap_or_default(); + log::error!( + "Please pick one of the available presets with \ + `--genesis-builder-preset=`. Available presets ({}): {:?}. Error: {:?}", + presets.len(), + presets, + e + ); })?; - json_merge(&mut genesis_json, dev); - } else { - log::warn!( - "Could not find genesis preset '{GENESIS_PRESET}'. Falling back to default." - ); - } - - let json_pretty_str = serde_json::to_string_pretty(&genesis_json) - .map_err(|e| format!("json to string failed: {e}"))?; - - let mut changes = Default::default(); - let build_res: GenesisBuildResult = Self::exec_state_machine( - StateMachine::new( - &state, - &mut changes, - &executor, - "GenesisBuilder_build_state", - &json_pretty_str.encode(), - &mut Extensions::default(), - &runtime_code, - CallContext::Offchain, - ), - "populate the genesis state", - )?; - if let Err(e) = build_res { - return Err(format!("GenesisBuilder::build_state failed: {}", e).into()) - } + storage.top.insert(CODE.into(), code.into()); - Ok(changes) + Ok(storage) } /// Execute a state machine and decode its return value as `R`. @@ -737,19 +690,10 @@ impl PalletCmd { &self, state: &'a BenchmarkingState, ) -> Result, H>> { - if let Some(runtime) = &self.runtime { - log::info!("Loading WASM from {}", runtime.display()); - let code = fs::read(runtime)?; - let hash = sp_core::blake2_256(&code).to_vec(); - let wrapped_code = WrappedRuntimeCode(Cow::Owned(code)); - - Ok(FetchedCode::FromFile { wrapped_code, heap_pages: self.heap_pages, hash }) - } else { - log::info!("Loading WASM from genesis state"); - let state = sp_state_machine::backend::BackendRuntimeCode::new(state); - - Ok(FetchedCode::FromGenesis { state }) - } + log::info!("Loading WASM from state"); + let state = sp_state_machine::backend::BackendRuntimeCode::new(state); + + Ok(FetchedCode { state }) } /// Allocation strategy for pallet benchmarking. @@ -990,19 +934,25 @@ impl PalletCmd { if let Some(output_path) = &self.output { if !output_path.is_dir() && output_path.file_name().is_none() { - return Err("Output file or path is invalid!".into()) + return Err(format!( + "Output path is neither a directory nor a file: {output_path:?}" + ) + .into()) } } if let Some(header_file) = &self.header { if !header_file.is_file() { - return Err("Header file is invalid!".into()) + return Err(format!("Header file could not be found: {header_file:?}").into()) }; } if let Some(handlebars_template_file) = &self.template { if !handlebars_template_file.is_file() { - return Err("Handlebars template file is invalid!".into()) + return Err(format!( + "Handlebars template file could not be found: {handlebars_template_file:?}" + ) + .into()) }; } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs index ebf737be1dbf..a507c1d1f548 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs @@ -19,7 +19,7 @@ mod command; mod types; mod writer; -use crate::{pallet::types::GenesisBuilder, shared::HostInfoParams}; +use crate::{pallet::types::GenesisBuilderPolicy, shared::HostInfoParams}; use clap::ValueEnum; use sc_cli::{ WasmExecutionMethod, WasmtimeInstantiationStrategy, DEFAULT_WASMTIME_INSTANTIATION_STRATEGY, @@ -177,9 +177,17 @@ pub struct PalletCmd { /// How to construct the genesis state. /// - /// Uses `GenesisBuilder::Spec` by default and `GenesisBuilder::Runtime` if `runtime` is set. - #[arg(long, value_enum)] - pub genesis_builder: Option, + /// Uses `GenesisBuilderPolicy::Spec` by default and `GenesisBuilderPolicy::Runtime` if + /// `runtime` is set. + #[arg(long, value_enum, alias = "genesis-builder-policy")] + pub genesis_builder: Option, + + /// The preset that we expect to find in the GenesisBuilder runtime API. + /// + /// This can be useful when a runtime has a dedicated benchmarking preset instead of using the + /// default one. + #[arg(long, default_value = sp_genesis_builder::DEV_RUNTIME_PRESET)] + pub genesis_builder_preset: String, /// DEPRECATED: This argument has no effect. #[arg(long = "execution")] diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs index 2bb00d66560f..ce37be455e8a 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/types.rs @@ -18,13 +18,13 @@ //! Various types used by this crate. use sc_cli::Result; -use sp_core::traits::{RuntimeCode, WrappedRuntimeCode}; +use sp_core::traits::RuntimeCode; use sp_runtime::traits::Hash; /// How the genesis state for benchmarking should be build. #[derive(clap::ValueEnum, Debug, Eq, PartialEq, Clone, Copy)] #[clap(rename_all = "kebab-case")] -pub enum GenesisBuilder { +pub enum GenesisBuilderPolicy { /// Do not provide any genesis state. /// /// Benchmarks are advised to function with this, since they should setup their own required @@ -32,16 +32,19 @@ pub enum GenesisBuilder { None, /// Let the runtime build the genesis state through its `BuildGenesisConfig` runtime API. Runtime, + // Use the runtime from the Spec file to build the genesis state. + SpecRuntime, /// Use the spec file to build the genesis state. This fails when there is no spec. + SpecGenesis, + /// Same as `SpecGenesis` - only here for backwards compatibility. Spec, } /// A runtime blob that was either fetched from genesis storage or loaded from a file. // NOTE: This enum is only needed for the annoying lifetime bounds on `RuntimeCode`. Otherwise we // could just directly return the blob. -pub enum FetchedCode<'a, B, H> { - FromGenesis { state: sp_state_machine::backend::BackendRuntimeCode<'a, B, H> }, - FromFile { wrapped_code: WrappedRuntimeCode<'a>, heap_pages: Option, hash: Vec }, +pub struct FetchedCode<'a, B, H> { + pub state: sp_state_machine::backend::BackendRuntimeCode<'a, B, H>, } impl<'a, B, H> FetchedCode<'a, B, H> @@ -51,14 +54,7 @@ where { /// The runtime blob. pub fn code(&'a self) -> Result> { - match self { - Self::FromGenesis { state } => state.runtime_code().map_err(Into::into), - Self::FromFile { wrapped_code, heap_pages, hash } => Ok(RuntimeCode { - code_fetcher: wrapped_code, - heap_pages: *heap_pages, - hash: hash.clone(), - }), - } + self.state.runtime_code().map_err(Into::into) } } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs index df7d81b2822e..34f31734da67 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs @@ -484,7 +484,12 @@ pub(crate) fn write_results( benchmarks: results.clone(), }; - let mut output_file = fs::File::create(&file_path)?; + let file_path = fs::canonicalize(&file_path).map_err(|e| { + format!("Could not get absolute path for: {:?}. Error: {:?}", &file_path, e) + })?; + let mut output_file = fs::File::create(&file_path).map_err(|e| { + format!("Could not write weight file to: {:?}. Error: {:?}", &file_path, e) + })?; handlebars .render_template_to_write(&template, &hbs_data, &mut output_file) .map_err(|e| io_error(&e.to_string()))?;