Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Re-add deprecated --execution arg on benchmark pallet (#14567)
Browse files Browse the repository at this point in the history
* Add DeferGuard::new

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Re-add deprecated 'execution' arg to benchmark pallet cmd.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Extend tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Remove from tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
  • Loading branch information
ggwpez authored Jul 13, 2023
1 parent 5591c9b commit f2a5ca0
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 7 deletions.
2 changes: 1 addition & 1 deletion bin/node/cli/tests/benchmark_block_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ async fn benchmark_block_works() {
.arg(base_dir.path())
.args(["--from", "1", "--to", "1"])
.args(["--repeat", "1"])
.args(["--wasm-execution", "compiled"])
.args(["--wasm-execution=compiled"])
.status()
.unwrap();

Expand Down
1 change: 1 addition & 0 deletions bin/node/cli/tests/benchmark_extrinsic_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ fn benchmark_extrinsic(pallet: &str, extrinsic: &str) {
.args(&["--pallet", pallet, "--extrinsic", extrinsic])
// Run with low repeats for faster execution.
.args(["--warmup=10", "--repeat=10", "--max-ext-per-block=10"])
.args(["--wasm-execution=compiled"])
.status()
.unwrap();

Expand Down
1 change: 1 addition & 0 deletions bin/node/cli/tests/benchmark_overhead_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ fn benchmark_overhead_works() {
.args(["--warmup", "10", "--repeat", "10"])
.args(["--add", "100", "--mul", "1.2", "--metric", "p75"])
.args(["--max-ext-per-block", "10"])
.args(["--wasm-execution=compiled"])
.status()
.unwrap();
assert!(status.success());
Expand Down
16 changes: 10 additions & 6 deletions bin/node/cli/tests/benchmark_pallet_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,20 @@ fn benchmark_pallet_works() {
}

fn benchmark_pallet(steps: u32, repeat: u32, should_work: bool) {
let output = Command::new(cargo_bin("substrate"))
let status = Command::new(cargo_bin("substrate"))
.args(["benchmark", "pallet", "--dev"])
// Use the `addition` benchmark since is the fastest.
.args(["--pallet", "frame-benchmarking", "--extrinsic", "addition"])
.args(["--steps", &format!("{}", steps), "--repeat", &format!("{}", repeat)])
.output()
.args([
"--wasm-execution=compiled",
"--no-storage-info",
"--no-median-slopes",
"--no-min-squares",
"--heap-pages=4096",
])
.status()
.unwrap();

if output.status.success() != should_work {
let log = String::from_utf8_lossy(&output.stderr).to_string();
panic!("Test failed:\n{}", log);
}
assert_eq!(status.success(), should_work);
}
7 changes: 7 additions & 0 deletions primitives/core/src/defer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
#[must_use]
pub struct DeferGuard<F: FnOnce()>(pub Option<F>);

impl<F: FnOnce()> DeferGuard<F> {
/// Creates a new `DeferGuard` with the given closure.
pub fn new(f: F) -> Self {
Self(Some(f))
}
}

impl<F: FnOnce()> Drop for DeferGuard<F> {
fn drop(&mut self) {
self.0.take().map(|f| f());
Expand Down
10 changes: 10 additions & 0 deletions utils/frame/benchmarking-cli/src/pallet/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ impl PalletCmd {
<<<BB as BlockT>::Header as HeaderT>::Number as std::str::FromStr>::Err: std::fmt::Debug,
ExtraHostFunctions: sp_wasm_interface::HostFunctions,
{
let _d = self.execution.as_ref().map(|exec| {
// We print the warning at the end, since there is often A LOT of output.
sp_core::defer::DeferGuard::new(move || {
log::warn!(
target: LOG_TARGET,
"⚠️ Argument `--execution` is deprecated. Its value of `{exec}` has on effect.",
)
})
});

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())
Expand Down
4 changes: 4 additions & 0 deletions utils/frame/benchmarking-cli/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ pub struct PalletCmd {
)]
pub wasmtime_instantiation_strategy: WasmtimeInstantiationStrategy,

/// DEPRECATED: This argument has no effect.
#[arg(long = "execution")]
pub execution: Option<String>,

/// Limit the memory the database cache can use.
#[arg(long = "db-cache", value_name = "MiB", default_value_t = 1024)]
pub database_cache_size: u32,
Expand Down

0 comments on commit f2a5ca0

Please sign in to comment.