Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MGX-290] remove unsafe benchmarks code #79

Merged
merged 2 commits into from
Jan 5, 2023
Merged

[MGX-290] remove unsafe benchmarks code #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
369ed5c
Merge pull request #76 from mangata-finance/mangata-dev-v0.9.31
devdanco Dec 31, 2022
f29020f
Merge remote-tracking branch 'origin/mangata-dev' into feature/remove…
mateuszaaa Jan 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions frame/support/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1594,7 +1594,8 @@ pub mod pallet_prelude {
/// want to write an alternative to the `frame_system` pallet.
///
/// Also see
/// [`pallet::disable_frame_system_supertrait_check`](`frame_support::pallet_macros::disable_frame_system_supertrait_check`)
/// [`pallet::disable_frame_system_supertrait_check`](`frame_support::pallet_macros::
/// disable_frame_system_supertrait_check`)
///
/// ## Macro expansion:
///
Expand Down Expand Up @@ -1638,7 +1639,8 @@ pub mod pallet_prelude {
/// storages can opt-out from this constraint by using `#[pallet::unbounded]` (see
/// `#[pallet::storage]` for more info).
///
/// Also see [`pallet::generate_storage_info`](`frame_support::pallet_macros::generate_storage_info`)
/// Also see [`pallet::generate_storage_info`](`frame_support::pallet_macros::
/// generate_storage_info`)
///
/// # `pallet::storage_version`
///
Expand Down
59 changes: 36 additions & 23 deletions utils/frame/benchmarking-cli/src/extrinsic/bench.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use sp_runtime::{
transaction_validity::{InvalidTransaction, TransactionValidityError},
Digest, DigestItem, OpaqueExtrinsic,
};
use std::{cell::RefCell, rc::Rc};
use ver_api::VerApi;

use clap::Args;
Expand Down Expand Up @@ -82,7 +83,7 @@ pub(crate) struct Benchmark<Block, BA, C> {
}

pub(crate) struct BenchmarkVer<Block, BA, C> {
client: Arc<C>,
client: Rc<RefCell<C>>,
params: BenchmarkParams,
inherent_data: (sp_inherents::InherentData, sp_inherents::InherentData),
_p: PhantomData<(Block, BA)>,
Expand Down Expand Up @@ -239,7 +240,7 @@ where
{
/// Create a new [`Self`] from the arguments.
pub fn new(
client: Arc<C>,
client: Rc<RefCell<C>>,
params: BenchmarkParams,
inherent_data: (sp_inherents::InherentData, sp_inherents::InherentData),
) -> Self {
Expand Down Expand Up @@ -299,13 +300,11 @@ where
StateAction::ApplyChanges(sc_consensus::StorageChanges::Changes(block.storage_changes));
params.fork_choice = Some(ForkChoiceStrategy::LongestChain);

let mut c = self.client.clone();
unsafe {
let mut_c = Arc::<C>::get_mut_unchecked(&mut c);
futures::executor::block_on(mut_c.import_block(params, Default::default()))
.expect("importing a block doesn't fail");
}
info!("best number: {} ", c.info().best_number);
futures::executor::block_on(
self.client.borrow_mut().import_block(params, Default::default()),
)
.expect("importing a block doesn't fail");
info!("best number: {} ", self.client.borrow().info().best_number);
}

/// Builds a block that enqueues maximum possible amount of extrinsics
Expand All @@ -314,8 +313,13 @@ where
ext_builder: &dyn ExtrinsicBuilder,
) -> Result<sc_block_builder_ver::BuiltBlock<Block, BA::State>> {
let digest = self.create_digest(1_u64);
let mut builder = self.client.new_block(digest)?;
// Create and insert the inherents.
info!("creating remarks");
let remarks = (0..self.max_ext_per_block())
.map(|nonce| ext_builder.build(nonce).expect("remark txs creation should not fail"))
.collect::<Vec<_>>();

let client = self.client.borrow();
let mut builder = client.new_block(digest)?;

info!("creating inherents");
let (seed, inherents) = builder.create_inherents(self.inherent_data.0.clone()).unwrap();
Expand All @@ -335,8 +339,7 @@ where
let block = builder.build_with_seed(seed, |at, api| {
let mut valid_txs: Vec<(Option<sp_runtime::AccountId32>, Block::Extrinsic)> =
Default::default();
for nonce in 0..self.max_ext_per_block() {
let remark = ext_builder.build(nonce).expect("remark txs creation should not fail");
for remark in remarks {
match validate_transaction::<Block, C>(at, &api, remark.clone()) {
Ok(()) => {
valid_txs.push((None, remark));
Expand All @@ -356,6 +359,10 @@ where
valid_txs
})?;
info!("Extrinsics per block: {}", txs_count);

if txs_count >= self.max_ext_per_block() as u64 {
panic!("fully filled block should not consume more than half of pregenrated extrinsics .. consider increasing --max-ext-per-block paramter value");
}
Ok(block)
}

Expand All @@ -366,9 +373,15 @@ where
apply_previous_block_extrinsics: bool,
) -> Result<sc_block_builder_ver::BuiltBlock<Block, BA::State>> {
// Return early if we just want a block with inherents and no additional extrinsics.
let remarks = (txs_count..(txs_count * 2))
.map(|nonce| {
ext_builder.build(nonce as u32).expect("remark txs creation should not fail")
})
.collect::<Vec<_>>();

let digest = self.create_digest(3_u64);
let mut builder = self.client.new_block(digest)?;
let client = self.client.borrow();
let mut builder = client.new_block(digest)?;
let (seed, inherents) = builder.create_inherents(self.inherent_data.1.clone()).unwrap();
info!("pushing inherents");
for inherent in inherents {
Expand All @@ -380,16 +393,14 @@ where
});

let block = builder.build_with_seed(seed, |_, _| {
(txs_count..(txs_count * 2))
.map(|nonce| {
let remark = ext_builder
.build(nonce as u32)
.expect("remark txs creation should not fail");
(None, remark)
})
.collect::<Vec<_>>()
remarks.into_iter().map(|remark| (None, remark)).collect::<Vec<_>>()
})?;

info!(
"created block #{} with {} extrinsics",
block.block.header().number(),
block.block.extrinsics().len()
);
debug!("created block {:?}", block.block.clone());
Ok(block)
}
Expand All @@ -401,6 +412,7 @@ where
info!("Running {} warmups...", self.params.warmup);
for _ in 0..self.params.warmup {
self.client
.borrow()
.runtime_api()
.execute_block(&block_id, block.clone())
.map_err(|e| Error::Client(RuntimeApiError(e)))?;
Expand All @@ -411,7 +423,8 @@ where
// Execute a block multiple times and record each execution time.
for _ in 0..self.params.repeat {
let block = block.clone();
let runtime_api = self.client.runtime_api();
let client = self.client.borrow();
let runtime_api = client.runtime_api();
let start = Instant::now();

runtime_api
Expand Down
1 change: 0 additions & 1 deletion utils/frame/benchmarking-cli/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
// limitations under the License.

//! Contains the root [`BenchmarkCmd`] command and exports its sub-commands.
#![feature(get_mut_unchecked)]

mod block;
mod extrinsic;
Expand Down
3 changes: 2 additions & 1 deletion utils/frame/benchmarking-cli/src/overhead/cmd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use sp_runtime::{
traits::{Block as BlockT, Header as HeaderT},
DigestItem, OpaqueExtrinsic,
};
use std::{cell::RefCell, rc::Rc};
use ver_api::VerApi;

use clap::{Args, Parser};
Expand Down Expand Up @@ -146,7 +147,7 @@ impl OverheadCmd {
pub fn run_ver<Block, BA, C>(
&self,
cfg: Configuration,
client: Arc<C>,
client: Rc<RefCell<C>>,
inherent_data: (sp_inherents::InherentData, sp_inherents::InherentData),
ext_builder: &dyn ExtrinsicBuilder,
) -> Result<()>
Expand Down