Add StorageValue::append and speed-up deposit_event (#2282)

* Adds deposit event benchmark

* Add `StorageValue::append`

`StorageValue::append` can be used by types that implement `EncodeAppend` to speed-up situations where you just want to append
an item to storage without wanting to decode all previous items.

* Stay at 100 events

* Fixes compilation

* Use correct year and increase spec version

substrate/srml/support/Cargo.toml

@@ -8,7 +8,7 @@ edition = "2018"
 hex-literal = { version = "0.1.0", optional = true }
 serde = { version = "1.0", optional = true }
 serde_derive = { version = "1.0", optional = true }
-parity-codec = { version = "3.4", default-features = false, features = ["derive"] }
+parity-codec = { version = "3.5.1", default-features = false, features = ["derive"] }
 srml-metadata = { path = "../metadata", default-features = false }
 sr-std = { path = "../../core/sr-std", default-features = false }
 runtime_io = { package = "sr-io", path = "../../core/sr-io", default-features = false }

substrate/srml/support/src/storage/generator.rs

@@ -97,6 +97,12 @@ pub trait Storage {
 
 	/// Take a value from storage, deleting it after reading.
 	fn take_or_default<T: codec::Decode + Default>(&self, key: &[u8]) -> T { self.take(key).unwrap_or_default() }
+
+	/// Get a Vec of bytes from storage.
+	fn get_raw(&self, key: &[u8]) -> Option<Vec<u8>>;
+
+	/// Put a raw byte slice into storage.
+	fn put_raw(&self, key: &[u8], value: &[u8]);
 }
 
 // We use a construct like this during when genesis storage is being built.
@@ -117,6 +123,14 @@ impl<S: sr_primitives::BuildStorage> Storage for (crate::rstd::cell::RefCell<&mu
 	fn kill(&self, key: &[u8]) {
 		UnhashedStorage::kill(self, &S::hash(key))
 	}
+
+	fn get_raw(&self, key: &[u8]) -> Option<Vec<u8>> {
+		UnhashedStorage::get_raw(self, key)
+	}
+
+	fn put_raw(&self, key: &[u8], value: &[u8]) {
+		UnhashedStorage::put_raw(self, key, value)
+	}
 }
 
 /// A strongly-typed value kept in storage.
@@ -150,6 +164,20 @@ pub trait StorageValue<T: codec::Codec> {
 	fn kill<S: Storage>(storage: &S) {
 		storage.kill(Self::key())
 	}
+
+	/// Append the given items to the value in the storage.
+	///
+	/// `T` is required to implement `codec::EncodeAppend`.
+	fn append<S: Storage, I: codec::Encode>(
+		items: &[I], storage: &S
+	) -> Result<(), &'static str> where T: codec::EncodeAppend<Item=I> {
+		let new_val = <T as codec::EncodeAppend>::append(
+			storage.get_raw(Self::key()).unwrap_or_default(),
+			items,
+		).ok_or_else(|| "Could not append given item")?;
+		storage.put_raw(Self::key(), &new_val);
+		Ok(())
+	}
 }
 
 /// A strongly-typed list in storage.
@@ -576,6 +604,14 @@ mod tests {
 		fn kill(&self, key: &[u8]) {
 			self.borrow_mut().remove(key);
 		}
+
+		fn put_raw(&self, key: &[u8], value: &[u8]) {
+			self.borrow_mut().insert(key.to_owned(), value.to_owned());
+		}
+
+		fn get_raw(&self, key: &[u8]) -> Option<Vec<u8>> {
+			self.borrow().get(key).cloned()
+		}
 	}
 
 	storage_items! {

substrate/srml/support/src/storage/mod.rs

@@ -19,7 +19,7 @@
 use crate::rstd::prelude::*;
 use crate::rstd::borrow::Borrow;
 use runtime_io::{self, twox_128};
-use crate::codec::{Codec, Encode, Decode, KeyedVec, Input};
+use crate::codec::{Codec, Encode, Decode, KeyedVec, Input, EncodeAppend};
 
 #[macro_use]
 pub mod generator;
@@ -153,6 +153,14 @@ impl crate::GenericStorage for RuntimeStorage {
 	fn take<T: Decode>(&self, key: &[u8]) -> Option<T> {
 		take(key)
 	}
+
+	fn get_raw(&self, key: &[u8]) -> Option<Vec<u8>> {
+		get_raw(key)
+	}
+
+	fn put_raw(&self, key: &[u8], value: &[u8]) {
+		put_raw(key, value)
+	}
 }
 
 impl crate::GenericUnhashedStorage for RuntimeStorage {
@@ -184,6 +192,14 @@ impl crate::GenericUnhashedStorage for RuntimeStorage {
 	fn take<T: Decode>(&self, key: &[u8]) -> Option<T> {
 		unhashed::take(key)
 	}
+
+	fn get_raw(&self, key: &[u8]) -> Option<Vec<u8>> {
+		unhashed::get_raw(key)
+	}
+
+	fn put_raw(&self, key: &[u8], value: &[u8]) {
+		unhashed::put_raw(key, value)
+	}
 }
 
 /// A trait for working with macro-generated storage values under the substrate storage API.
@@ -211,6 +227,12 @@ pub trait StorageValue<T: Codec> {
 
 	/// Take a value from storage, removing it afterwards.
 	fn take() -> Self::Query;
+
+	/// Append the given item to the value in the storage.
+	///
+	/// `T` is required to implement `codec::EncodeAppend`.
+	fn append<I: Encode>(items: &[I]) -> Result<(), &'static str>
+		where T: EncodeAppend<Item=I>;
 }
 
 impl<T: Codec, U> StorageValue<T> for U where U: generator::StorageValue<T> {
@@ -237,6 +259,11 @@ impl<T: Codec, U> StorageValue<T> for U where U: generator::StorageValue<T> {
 	fn take() -> Self::Query {
 		U::take(&RuntimeStorage)
 	}
+	fn append<I: Encode>(items: &[I]) -> Result<(), &'static str>
+		where T: EncodeAppend<Item=I>
+	{
+		U::append(items, &RuntimeStorage)
+	}
 }
 
 /// A strongly-typed list in storage.
@@ -560,7 +587,7 @@ pub trait StorageVec {
 /// child storage NOTE could replace unhashed by having only one kind of storage (root being null storage
 /// key (storage_key can become Option<&[u8]>).
 /// This module is a currently only a variant of unhashed with additional `storage_key`.
-/// Note that `storage_key` must be unique and strong (strong in the sense of being long enough to 
+/// Note that `storage_key` must be unique and strong (strong in the sense of being long enough to
 /// avoid collision from a resistant hash function (which unique implies)).
 pub mod child {
 	use super::{runtime_io, Codec, Decode, Vec, IncrementalChildInput};
@@ -632,7 +659,7 @@ pub mod child {
 		runtime_io::read_child_storage(storage_key, key, &mut [0;0][..], 0).is_some()
 	}
 
-	/// Remove all `storage_key` key/values 
+	/// Remove all `storage_key` key/values
 	pub fn kill_storage(storage_key: &[u8]) {
 		runtime_io::kill_child_storage(storage_key)
 	}

substrate/srml/support/src/storage/unhashed/generator.rs

@@ -55,6 +55,12 @@ pub trait UnhashedStorage {
 
 	/// Take a value from storage, deleting it after reading.
 	fn take_or_default<T: codec::Decode + Default>(&self, key: &[u8]) -> T { self.take(key).unwrap_or_default() }
+
+	/// Get a Vec of bytes from storage.
+	fn get_raw(&self, key: &[u8]) -> Option<Vec<u8>>;
+
+	/// Put a raw byte slice into storage.
+	fn put_raw(&self, key: &[u8], value: &[u8]);
 }
 
 // We use a construct like this during when genesis storage is being built.
@@ -82,6 +88,14 @@ impl<H> UnhashedStorage for (crate::rstd::cell::RefCell<&mut sr_primitives::Stor
 			!key.starts_with(prefix)
 		})
 	}
+
+	fn get_raw(&self, key: &[u8]) -> Option<Vec<u8>> {
+		self.0.borrow().get(key).cloned()
+	}
+
+	fn put_raw(&self, key: &[u8], value: &[u8]) {
+		self.0.borrow_mut().insert(key.to_vec(), value.to_vec());
+	}
 }
 
 /// An implementation of a map with a two keys.

substrate/srml/system/Cargo.toml

@@ -9,13 +9,16 @@ hex-literal = "0.1.0"
 serde = { version = "1.0", optional = true }
 serde_derive = { version = "1.0", optional = true }
 safe-mix = { version = "1.0", default-features = false}
-parity-codec = { version = "3.3", default-features = false, features = ["derive"] }
+parity-codec = { version = "3.5", default-features = false, features = ["derive"] }
 substrate-primitives = { path = "../../core/primitives", default-features = false }
 rstd = { package = "sr-std", path = "../../core/sr-std", default-features = false }
 runtime_io = { package = "sr-io", path = "../../core/sr-io", default-features = false }
 primitives = { package = "sr-primitives", path = "../../core/sr-primitives", default-features = false }
 srml-support = { path = "../support", default-features = false }
 
+[dev-dependencies]
+criterion = "0.2"
+
 [features]
 default = ["std"]
 std = [
@@ -29,3 +32,7 @@ std = [
 	"srml-support/std",
 	"primitives/std",
 ]
+
+[[bench]]
+name = "bench"
+harness = false

substrate/srml/system/benches/bench.rs

@@ -0,0 +1,99 @@
+// Copyright 2019 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/>.
+
+use criterion::{Criterion, criterion_group, criterion_main, black_box};
+use srml_system as system;
+use srml_support::{decl_module, decl_event, impl_outer_origin, impl_outer_event};
+use runtime_io::{with_externalities, Blake2Hasher};
+use substrate_primitives::H256;
+use primitives::{
+	BuildStorage, traits::{BlakeTwo256, IdentityLookup},
+	testing::{Digest, DigestItem, Header},
+};
+
+mod module {
+	use super::*;
+
+	pub trait Trait: system::Trait {
+		type Event: From<Event> + Into<<Self as system::Trait>::Event>;
+	}
+
+	decl_module! {
+		pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+			pub fn deposit_event() = default;
+		}
+	}
+
+	decl_event!(
+		pub enum Event {
+			Complex(Vec<u8>, u32, u16, u128),
+		}
+	);
+}
+
+impl_outer_origin!{
+	pub enum Origin for Runtime {}
+}
+
+impl_outer_event! {
+	pub enum Event for Runtime {
+		module,
+	}
+}
+
+#[derive(Clone, Eq, PartialEq)]
+pub struct Runtime;
+impl system::Trait for Runtime {
+	type Origin = Origin;
+	type Index = u64;
+	type BlockNumber = u64;
+	type Hash = H256;
+	type Hashing = BlakeTwo256;
+	type Digest = Digest;
+	type AccountId = u64;
+	type Lookup = IdentityLookup<Self::AccountId>;
+	type Header = Header;
+	type Event = Event;
+	type Log = DigestItem;
+}
+
+impl module::Trait for Runtime {
+	type Event = Event;
+}
+
+fn new_test_ext() -> runtime_io::TestExternalities<Blake2Hasher> {
+	system::GenesisConfig::<Runtime>::default().build_storage().unwrap().0.into()
+}
+
+fn deposit_events(n: usize) {
+	let mut t = new_test_ext();
+	with_externalities(&mut t, || {
+		for _ in 0..n {
+			module::Module::<Runtime>::deposit_event(
+				module::Event::Complex(vec![1, 2, 3], 2, 3, 899)
+			);
+		}
+	});
+}
+
+fn sr_system_benchmark(c: &mut Criterion) {
+	c.bench_function("deposit 100 events", |b| {
+		b.iter(|| deposit_events(black_box(100)))
+	});
+}
+
+criterion_group!(benches, sr_system_benchmark);
+criterion_main!(benches);

substrate/srml/system/src/lib.rs

@@ -15,46 +15,46 @@
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
 //! # System module
-//! 
+//!
 //! The system module provides low-level access to core types and cross-cutting utilities.
 //! It acts as the base layer for other SRML modules to interact with the Substrate framework components.
 //! To use it in your module, you should ensure your module's trait implies the system [`Trait`].
-//! 
+//!
 //! ## Overview
-//! 
+//!
 //! The system module defines the core data types used in a Substrate runtime.
 //! It also provides several utility functions (see [`Module`]) for other runtime modules.
-//! 
-//! In addition, it manages the storage items for extrinsics data, indexes, event record and digest items, 
+//!
+//! In addition, it manages the storage items for extrinsics data, indexes, event record and digest items,
 //! among other things that support the execution of the current block.
-//! 
+//!
 //! It also handles low level tasks like depositing logs, basic set up and take down of
 //! temporary storage entries and access to previous block hashes.
-//! 
+//!
 //! ## Interface
-//! 
+//!
 //! ### Dispatchable functions
-//! 
+//!
 //! The system module does not implement any dispatchable functions.
-//! 
+//!
 //! ### Public functions
-//! 
+//!
 //! All public functions are available as part of the [`Module`] type.
-//! 
+//!
 //! ## Usage
-//! 
+//!
 //! ### Prerequisites
-//! 
+//!
 //! Import the system module and derive your module's configuration trait from the system trait.
-//! 
+//!
 //! ### Example - Get random seed and extrinsic count for the current block
-//! 
+//!
 //! ```
 //! use srml_support::{decl_module, dispatch::Result};
 //! use srml_system::{self as system, ensure_signed};
-//! 
+//!
 //! pub trait Trait: system::Trait {}
-//! 
+//!
 //! decl_module! {
 //! 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
 //! 		pub fn system_module_example(origin) -> Result {
@@ -135,12 +135,12 @@ pub trait Trait: 'static + Eq + Clone {
 	type BlockNumber:
 		Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleArithmetic + Default + Bounded + Copy
 		+ rstd::hash::Hash;
-	
+
 	/// The output of the `Hashing` function.
 	type Hash:
 		Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleBitOps + Default + Copy + CheckEqual
 		+ rstd::hash::Hash + AsRef<[u8]> + AsMut<[u8]>;
-	
+
 	/// The hashing system (algorithm) being used in the runtime (e.g. Blake2).
 	type Hashing: Hash<Output = Self::Hash>;
 
@@ -180,9 +180,16 @@ decl_module! {
 		pub fn deposit_event(event: T::Event) {
 			let extrinsic_index = Self::extrinsic_index();
 			let phase = extrinsic_index.map_or(Phase::Finalization, |c| Phase::ApplyExtrinsic(c));
-			let mut events = Self::events();
-			events.push(EventRecord { phase, event });
-			<Events<T>>::put(events);
+			let event = EventRecord { phase, event };
+
+			// Appending can only fail if `Events<T>` can not be decoded or
+			// when we try to insert more than `u32::max_value()` events.
+			// If one of these conditions is met, we just insert the new event.
+			let events = [event];
+			if <Events<T>>::append(&events).is_err() {
+				let [event] = events;
+				<Events<T>>::put(vec![event]);
+			}
 		}
 	}
 }
@@ -599,7 +606,10 @@ mod tests {
 			System::note_finished_extrinsics();
 			System::deposit_event(1u16);
 			System::finalize();
-			assert_eq!(System::events(), vec![EventRecord { phase: Phase::Finalization, event: 1u16 }]);
+			assert_eq!(
+				System::events(),
+				vec![EventRecord { phase: Phase::Finalization, event: 1u16 }]
+			);
 
 			System::initialize(&2, &[0u8; 32].into(), &[0u8; 32].into());
 			System::deposit_event(42u16);