diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 48c1252236..fb112d433b 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -1556,6 +1556,21 @@ dependencies = [ "substrate-test-runtime-client", ] +[[package]] +name = "frame-system-benchmarking" +version = "2.0.0-dev" +dependencies = [ + "frame-benchmarking", + "frame-support", + "frame-system", + "parity-scale-codec", + "serde", + "sp-core", + "sp-io", + "sp-runtime", + "sp-std", +] + [[package]] name = "frame-system-rpc-runtime-api" version = "2.0.0-dev" @@ -3574,6 +3589,7 @@ dependencies = [ "frame-executive", "frame-support", "frame-system", + "frame-system-benchmarking", "frame-system-rpc-runtime-api", "integer-sqrt", "node-primitives", diff --git a/substrate/Cargo.toml b/substrate/Cargo.toml index 35d396741f..3882e96611 100644 --- a/substrate/Cargo.toml +++ b/substrate/Cargo.toml @@ -103,6 +103,7 @@ members = [ "frame/support/procedural/tools/derive", "frame/support/test", "frame/system", + "frame/system/benchmarking", "frame/system/rpc/runtime-api", "frame/timestamp", "frame/transaction-payment", @@ -181,4 +182,3 @@ members = [ [profile.release] # Substrate runtime requires unwinding. panic = "unwind" - diff --git a/substrate/bin/node/runtime/Cargo.toml b/substrate/bin/node/runtime/Cargo.toml index bf360821c8..9e39d9d24e 100644 --- a/substrate/bin/node/runtime/Cargo.toml +++ b/substrate/bin/node/runtime/Cargo.toml @@ -40,6 +40,7 @@ frame-executive = { version = "2.0.0-dev", default-features = false, path = "../ frame-benchmarking = { version = "2.0.0-dev", default-features = false, path = "../../../frame/benchmarking", optional = true } frame-support = { version = "2.0.0-dev", default-features = false, path = "../../../frame/support" } frame-system = { version = "2.0.0-dev", default-features = false, path = "../../../frame/system" } +frame-system-benchmarking = { version = "2.0.0-dev", default-features = false, path = "../../../frame/system/benchmarking", optional = true } frame-system-rpc-runtime-api = { version = "2.0.0-dev", default-features = false, path = "../../../frame/system/rpc/runtime-api/" } pallet-authority-discovery = { version = "2.0.0-dev", default-features = false, path = "../../../frame/authority-discovery" } pallet-authorship = { version = "2.0.0-dev", default-features = false, path = "../../../frame/authorship" } @@ -155,4 +156,5 @@ runtime-benchmarks = [ "pallet-vesting/runtime-benchmarks", "pallet-offences-benchmarking", "pallet-session-benchmarking", + "frame-system-benchmarking", ] diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 641cbf44eb..1458c00d81 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -902,9 +902,11 @@ impl_runtime_apis! { // we need these two lines below. use pallet_session_benchmarking::Module as SessionBench; use pallet_offences_benchmarking::Module as OffencesBench; + use frame_system_benchmarking::Module as SystemBench; impl pallet_session_benchmarking::Trait for Runtime {} impl pallet_offences_benchmarking::Trait for Runtime {} + impl frame_system_benchmarking::Trait for Runtime {} let mut batches = Vec::::new(); let params = (&pallet, &benchmark, &lowest_range_values, &highest_range_values, &steps, repeat); @@ -917,6 +919,7 @@ impl_runtime_apis! { add_benchmark!(params, batches, b"offences", OffencesBench::); add_benchmark!(params, batches, b"session", SessionBench::); add_benchmark!(params, batches, b"staking", Staking); + add_benchmark!(params, batches, b"system", SystemBench::); add_benchmark!(params, batches, b"timestamp", Timestamp); add_benchmark!(params, batches, b"treasury", Treasury); add_benchmark!(params, batches, b"utility", Utility); diff --git a/substrate/frame/benchmarking/src/analysis.rs b/substrate/frame/benchmarking/src/analysis.rs index fdf1210832..1d30b86fc9 100644 --- a/substrate/frame/benchmarking/src/analysis.rs +++ b/substrate/frame/benchmarking/src/analysis.rs @@ -138,6 +138,8 @@ impl Analysis { .collect(); let value_dists = results.iter().map(|(p, vs)| { + // Avoid divide by zero + if vs.len() == 0 { return (p.clone(), 0, 0) } let total = vs.iter() .fold(0u128, |acc, v| acc + *v); let mean = total / vs.len() as u128; @@ -178,13 +180,23 @@ impl std::fmt::Display for Analysis { writeln!(f, "\nData points distribution:")?; writeln!(f, "{} mean µs sigma µs %", self.names.iter().map(|p| format!("{:>5}", p)).collect::>().join(" "))?; for (param_values, mean, sigma) in value_dists.iter() { - writeln!(f, "{} {:>8} {:>8} {:>3}.{}%", - param_values.iter().map(|v| format!("{:>5}", v)).collect::>().join(" "), - ms(*mean), - ms(*sigma), - (sigma * 100 / mean), - (sigma * 1000 / mean % 10) - )?; + if *mean == 0 { + writeln!(f, "{} {:>8} {:>8} {:>3}.{}%", + param_values.iter().map(|v| format!("{:>5}", v)).collect::>().join(" "), + ms(*mean), + ms(*sigma), + "?", + "?" + )?; + } else { + writeln!(f, "{} {:>8} {:>8} {:>3}.{}%", + param_values.iter().map(|v| format!("{:>5}", v)).collect::>().join(" "), + ms(*mean), + ms(*sigma), + (sigma * 100 / mean), + (sigma * 1000 / mean % 10) + )?; + } } } if let Some(ref model) = self.model { diff --git a/substrate/frame/session/benchmarking/src/mock.rs b/substrate/frame/session/benchmarking/src/mock.rs index d488fe4eac..5a7082dabb 100644 --- a/substrate/frame/session/benchmarking/src/mock.rs +++ b/substrate/frame/session/benchmarking/src/mock.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! Mock file for staking fuzzing. +//! Mock file for session benchmarking. #![cfg(test)] diff --git a/substrate/frame/system/benchmarking/Cargo.toml b/substrate/frame/system/benchmarking/Cargo.toml new file mode 100644 index 0000000000..0518bd705d --- /dev/null +++ b/substrate/frame/system/benchmarking/Cargo.toml @@ -0,0 +1,37 @@ +[package] +name = "frame-system-benchmarking" +version = "2.0.0-dev" +authors = ["Parity Technologies "] +edition = "2018" +license = "GPL-3.0" +homepage = "https://substrate.dev" +repository = "https://github.com/paritytech/substrate/" +description = "FRAME System benchmarking" + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +codec = { package = "parity-scale-codec", version = "1.3.0", default-features = false } +sp-std = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/std" } +sp-runtime = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/runtime" } +frame-benchmarking = { version = "2.0.0-dev", default-features = false, path = "../../benchmarking" } +frame-system = { version = "2.0.0-dev", default-features = false, path = "../../system" } +frame-support = { version = "2.0.0-dev", default-features = false, path = "../../support" } +sp-core = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/core" } + +[dev-dependencies] +serde = { version = "1.0.101" } +sp-io ={ path = "../../../primitives/io", version = "2.0.0-dev"} + +[features] +default = ["std"] +std = [ + "codec/std", + "sp-runtime/std", + "sp-std/std", + "frame-benchmarking/std", + "frame-system/std", + "frame-support/std", + "sp-core/std", +] diff --git a/substrate/frame/system/benchmarking/src/lib.rs b/substrate/frame/system/benchmarking/src/lib.rs new file mode 100644 index 0000000000..b9b9619c09 --- /dev/null +++ b/substrate/frame/system/benchmarking/src/lib.rs @@ -0,0 +1,179 @@ +// Copyright 2019-2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +// Benchmarks for Utility Pallet + +#![cfg_attr(not(feature = "std"), no_std)] + +use codec::Encode; +use sp_std::vec; +use sp_std::prelude::*; +use sp_core::{ChangesTrieConfiguration, storage::well_known_keys}; +use sp_runtime::traits::Hash; +use frame_benchmarking::{benchmarks, account}; +use frame_support::storage::{self, StorageMap}; +use frame_system::{Module as System, Call, RawOrigin, DigestItemOf, AccountInfo}; + +mod mock; + +const SEED: u32 = 0; + +pub struct Module(System); +pub trait Trait: frame_system::Trait {} + +benchmarks! { + _ { } + + remark { + // # of Bytes + let b in 0 .. 16_384; + let remark_message = vec![1; b as usize]; + let caller = account("caller", 0, SEED); + }: _(RawOrigin::Signed(caller), remark_message) + + set_heap_pages { + // Heap page size + let i in 0 .. u32::max_value(); + }: _(RawOrigin::Root, i.into()) + + // `set_code` was not benchmarked because it is pretty hard to come up with a real + // Wasm runtime to test the upgrade with. But this is okay because we will make + // `set_code` take a full block anyway. + + set_code_without_checks { + // Version number + let b in 0 .. 16_384; + let code = vec![1; b as usize]; + }: _(RawOrigin::Root, code) + verify { + let current_code = storage::unhashed::get_raw(well_known_keys::CODE).ok_or("Code not stored.")?; + assert_eq!(current_code.len(), b as usize); + } + + set_changes_trie_config { + let d in 0 .. 1000; + + let digest_item = DigestItemOf::::Other(vec![]); + + for i in 0 .. d { + System::::deposit_log(digest_item.clone()); + } + let changes_trie_config = ChangesTrieConfiguration { + digest_interval: d, + digest_levels: d, + }; + }: _(RawOrigin::Root, Some(changes_trie_config)) + verify { + assert_eq!(System::::digest().logs.len(), (d + 1) as usize) + } + + set_storage { + let i in 1 .. 1000; + + // Set up i items to add + let mut items = Vec::new(); + for j in 0 .. i { + let hash = (i, j).using_encoded(T::Hashing::hash).as_ref().to_vec(); + items.push((hash.clone(), hash.clone())); + } + }: _(RawOrigin::Root, items) + verify { + let last_hash = (i, i - 1).using_encoded(T::Hashing::hash); + let value = storage::unhashed::get_raw(last_hash.as_ref()).ok_or("No value stored")?; + assert_eq!(value, last_hash.as_ref().to_vec()); + } + + kill_storage { + let i in 1 .. 1000; + + // Add i items to storage + let mut items = Vec::new(); + for j in 0 .. i { + let hash = (i, j).using_encoded(T::Hashing::hash).as_ref().to_vec(); + storage::unhashed::put_raw(&hash, &hash); + items.push(hash); + } + + // We will verify this value is removed + let last_hash = (i, i - 1).using_encoded(T::Hashing::hash); + let value = storage::unhashed::get_raw(last_hash.as_ref()).ok_or("No value stored")?; + assert_eq!(value, last_hash.as_ref().to_vec()); + + }: _(RawOrigin::Root, items) + verify { + assert_eq!(storage::unhashed::get_raw(last_hash.as_ref()), None); + } + + kill_prefix { + let p in 1 .. 1000; + + let prefix = p.using_encoded(T::Hashing::hash).as_ref().to_vec(); + // add p items that share a prefix + for i in 0 .. p { + let hash = (p, i).using_encoded(T::Hashing::hash).as_ref().to_vec(); + let key = [&prefix[..], &hash[..]].concat(); + storage::unhashed::put_raw(&key, &key); + } + + // We will verify this value is removed + let last_hash = (p, p - 1).using_encoded(T::Hashing::hash).as_ref().to_vec(); + let last_key = [&prefix[..], &last_hash[..]].concat(); + let value = storage::unhashed::get_raw(&last_key).ok_or("No value stored")?; + assert_eq!(value, last_key); + + }: _(RawOrigin::Root, prefix) + verify { + assert_eq!(storage::unhashed::get_raw(&last_key), None); + } + + suicide { + let n in 1 .. 1000; + let caller: T::AccountId = account("caller", 0, SEED); + let account_info = AccountInfo:: { + nonce: n.into(), + refcount: 0, + data: T::AccountData::default() + }; + frame_system::Account::::insert(&caller, account_info); + let new_account_info = System::::account(caller.clone()); + assert_eq!(new_account_info.nonce, n.into()); + }: _(RawOrigin::Signed(caller.clone())) + verify { + let account_info = System::::account(&caller); + assert_eq!(account_info.nonce, 0.into()); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mock::{new_test_ext, Test}; + use frame_support::assert_ok; + + #[test] + fn test_benchmarks() { + new_test_ext().execute_with(|| { + assert_ok!(test_benchmark_remark::()); + assert_ok!(test_benchmark_set_heap_pages::()); + assert_ok!(test_benchmark_set_code_without_checks::()); + assert_ok!(test_benchmark_set_changes_trie_config::()); + assert_ok!(test_benchmark_set_storage::()); + assert_ok!(test_benchmark_kill_storage::()); + assert_ok!(test_benchmark_kill_prefix::()); + assert_ok!(test_benchmark_suicide::()); + }); + } +} diff --git a/substrate/frame/system/benchmarking/src/mock.rs b/substrate/frame/system/benchmarking/src/mock.rs new file mode 100644 index 0000000000..1e72665c15 --- /dev/null +++ b/substrate/frame/system/benchmarking/src/mock.rs @@ -0,0 +1,82 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Mock file for system benchmarking. + +#![cfg(test)] + +use sp_runtime::traits::IdentityLookup; +use frame_support::{ + impl_outer_origin, + dispatch::{Dispatchable, DispatchInfo, PostDispatchInfo}, +}; + +type AccountId = u64; +type AccountIndex = u32; +type BlockNumber = u64; + +impl_outer_origin! { + pub enum Origin for Test where system = frame_system {} +} + +#[derive(Debug, codec::Encode, codec::Decode)] +pub struct Call; + +impl Dispatchable for Call { + type Origin = (); + type Trait = (); + type Info = DispatchInfo; + type PostInfo = PostDispatchInfo; + fn dispatch(self, _origin: Self::Origin) + -> sp_runtime::DispatchResultWithInfo { + panic!("Do not use dummy implementation for dispatch."); + } +} + +#[derive(Clone, Eq, PartialEq, Debug)] +pub struct Test; + +impl frame_system::Trait for Test { + type Origin = Origin; + type Index = AccountIndex; + type BlockNumber = BlockNumber; + type Call = Call; + type Hash = sp_core::H256; + type Hashing = ::sp_runtime::traits::BlakeTwo256; + type AccountId = AccountId; + type Lookup = IdentityLookup; + type Header = sp_runtime::testing::Header; + type Event = (); + type BlockHashCount = (); + type MaximumBlockWeight = (); + type DbWeight = (); + type BlockExecutionWeight = (); + type ExtrinsicBaseWeight = (); + type AvailableBlockRatio = (); + type MaximumBlockLength = (); + type Version = (); + type ModuleToIndex = (); + type AccountData = (); + type OnNewAccount = (); + type OnKilledAccount = (); +} + +impl crate::Trait for Test {} + +pub fn new_test_ext() -> sp_io::TestExternalities { + let t = frame_system::GenesisConfig::default().build_storage::().unwrap(); + sp_io::TestExternalities::new(t) +} diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 234e427d3a..5a3097e62e 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -470,7 +470,6 @@ decl_error! { /// /// Either calling `Core_version` or decoding `RuntimeVersion` failed. FailedToExtractRuntimeVersion, - /// Suicide called when the account has non-default composite data. NonDefaultComposite, /// There is a non-zero reference count preventing the account from being purged. @@ -629,10 +628,8 @@ decl_module! { /// data is equal to its default value. /// /// # - /// - `O(K)` with `K` being complexity of `on_killed_account` + /// - `O(1)` /// - 1 storage read and deletion. - /// - 1 call to `on_killed_account` callback with unknown complexity `K` - /// - 1 event. /// # #[weight = (25_000_000, DispatchClass::Operational)] fn suicide(origin) {