Unverified Commit d4cbfcea authored by Michael Müller's avatar Michael Müller Committed by GitHub

Add new ops for storage2/vec (#453)

* Fix typo

* [core] Add vec.set()

* [core] Add vec.clear()

* [core] Add benchmarks for vec.put() and vec.clear()

* [core] Minor improvements

* [core] Add user comment

* [core] Improve style

* [core] Add worst case benches

* [core] Include comments from review

* [core] Apply cargo fmt

* [core] Remove unnecessary variable

* [core] Include put worst case bench

* [core] Rename benches to lazy vs. cached

* [core] Reduce benchmark overhead by preventing cache writes

* [core] Add black boxes to prevent over optimization

* [core] improve benchmarks for new storage vector operations (#457)

* [core] optimize LazyIndexMap::put

* [core] put some forgotten black_box instances

* [core] further minor improvements to the benchmarks

* [core] Add criterion dependency workaround

* [core] Change vec.set() to return Result<(), IndexOutOfBounds>

* [core] Remove outdated comment

* [core] Minor fixes

* [core] Minor test fixes

* Apply suggestions from code review
Co-authored-by: default avatarHero Bird <robin.freyler@gmail.com>
Co-authored-by: default avatarHero Bird <robin.freyler@gmail.com>
parent f5b3c481
Pipeline #97684 failed with stages
in 6 minutes and 45 seconds
......@@ -41,12 +41,19 @@ blake2 = { version = "0.9", optional = true }
rand = { version = "0.7", default-features = false, features = ["alloc"], optional = true }
scale-info = { version = "0.2", default-features = false, features = ["derive"], optional = true }
# Workaround: we actually just need criterion as a dev-dependency, but
# there is an issue with a doubly included std lib when executing
# `cargo check --no-default-features --target wasm32-unknown-unknown`.
# We haven't found a better solution yet.
criterion = { version = "0.3", optional = true }
[dev-dependencies]
itertools = "0.9"
[features]
default = ["std"]
std = [
"criterion",
"ink_abi",
"ink_abi/std",
"ink_alloc/std",
......@@ -63,3 +70,7 @@ std = [
"sha3",
"blake2",
]
[[bench]]
name = "bench"
harness = false
// Copyright 2019-2020 Parity Technologies (UK) Ltd.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
use criterion::{
black_box,
criterion_group,
criterion_main,
BenchmarkId,
Criterion,
};
use ink_core::{
env,
storage2::{
collections::Vec as StorageVec,
traits::{
KeyPtr,
SpreadLayout,
},
},
};
use ink_primitives::Key;
criterion_group!(
populated_cache,
bench_clear_populated_cache,
bench_put_populated_cache,
);
criterion_group!(empty_cache, bench_clear_empty_cache, bench_put_empty_cache,);
criterion_main!(populated_cache, empty_cache,);
/// Returns some test values for use in benchmarks.
#[rustfmt::skip]
fn test_values() -> &'static [u8] {
&[
b'0', b'1', b'2', b'3', b'4', b'5', b'6', b'7', b'8', b'9',
b'A', b'B', b'C', b'D', b'E', b'F'
]
}
/// Creates a storage vector from the given slice.
fn storage_vec_from_slice(slice: &[u8]) -> StorageVec<u8> {
slice.iter().copied().collect::<StorageVec<u8>>()
}
/// Creates a storage vector and pushes it to the contract storage.
fn push_storage_vec() {
let vec = storage_vec_from_slice(test_values());
let root_key = Key::from([0x00; 32]);
SpreadLayout::push_spread(&vec, &mut KeyPtr::from(root_key));
}
/// Pulls a lazily loading storage vector instance from the contract storage.
fn pull_storage_vec() -> StorageVec<u8> {
let root_key = Key::from([0x00; 32]);
<StorageVec<u8> as SpreadLayout>::pull_spread(&mut KeyPtr::from(root_key))
}
mod populated_cache {
use super::*;
pub fn clear(test_values: &[u8]) {
let mut vec = storage_vec_from_slice(&test_values);
black_box(vec.clear());
}
pub fn pop_all(test_values: &[u8]) {
let mut vec = storage_vec_from_slice(&test_values);
while let Some(ignored) = black_box(vec.pop()) {
black_box(ignored);
}
}
pub fn set(test_values: &[u8]) {
let mut vec = storage_vec_from_slice(&test_values);
for (index, _value) in test_values.iter().enumerate() {
black_box(vec.set(index as u32, b'X'));
}
}
pub fn get_mut(test_values: &[u8]) {
let mut vec = storage_vec_from_slice(&test_values);
for (index, _value) in test_values.iter().enumerate() {
*black_box(vec.get_mut(index as u32).unwrap()) = b'X';
}
}
}
fn bench_put_populated_cache(c: &mut Criterion) {
let mut group = c.benchmark_group("Compare: `set` and `get_mut` (populated cache)");
group.bench_with_input(BenchmarkId::new("set", 0), test_values(), |b, i| {
b.iter(|| populated_cache::set(i))
});
group.bench_with_input(BenchmarkId::new("get_mut", 0), test_values(), |b, i| {
b.iter(|| populated_cache::get_mut(i))
});
group.finish();
}
fn bench_clear_populated_cache(c: &mut Criterion) {
let mut group = c.benchmark_group("Compare: `clear` and `pop_all` (populated cache)");
let test_values = [b'A', b'B', b'C', b'D', b'E', b'F'];
group.bench_with_input("clear", &test_values, |b, i| {
b.iter(|| populated_cache::clear(i))
});
group.bench_with_input("pop_all", &test_values, |b, i| {
b.iter(|| populated_cache::pop_all(i))
});
group.finish();
}
mod empty_cache {
use super::*;
/// In this case we lazily load the vec from storage using `pull_spread`.
/// This will just load lazily and won't pull anything from the storage.
pub fn clear() {
push_storage_vec();
let mut vec = pull_storage_vec();
black_box(vec.clear());
}
/// In this case we lazily load the vec from storage using `pull_spread`.
/// This will just load lazily and won't pull anything from the storage.
/// `pop` will then result in loading from storage.
pub fn pop_all() {
push_storage_vec();
let mut vec = pull_storage_vec();
while let Some(ignored) = black_box(vec.pop()) {
black_box(ignored);
}
}
/// In this case we lazily load the vec from storage using `pull_spread`.
/// This will just load lazily and won't pull anything from the storage.
/// The `deref` will then load from storage.
pub fn get_mut() {
push_storage_vec();
let mut vec = pull_storage_vec();
for index in 0..vec.len() {
*black_box(vec.get_mut(index).unwrap()) = b'X';
}
}
/// In this case we lazily load the vec from storage using `pull_spread`.
/// This will just load lazily and won't pull anything from the storage.
/// The `vec.set()` will not load anything from storage.
pub fn set() {
push_storage_vec();
let mut vec = pull_storage_vec();
for index in 0..vec.len() {
black_box(vec.set(index, b'X'));
}
}
}
/// In this case we lazily instantiate a `StorageVec` by first `create_and_store`-ing
/// into the contract storage. We then load the vec from storage lazily in each
/// benchmark iteration.
fn bench_clear_empty_cache(c: &mut Criterion) {
let _ = env::test::run_test::<env::DefaultEnvTypes, _>(|_| {
let mut group = c.benchmark_group("Compare: `clear` and `pop_all` (empty cache)");
group.bench_function("clear", |b| b.iter(|| empty_cache::clear()));
group.bench_function("pop_all", |b| b.iter(|| empty_cache::pop_all()));
group.finish();
Ok(())
})
.unwrap();
}
/// In this case we lazily instantiate a `StorageVec` by first `create_and_store`-ing
/// into the contract storage. We then load the vec from storage lazily in each
/// benchmark iteration.
fn bench_put_empty_cache(c: &mut Criterion) {
let _ = env::test::run_test::<env::DefaultEnvTypes, _>(|_| {
let mut group = c.benchmark_group("Compare: `set` and `get_mut` (empty cache)");
group.bench_function("set", |b| b.iter(|| empty_cache::set()));
group.bench_function("get_mut", |b| b.iter(|| empty_cache::get_mut()));
group.finish();
Ok(())
})
.unwrap();
}
......@@ -61,6 +61,10 @@ where
elems: LazyIndexMap<T>,
}
/// The index is out of the bounds of this vector.
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
pub struct IndexOutOfBounds;
impl<T> Default for Vec<T>
where
T: PackedLayout,
......@@ -144,7 +148,7 @@ where
IterMut::new(self)
}
/// Returns the index if it is witihn bounds or `None` otherwise.
/// Returns the index if it is within bounds or `None` otherwise.
fn within_bounds(&self, index: u32) -> Option<u32> {
if index < self.len() {
return Some(index)
......@@ -303,4 +307,35 @@ where
*self.len = last_index;
Some(())
}
/// Sets the elements at the given index to the new value.
///
/// Won't return the old element back to the caller.
/// Prefer this operation over other method of overriding an element
/// in the storage vector since this is more efficient.
#[inline]
pub fn set(&mut self, index: u32, new_value: T) -> Result<(), IndexOutOfBounds> {
if self.within_bounds(index).is_none() {
return Err(IndexOutOfBounds)
}
self.elems.put(index, Some(new_value));
Ok(())
}
/// Removes all elements from this vector.
///
/// # Note
///
/// Use this method to clear the vector instead of e.g. iterative `pop()`.
/// This method performs significantly better and does not actually read
/// any of the elements (whereas `pop()` does).
pub fn clear(&mut self) {
if self.is_empty() {
return
}
for index in 0..self.len() {
self.elems.put(index, None);
}
*self.len = 0;
}
}
......@@ -15,9 +15,12 @@
use super::Vec as StorageVec;
use crate::{
env,
storage2::traits::{
KeyPtr,
SpreadLayout,
storage2::{
collections::vec::IndexOutOfBounds,
traits::{
KeyPtr,
SpreadLayout,
},
},
};
use ink_primitives::Key;
......@@ -392,3 +395,32 @@ fn spread_layout_clear_works() {
})
.unwrap()
}
#[test]
fn set_works() {
let mut vec = vec_from_slice(&[b'a', b'b', b'c', b'd']);
let _ = vec.set(0, b'x').unwrap();
let expected = vec_from_slice(&[b'x', b'b', b'c', b'd']);
assert_eq!(vec, expected);
}
#[test]
fn set_fails_when_index_oob() {
let mut vec = vec_from_slice(&[b'a']);
let res = vec.set(1, b'x');
assert_eq!(res, Err(IndexOutOfBounds));
}
#[test]
fn clear_works_on_filled_vec() {
let mut vec = vec_from_slice(&[b'a', b'b', b'c', b'd']);
vec.clear();
assert!(vec.is_empty());
}
#[test]
fn clear_works_on_empty_vec() {
let mut vec = vec_from_slice(&[]);
vec.clear();
assert!(vec.is_empty());
}
......@@ -192,8 +192,17 @@ impl<V> LazyIndexMap<V> {
/// - If the lazy chunk is in an invalid state that forbids interaction.
/// - If the decoding of the old element at the given index failed.
pub fn put(&mut self, index: Index, new_value: Option<V>) {
self.entries_mut()
.insert(index, Box::new(Entry::new(new_value, EntryState::Mutated)));
use ink_prelude::collections::btree_map::Entry as BTreeMapEntry;
match self.entries_mut().entry(index) {
BTreeMapEntry::Occupied(mut occupied) => {
// We can re-use the already existing boxed `Entry` and simply
// swap the underlying values.
occupied.get_mut().put(new_value);
}
BTreeMapEntry::Vacant(vacant) => {
vacant.insert(Box::new(Entry::new(new_value, EntryState::Mutated)));
}
}
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment