Frame System Benchmarking (#5834)

* Frame System Benchmarking

* Add to substrate node, avoid divide by zero errors in analysis

* reduce features

* some fixes

* copy pasta

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",

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"
-

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",
 ]

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::<BenchmarkBatch>::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::<Runtime>);
 			add_benchmark!(params, batches, b"session", SessionBench::<Runtime>);
 			add_benchmark!(params, batches, b"staking", Staking);
+			add_benchmark!(params, batches, b"system", SystemBench::<Runtime>);
 			add_benchmark!(params, batches, b"timestamp", Timestamp);
 			add_benchmark!(params, batches, b"treasury", Treasury);
 			add_benchmark!(params, batches, b"utility", Utility);

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::<Vec<_>>().join(" "))?;
 			for (param_values, mean, sigma) in value_dists.iter() {
-				writeln!(f, "{}  {:>8}  {:>8}  {:>3}.{}%",
-					param_values.iter().map(|v| format!("{:>5}", v)).collect::<Vec<_>>().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::<Vec<_>>().join(" "),
+						ms(*mean),
+						ms(*sigma),
+						"?",
+						"?"
+					)?;
+				} else {
+					writeln!(f, "{}  {:>8}  {:>8}  {:>3}.{}%",
+						param_values.iter().map(|v| format!("{:>5}", v)).collect::<Vec<_>>().join(" "),
+						ms(*mean),
+						ms(*sigma),
+						(sigma * 100 / mean),
+						(sigma * 1000 / mean % 10)
+					)?;
+				}
 			}
 		}
 		if let Some(ref model) = self.model {

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 <http://www.gnu.org/licenses/>.
 
-//! Mock file for staking fuzzing.
+//! Mock file for session benchmarking.
 
 #![cfg(test)]
 

substrate/frame/system/benchmarking/Cargo.toml

@@ -0,0 +1,37 @@
+[package]
+name = "frame-system-benchmarking"
+version = "2.0.0-dev"
+authors = ["Parity Technologies <admin@parity.io>"]
+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",
+]

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 <http://www.gnu.org/licenses/>.
+
+// 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<T: Trait>(System<T>);
+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::<T>::Other(vec![]);
+
+		for i in 0 .. d {
+			System::<T>::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::<T>::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::<T::Index, T::AccountData> {
+			nonce: n.into(),
+			refcount: 0,
+			data: T::AccountData::default()
+		};
+		frame_system::Account::<T>::insert(&caller, account_info);
+		let new_account_info = System::<T>::account(caller.clone());
+		assert_eq!(new_account_info.nonce, n.into());
+	}: _(RawOrigin::Signed(caller.clone()))
+	verify {
+		let account_info = System::<T>::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::<Test>());
+			assert_ok!(test_benchmark_set_heap_pages::<Test>());
+			assert_ok!(test_benchmark_set_code_without_checks::<Test>());
+			assert_ok!(test_benchmark_set_changes_trie_config::<Test>());
+			assert_ok!(test_benchmark_set_storage::<Test>());
+			assert_ok!(test_benchmark_kill_storage::<Test>());
+			assert_ok!(test_benchmark_kill_prefix::<Test>());
+			assert_ok!(test_benchmark_suicide::<Test>());
+		});
+	}
+}

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 <http://www.gnu.org/licenses/>.
+
+//! 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<Self::PostInfo> {
+			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<Self::AccountId>;
+	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::<Test>().unwrap();
+	sp_io::TestExternalities::new(t)
+}

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.
 		///
 		/// # <weight>
-		/// - `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>
 		#[weight = (25_000_000, DispatchClass::Operational)]
 		fn suicide(origin) {