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.

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

Implement Entry API for storage2::LazyHashMap #480

Merged
merged 30 commits into from Sep 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
98fe682
[core] Rename Entry to Internal Entry
cmichi Jul 22, 2020
660adef
[core] Add Entry API for storage2::LazyHashMap
cmichi Jul 22, 2020
e3149a0
[core] Add storage2::LazyHashMap::len()
cmichi Jul 22, 2020
94775ab
[core] Migrate tests to use storage2::LazyHashMap::len()
cmichi Jul 22, 2020
0384579
[core] Implement FromIterator and Extend for storage2::LazyHashMap
cmichi Jul 23, 2020
7460c5d
[core] Implement macro to generate LazyHashMap + HashMap Entry API tests
cmichi Jul 23, 2020
5dc160f
[core] Remove redundant storage2::HashMap Entry API tests
cmichi Jul 23, 2020
3f7c8d6
[core] Make storage2::HashMap Entry API use storage2::LazyHashMap's E…
cmichi Jul 24, 2020
0e7ebff
[core] Move parameterized Entry API tests into separate file
cmichi Jul 24, 2020
465f73c
[core] Rename InternalEntry to StorageEntry
cmichi Jul 24, 2020
e3a561c
[core] Make lazy_hmap module public
cmichi Jul 24, 2020
6748f60
[core] Generate Entry API benches for LazyHashMap and HashMap from macro
cmichi Jul 24, 2020
2a94f64
[core] Minor streamlining
cmichi Jul 24, 2020
5fe1bec
[core] Display hashmap variant in benchmark description
cmichi Jul 24, 2020
6be446a
[core] Fix comment
cmichi Jul 24, 2020
6bbc5e4
[core] Fix typos
cmichi Jul 31, 2020
f21e3d0
[core] Make more use of BTreeMap Entry API
cmichi Jul 31, 2020
5a39bd3
[core] Replace unwrap with expect
cmichi Jul 31, 2020
b0471e0
[core] Improve comment
cmichi Jul 31, 2020
093a814
[core] Handle loading from storage
cmichi Jul 31, 2020
dc33fc3
[core] Restrict unsafe
cmichi Jul 31, 2020
bc8a1ad
[core] Less ops for case "entry not in cache, but in storage"
cmichi Aug 28, 2020
7292c29
[core] Rename len()
cmichi Aug 31, 2020
c45ad81
[core] Fix typo
cmichi Aug 31, 2020
5a6cefb
[core] Fix visibility
cmichi Aug 31, 2020
debb98e
[core] Address comments
cmichi Sep 1, 2020
f5d011d
[core] Add test to verify that cache is marked as 'Mutated'
cmichi Sep 2, 2020
d64972c
Merge branch 'master' into cmichi-implement-lazyhashmap-entry-api
cmichi Sep 2, 2020
ee66762
[core] Shorten code with utility function
cmichi Sep 2, 2020
758643c
[core] Improve naming
cmichi Sep 2, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
453 changes: 263 additions & 190 deletions core/benches/bench_hashmap.rs

Large diffs are not rendered by default.

164 changes: 71 additions & 93 deletions core/src/storage2/collections/hashmap/mod.rs
Expand Up @@ -38,7 +38,12 @@ use crate::{
},
storage2::{
collections::Stash,
lazy::LazyHashMap,
lazy::lazy_hmap::{
Entry as LazyEntry,
LazyHashMap,
OccupiedEntry as LazyOccupiedEntry,
VacantEntry as LazyVacantEntry,
},
traits::PackedLayout,
},
};
Expand Down Expand Up @@ -101,51 +106,43 @@ struct ValueEntry<V> {
key_index: KeyIndex,
}

/// A vacant entry with previous and next vacant indices.
pub struct OccupiedEntry<'a, K, V, H = Blake2x256Hasher>
/// An occupied entry that holds the value.
pub struct OccupiedEntry<'a, K, V>
where
K: Ord + Clone + PackedLayout,
V: PackedLayout,
H: Hasher,
Key: From<<H as Hasher>::Output>,
{
/// A reference to the used `HashMap` instance.
base: &'a mut HashMap<K, V, H>,
/// The index of the key associated with this value.
key_index: KeyIndex,
/// The key stored in this entry.
key: K,
/// A reference to the `Stash` instance, containing the keys.
keys: &'a mut Stash<K>,
/// The `LazyHashMap::OccupiedEntry`.
values_entry: LazyOccupiedEntry<'a, K, ValueEntry<V>>,
}

/// A vacant entry with previous and next vacant indices.
pub struct VacantEntry<'a, K, V, H = Blake2x256Hasher>
pub struct VacantEntry<'a, K, V>
where
K: Ord + Clone + PackedLayout,
V: PackedLayout,
H: Hasher,
Key: From<<H as Hasher>::Output>,
{
/// A reference to the used `HashMap` instance.
base: &'a mut HashMap<K, V, H>,
/// The key stored in this entry.
key: K,
/// A reference to the `Stash` instance, containing the keys.
keys: &'a mut Stash<K>,
/// The `LazyHashMap::VacantEntry`.
values_entry: LazyVacantEntry<'a, K, ValueEntry<V>>,
}

/// An entry within the stash.
///
/// The vacant entries within a storage stash form a doubly linked list of
/// vacant entries that is used to quickly re-use their vacant storage.
pub enum Entry<'a, K: 'a, V: 'a, H = Blake2x256Hasher>
pub enum Entry<'a, K: 'a, V: 'a>
where
K: Ord + Clone + PackedLayout,
V: PackedLayout,
H: Hasher,
Key: From<<H as Hasher>::Output>,
{
/// A vacant entry that holds the index to the next and previous vacant entry.
Vacant(VacantEntry<'a, K, V, H>),
Vacant(VacantEntry<'a, K, V>),
/// An occupied entry that holds the value.
Occupied(OccupiedEntry<'a, K, V, H>),
Occupied(OccupiedEntry<'a, K, V>),
}

impl<K, V, H> HashMap<K, V, H>
Expand All @@ -163,11 +160,17 @@ where
}
}

/// Returns the number of key- value pairs stored in the hash map.
/// Returns the number of key-value pairs stored in the hash map.
pub fn len(&self) -> u32 {
self.keys.len()
}

/// Returns the number of key-value pairs stored in the cache.
#[cfg(test)]
pub(crate) fn len_cached_entries(&self) -> u32 {
self.keys.len()
}

/// Returns `true` if the hash map is empty.
pub fn is_empty(&self) -> bool {
self.keys.is_empty()
Expand Down Expand Up @@ -386,41 +389,43 @@ where
}

/// Gets the given key's corresponding entry in the map for in-place manipulation.
pub fn entry(&mut self, key: K) -> Entry<K, V, H> {
let v = self.values.get(&key);
match v {
Some(entry) => {
pub fn entry(&mut self, key: K) -> Entry<K, V> {
let entry = self.values.entry(key);
match entry {
LazyEntry::Occupied(o) => {
Entry::Occupied(OccupiedEntry {
key,
key_index: entry.key_index,
base: self,
keys: &mut self.keys,
values_entry: o,
})
}
LazyEntry::Vacant(v) => {
Entry::Vacant(VacantEntry {
keys: &mut self.keys,
values_entry: v,
})
}
None => Entry::Vacant(VacantEntry { key, base: self }),
}
}
}

impl<'a, K, V, H> Entry<'a, K, V, H>
impl<'a, K, V> Entry<'a, K, V>
where
K: Ord + Clone + PackedLayout,
V: PackedLayout + core::fmt::Debug + core::cmp::Eq + Default,
H: Hasher,
Key: From<<H as Hasher>::Output>,
{
/// Returns a reference to this entry's key.
pub fn key(&self) -> &K {
match self {
Entry::Occupied(entry) => &entry.key,
Entry::Vacant(entry) => &entry.key,
Entry::Occupied(entry) => &entry.values_entry.key(),
Entry::Vacant(entry) => &entry.values_entry.key(),
}
}

/// Ensures a value is in the entry by inserting the default value if empty, and returns
/// a reference to the value in the entry.
pub fn or_default(self) -> &'a V {
match self {
Entry::Occupied(entry) => entry.into_mut(),
Entry::Occupied(entry) => &mut entry.values_entry.into_mut().value,
Entry::Vacant(entry) => entry.insert(V::default()),
}
}
Expand All @@ -429,7 +434,7 @@ where
/// a mutable reference to the value in the entry.
pub fn or_insert(self, default: V) -> &'a mut V {
match self {
Entry::Occupied(entry) => entry.into_mut(),
Entry::Occupied(entry) => &mut entry.values_entry.into_mut().value,
Entry::Vacant(entry) => entry.insert(default),
}
}
Expand All @@ -441,7 +446,7 @@ where
F: FnOnce() -> V,
{
match self {
Entry::Occupied(entry) => entry.into_mut(),
Entry::Occupied(entry) => &mut entry.values_entry.into_mut().value,
Entry::Vacant(entry) => Entry::insert(default(), entry),
}
}
Expand All @@ -454,8 +459,8 @@ where
F: FnOnce(&K) -> V,
{
match self {
Entry::Occupied(entry) => entry.into_mut(),
Entry::Vacant(entry) => Entry::insert(default(&entry.key), entry),
Entry::Occupied(entry) => &mut entry.values_entry.into_mut().value,
Entry::Vacant(entry) => Entry::insert(default(&entry.key()), entry),
}
}

Expand All @@ -468,8 +473,8 @@ where
match self {
Entry::Occupied(mut entry) => {
{
let v = entry.get_mut();
f(v);
let v = entry.values_entry.get_mut();
f(&mut v.value);
}
Entry::Occupied(entry)
}
Expand All @@ -478,98 +483,73 @@ where
}

/// Inserts `value` into `entry`.
fn insert(value: V, entry: VacantEntry<'a, K, V, H>) -> &'a mut V {
let old_value = entry.base.insert(entry.key.clone(), value);
debug_assert!(old_value.is_none());
entry
.base
.get_mut(&entry.key)
.expect("encountered invalid vacant entry")
fn insert(value: V, entry: VacantEntry<'a, K, V>) -> &'a mut V {
entry.insert(value)
}
}

impl<'a, K, V, H> VacantEntry<'a, K, V, H>
impl<'a, K, V> VacantEntry<'a, K, V>
where
K: Ord + Clone + PackedLayout,
V: PackedLayout,
H: Hasher,
Key: From<<H as Hasher>::Output>,
{
/// Gets a reference to the key that would be used when inserting a value through the VacantEntry.
pub fn key(&self) -> &K {
&self.key
&self.values_entry.key()
}

/// Take ownership of the key.
pub fn into_key(self) -> K {
self.key
self.values_entry.into_key()
}

/// Sets the value of the entry with the VacantEntry's key, and returns a mutable reference to it.
/// Sets the value of the entry with the `VacantEntry`'s key, and returns a mutable reference to it.
pub fn insert(self, value: V) -> &'a mut V {
// At this point we know that `key` does not yet exist in the map.
let key_index = self.base.keys.put(self.key.clone());
self.base
.values
.put(self.key.clone(), Some(ValueEntry { value, key_index }));
self.base
.get_mut(&self.key)
.expect("put was just executed; qed")
let key_index = self.keys.put(self.key().to_owned());
&mut self
.values_entry
.insert(ValueEntry { value, key_index })
.value
}
}

impl<'a, K, V, H> OccupiedEntry<'a, K, V, H>
impl<'a, K, V> OccupiedEntry<'a, K, V>
where
K: Ord + Clone + PackedLayout,
V: PackedLayout,
H: Hasher,
Key: From<<H as Hasher>::Output>,
{
/// Gets a reference to the key in the entry.
pub fn key(&self) -> &K {
&self.key
&self.values_entry.key()
}

/// Take the ownership of the key and value from the map.
pub fn remove_entry(self) -> (K, V) {
let entry = self
.base
.values
.put_get(&self.key, None)
.expect("`key` must exist");
self.base
.keys
.take(self.key_index)
let k = self.values_entry.key().to_owned();
let v = self.values_entry.remove();
self.keys
.take(v.key_index)
.expect("`key_index` must point to a valid key entry");
(self.key, entry.value)
(k, v.value)
}

/// Gets a reference to the value in the entry.
pub fn get(&self) -> &V {
&self
.base
.get(&self.key)
.expect("entry behind `OccupiedEntry` must always exist")
&self.values_entry.get().value
}

/// Gets a mutable reference to the value in the entry.
///
/// If you need a reference to the `OccupiedEntry` which may outlive the destruction of the
/// `Entry` value, see `into_mut`.
pub fn get_mut(&mut self) -> &mut V {
self.base
.get_mut(&self.key)
.expect("entry behind `OccupiedEntry` must always exist")
&mut self.values_entry.get_mut().value
}

/// Sets the value of the entry, and returns the entry's old value.
pub fn insert(&mut self, new_value: V) -> V {
let occupied = self
.base
.values
.get_mut(&self.key)
.expect("entry behind `OccupiedEntry` must always exist");
core::mem::replace(&mut occupied.value, new_value)
core::mem::replace(&mut self.values_entry.get_mut().value, new_value)
}

/// Takes the value out of the entry, and returns it.
Expand All @@ -580,8 +560,6 @@ where
/// Converts the OccupiedEntry into a mutable reference to the value in the entry
/// with a lifetime bound to the map itself.
pub fn into_mut(self) -> &'a mut V {
self.base
.get_mut(&self.key)
.expect("entry behind `OccupiedEntry` must always exist")
&mut self.values_entry.into_mut().value
}
}