Skip to content

Commit

Permalink
Do not drop the task_manager for benchmarking stuff (paritytech#12147)
Browse files Browse the repository at this point in the history
We can not drop the `task_manager` for benchmarking stuff, because otherwise stuff that may needs
this feature (like background signature verification) will fail. Besides the base path setup is
moved to `SharedParams` directly. Meaning any call to `base_path` will now directly return a tmp
path when `--dev` is given.
  • Loading branch information
bkchr authored and ark0f committed Feb 27, 2023
1 parent 29b10fe commit 4af37e4
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 18 deletions.
30 changes: 17 additions & 13 deletions bin/node/cli/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,35 +115,39 @@ pub fn run() -> Result<()> {
cmd.run::<Block, ExecutorDispatch>(config)
},
BenchmarkCmd::Block(cmd) => {
let PartialComponents { client, .. } = new_partial(&config)?;
cmd.run(client)
// ensure that we keep the task manager alive
let partial = new_partial(&config)?;
cmd.run(partial.client)
},
BenchmarkCmd::Storage(cmd) => {
let PartialComponents { client, backend, .. } = new_partial(&config)?;
let db = backend.expose_db();
let storage = backend.expose_storage();
// ensure that we keep the task manager alive
let partial = new_partial(&config)?;
let db = partial.backend.expose_db();
let storage = partial.backend.expose_storage();

cmd.run(config, client, db, storage)
cmd.run(config, partial.client, db, storage)
},
BenchmarkCmd::Overhead(cmd) => {
let PartialComponents { client, .. } = new_partial(&config)?;
let ext_builder = RemarkBuilder::new(client.clone());
// ensure that we keep the task manager alive
let partial = new_partial(&config)?;
let ext_builder = RemarkBuilder::new(partial.client.clone());

cmd.run(config, client, inherent_benchmark_data()?, &ext_builder)
cmd.run(config, partial.client, inherent_benchmark_data()?, &ext_builder)
},
BenchmarkCmd::Extrinsic(cmd) => {
let PartialComponents { client, .. } = service::new_partial(&config)?;
// ensure that we keep the task manager alive
let partial = service::new_partial(&config)?;
// Register the *Remark* and *TKA* builders.
let ext_factory = ExtrinsicFactory(vec![
Box::new(RemarkBuilder::new(client.clone())),
Box::new(RemarkBuilder::new(partial.client.clone())),
Box::new(TransferKeepAliveBuilder::new(
client.clone(),
partial.client.clone(),
Sr25519Keyring::Alice.to_account_id(),
ExistentialDeposit::get(),
)),
]);

cmd.run(client, inherent_benchmark_data()?, &ext_factory)
cmd.run(partial.client, inherent_benchmark_data()?, &ext_factory)
},
BenchmarkCmd::Machine(cmd) =>
cmd.run(&config, SUBSTRATE_REFERENCE_HARDWARE.clone()),
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/commands/insert_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl InsertKeyCmd {
let suri = utils::read_uri(self.suri.as_ref())?;
let base_path = self
.shared_params
.base_path()
.base_path()?
.unwrap_or_else(|| BasePath::from_project("", "", &C::executable_name()));
let chain_id = self.shared_params.chain_id(self.shared_params.is_dev());
let chain_spec = cli.load_spec(&chain_id)?;
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/commands/run_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ impl CliConfiguration for RunCmd {
Ok(if self.tmp {
Some(BasePath::new_temp_dir()?)
} else {
match self.shared_params().base_path() {
match self.shared_params().base_path()? {
Some(r) => Some(r),
// If `dev` is enabled, we use the temp base path.
None if self.shared_params().is_dev() => Some(BasePath::new_temp_dir()?),
Expand Down
2 changes: 1 addition & 1 deletion client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
///
/// By default this is retrieved from `SharedParams`.
fn base_path(&self) -> Result<Option<BasePath>> {
Ok(self.shared_params().base_path())
self.shared_params().base_path()
}

/// Returns `true` if the node is for development or not
Expand Down
9 changes: 7 additions & 2 deletions client/cli/src/params/shared_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,13 @@ pub struct SharedParams {

impl SharedParams {
/// Specify custom base path.
pub fn base_path(&self) -> Option<BasePath> {
self.base_path.clone().map(Into::into)
pub fn base_path(&self) -> Result<Option<BasePath>, crate::Error> {
match &self.base_path {
Some(r) => Ok(Some(r.clone().into())),
// If `dev` is enabled, we use the temp base path.
None if self.is_dev() => Ok(Some(BasePath::new_temp_dir()?)),
None => Ok(None),
}
}

/// Specify the development chain.
Expand Down

0 comments on commit 4af37e4

Please sign in to comment.