Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

reduce number of constraints for triedb types #9043

Merged
merged 4 commits into from
Jul 5, 2018
Merged

reduce number of constraints for triedb types #9043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e7312cc
NodeHandle does not require Hasher
debris Jul 4, 2018
48cc264
Node does not require Hasher
debris Jul 4, 2018
17bc647
Merge branch 'master' into triedb
debris Jul 5, 2018
9a5870f
change name of the template typo from I to O
debris Jul 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion util/hashdb/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use heapsize::HeapSizeOf;
use std::collections::HashMap;
use std::{fmt::Debug, hash::Hash};

/// Trait describing an object that can hash a slice of bytes. Used to abstract
/// Trait describing an object that can hash a slice of bytes. Used to abstract
/// other types over the hashing algorithm. Defines a single `hash` method and an
/// `Out` associated type with the necessary bounds.
pub trait Hasher: Sync + Send {
Expand Down
5 changes: 3 additions & 2 deletions util/patricia_trie/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ license = "GPL-3.0"

[dependencies]
elastic-array = "0.10"
ethcore-bytes = { version = "0.1.0", path = "../bytes" }
hashdb = { version = "0.2", path = "../hashdb" }
heapsize = "0.4"
log = "0.3"
rand = "0.4"
hashdb = { version = "0.2", path = "../hashdb" }
ethcore-bytes = { version = "0.1.0", path = "../bytes" }

[dev-dependencies]
env_logger = "0.5"
Expand Down
5 changes: 3 additions & 2 deletions util/patricia_trie/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
extern crate elastic_array;
extern crate ethcore_bytes as bytes;
extern crate hashdb;
extern crate heapsize;
extern crate rand;
#[macro_use]
extern crate log;
Expand Down Expand Up @@ -277,8 +278,8 @@ impl<'db, H: Hasher, C: NodeCodec<H>> Trie<H, C> for TrieKinds<'db, H, C> {
}

impl<'db, H, C> TrieFactory<H, C>
where
H: Hasher,
where
H: Hasher,
C: NodeCodec<H> + 'db
{
/// Creates new factory.
Expand Down
85 changes: 45 additions & 40 deletions util/patricia_trie/src/triedbmut.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ use std::collections::{HashSet, VecDeque};
use std::marker::PhantomData;
use std::mem;
use std::ops::Index;
use heapsize::HeapSizeOf;
use std::{fmt::Debug, hash::Hash};

// For lookups into the Node storage buffer.
// This is deliberately non-copyable.
Expand All @@ -39,20 +41,20 @@ struct StorageHandle(usize);

// Handles to nodes in the trie.
#[derive(Debug)]
enum NodeHandle<H: Hasher> {
enum NodeHandle<H> {
/// Loaded into memory.
InMemory(StorageHandle),
/// Either a hash or an inline node
Hash(H::Out),
Hash(H),
}

impl<H: Hasher> From<StorageHandle> for NodeHandle<H> {
impl<H> From<StorageHandle> for NodeHandle<H> {
fn from(handle: StorageHandle) -> Self {
NodeHandle::InMemory(handle)
}
}

fn empty_children<H: Hasher>() -> Box<[Option<NodeHandle<H>>; 16]> {
fn empty_children<H>() -> Box<[Option<NodeHandle<H>>; 16]> {
Box::new([
None, None, None, None, None, None, None, None,
None, None, None, None, None, None, None, None,
Expand All @@ -61,7 +63,7 @@ fn empty_children<H: Hasher>() -> Box<[Option<NodeHandle<H>>; 16]> {

/// Node types in the Trie.
#[derive(Debug)]
enum Node<H: Hasher> {
enum Node<H> {
/// Empty node.
Empty,
/// A leaf node contains the end of a key and a value.
Expand All @@ -77,36 +79,38 @@ enum Node<H: Hasher> {
Branch(Box<[Option<NodeHandle<H>>; 16]>, Option<DBValue>)
}

impl<H: Hasher> Node<H> {
impl<O> Node<O> where O: AsRef<[u8]> + AsMut<[u8]> + Default + HeapSizeOf + Debug + PartialEq + Eq + Hash + Send + Sync + Clone + Copy {
// load an inline node into memory or get the hash to do the lookup later.
fn inline_or_hash<C>(node: &[u8], db: &HashDB<H>, storage: &mut NodeStorage<H>) -> NodeHandle<H>
where C: NodeCodec<H>
fn inline_or_hash<C, H>(node: &[u8], db: &HashDB<H>, storage: &mut NodeStorage<H::Out>) -> NodeHandle<H::Out>
where C: NodeCodec<H>,
H: Hasher<Out = O>,
{
C::try_decode_hash(&node)
.map(NodeHandle::Hash)
.unwrap_or_else(|| {
let child = Node::from_encoded::<C>(node, db, storage);
let child = Node::from_encoded::<C, H>(node, db, storage);
NodeHandle::InMemory(storage.alloc(Stored::New(child)))
})
}

// decode a node from encoded bytes without getting its children.
fn from_encoded<C>(data: &[u8], db: &HashDB<H>, storage: &mut NodeStorage<H>) -> Self
where C: NodeCodec<H>
fn from_encoded<C, H>(data: &[u8], db: &HashDB<H>, storage: &mut NodeStorage<H::Out>) -> Self
where C: NodeCodec<H>,
H: Hasher<Out = O>,
{
match C::decode(data).expect("encoded bytes read from db; qed") {
EncodedNode::Empty => Node::Empty,
EncodedNode::Leaf(k, v) => Node::Leaf(k.encoded(true), DBValue::from_slice(&v)),
EncodedNode::Extension(key, cb) => {
Node::Extension(
key.encoded(false),
Self::inline_or_hash::<C>(cb, db, storage))
Self::inline_or_hash::<C, H>(cb, db, storage))
}
EncodedNode::Branch(ref encoded_children, val) => {
let mut child = |i:usize| {
let raw = encoded_children[i];
if !C::is_empty_node(raw) {
Some(Self::inline_or_hash::<C>(raw, db, storage))
Some(Self::inline_or_hash::<C, H>(raw, db, storage))
} else {
None
}
Expand All @@ -125,10 +129,11 @@ impl<H: Hasher> Node<H> {
}

// TODO: parallelize
fn into_encoded<F, C>(self, mut child_cb: F) -> ElasticArray1024<u8>
fn into_encoded<F, C, H>(self, mut child_cb: F) -> ElasticArray1024<u8>
where
C: NodeCodec<H>,
F: FnMut(NodeHandle<H>) -> ChildReference<H::Out>
F: FnMut(NodeHandle<H::Out>) -> ChildReference<H::Out>,
H: Hasher<Out = O>,
{
match self {
Node::Empty => C::empty_node(),
Expand All @@ -139,7 +144,7 @@ impl<H: Hasher> Node<H> {
// map the `NodeHandle`s from the Branch to `ChildReferences`
children.iter_mut()
.map(Option::take)
.map(|maybe_child|
.map(|maybe_child|
maybe_child.map(|child| child_cb(child))
),
value
Expand All @@ -150,7 +155,7 @@ impl<H: Hasher> Node<H> {
}

// post-inspect action.
enum Action<H: Hasher> {
enum Action<H> {
// Replace a node with a new one.
Replace(Node<H>),
// Restore the original node. This trusts that the node is actually the original.
Expand All @@ -160,14 +165,14 @@ enum Action<H: Hasher> {
}

// post-insert action. Same as action without delete
enum InsertAction<H: Hasher> {
enum InsertAction<H> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my education, why is it better to have the type param be unbounded like this? Knowing that H is a Hasher::Out is – to my eyes at least – helpful to understand what is going on, so curious about the reasoning here.

Copy link
Collaborator Author

@debris debris Jul 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my education, why is it better to have the type param be unbounded like this?

It's more simple. It's easier to write unit tests and integrate it with other parts of the code. We don't need bounded types at all. std does not use them. Only the impl are bounded

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, makes sense. I think rob pointed me to https://rust-lang-nursery.github.io/api-guidelines/future-proofing.html#c-struct-bounds at one time.

// Replace a node with a new one.
Replace(Node<H>),
// Restore the original node.
Restore(Node<H>),
}

impl<H: Hasher> InsertAction<H> {
impl<H> InsertAction<H> {
fn into_action(self) -> Action<H> {
match self {
InsertAction::Replace(n) => Action::Replace(n),
Expand All @@ -184,11 +189,11 @@ impl<H: Hasher> InsertAction<H> {
}

// What kind of node is stored here.
enum Stored<H: Hasher> {
enum Stored<H> {
// A new node.
New(Node<H>),
// A cached node, loaded from the DB.
Cached(Node<H>, H::Out),
Cached(Node<H>, H),
}

/// Used to build a collection of child nodes from a collection of `NodeHandle`s
Expand All @@ -198,12 +203,12 @@ pub enum ChildReference<HO> { // `HO` is e.g. `H256`, i.e. the output of a `Hash
}

/// Compact and cache-friendly storage for Trie nodes.
struct NodeStorage<H: Hasher> {
struct NodeStorage<H> {
nodes: Vec<Stored<H>>,
free_indices: VecDeque<usize>,
}

impl<H: Hasher> NodeStorage<H> {
impl<H> NodeStorage<H> {
/// Create a new storage.
fn empty() -> Self {
NodeStorage {
Expand Down Expand Up @@ -232,7 +237,7 @@ impl<H: Hasher> NodeStorage<H> {
}
}

impl<'a, H: Hasher> Index<&'a StorageHandle> for NodeStorage<H> {
impl<'a, H> Index<&'a StorageHandle> for NodeStorage<H> {
type Output = Node<H>;

fn index(&self, handle: &'a StorageHandle) -> &Node<H> {
Expand Down Expand Up @@ -284,10 +289,10 @@ where
H: Hasher + 'a,
C: NodeCodec<H>
{
storage: NodeStorage<H>,
storage: NodeStorage<H::Out>,
db: &'a mut HashDB<H>,
root: &'a mut H::Out,
root_handle: NodeHandle<H>,
root_handle: NodeHandle<H::Out>,
death_row: HashSet<H::Out>,
/// The number of hash operations this trie has performed.
/// Note that none are performed until changes are committed.
Expand Down Expand Up @@ -347,7 +352,7 @@ where
// cache a node by hash
fn cache(&mut self, hash: H::Out) -> Result<StorageHandle, H::Out, C::Error> {
let node_encoded = self.db.get(&hash).ok_or_else(|| Box::new(TrieError::IncompleteDatabase(hash)))?;
let node = Node::from_encoded::<C>(
let node = Node::from_encoded::<C, H>(
&node_encoded,
&*self.db,
&mut self.storage
Expand All @@ -357,8 +362,8 @@ where

// inspect a node, choosing either to replace, restore, or delete it.
// if restored or replaced, returns the new node along with a flag of whether it was changed.
fn inspect<F>(&mut self, stored: Stored<H>, inspector: F) -> Result<Option<(Stored<H>, bool)>, H::Out, C::Error>
where F: FnOnce(&mut Self, Node<H>) -> Result<Action<H>, H::Out, C::Error> {
fn inspect<F>(&mut self, stored: Stored<H::Out>, inspector: F) -> Result<Option<(Stored<H::Out>, bool)>, H::Out, C::Error>
where F: FnOnce(&mut Self, Node<H::Out>) -> Result<Action<H::Out>, H::Out, C::Error> {
Ok(match stored {
Stored::New(node) => match inspector(self, node)? {
Action::Restore(node) => Some((Stored::New(node), false)),
Expand All @@ -380,7 +385,7 @@ where
}

// walk the trie, attempting to find the key's node.
fn lookup<'x, 'key>(&'x self, mut partial: NibbleSlice<'key>, handle: &NodeHandle<H>) -> Result<Option<DBValue>, H::Out, C::Error>
fn lookup<'x, 'key>(&'x self, mut partial: NibbleSlice<'key>, handle: &NodeHandle<H::Out>) -> Result<Option<DBValue>, H::Out, C::Error>
where 'x: 'key
{
let mut handle = handle;
Expand Down Expand Up @@ -429,7 +434,7 @@ where
}

/// insert a key-value pair into the trie, creating new nodes if necessary.
fn insert_at(&mut self, handle: NodeHandle<H>, partial: NibbleSlice, value: DBValue, old_val: &mut Option<DBValue>) -> Result<(StorageHandle, bool), H::Out, C::Error> {
fn insert_at(&mut self, handle: NodeHandle<H::Out>, partial: NibbleSlice, value: DBValue, old_val: &mut Option<DBValue>) -> Result<(StorageHandle, bool), H::Out, C::Error> {
let h = match handle {
NodeHandle::InMemory(h) => h,
NodeHandle::Hash(h) => self.cache(h)?,
Expand All @@ -443,7 +448,7 @@ where
}

/// the insertion inspector.
fn insert_inspector(&mut self, node: Node<H>, partial: NibbleSlice, value: DBValue, old_val: &mut Option<DBValue>) -> Result<InsertAction<H>, H::Out, C::Error> {
fn insert_inspector(&mut self, node: Node<H::Out>, partial: NibbleSlice, value: DBValue, old_val: &mut Option<DBValue>) -> Result<InsertAction<H::Out>, H::Out, C::Error> {
trace!(target: "trie", "augmented (partial: {:?}, value: {:?})", partial, value.pretty());

Ok(match node {
Expand Down Expand Up @@ -604,7 +609,7 @@ where
}

/// Remove a node from the trie based on key.
fn remove_at(&mut self, handle: NodeHandle<H>, partial: NibbleSlice, old_val: &mut Option<DBValue>) -> Result<Option<(StorageHandle, bool)>, H::Out, C::Error> {
fn remove_at(&mut self, handle: NodeHandle<H::Out>, partial: NibbleSlice, old_val: &mut Option<DBValue>) -> Result<Option<(StorageHandle, bool)>, H::Out, C::Error> {
let stored = match handle {
NodeHandle::InMemory(h) => self.storage.destroy(h),
NodeHandle::Hash(h) => {
Expand All @@ -619,7 +624,7 @@ where
}

/// the removal inspector
fn remove_inspector(&mut self, node: Node<H>, partial: NibbleSlice, old_val: &mut Option<DBValue>) -> Result<Action<H>, H::Out, C::Error> {
fn remove_inspector(&mut self, node: Node<H::Out>, partial: NibbleSlice, old_val: &mut Option<DBValue>) -> Result<Action<H::Out>, H::Out, C::Error> {
Ok(match (node, partial.is_empty()) {
(Node::Empty, _) => Action::Delete,
(Node::Branch(c, None), true) => Action::Restore(Node::Branch(c, None)),
Expand Down Expand Up @@ -705,7 +710,7 @@ where
/// _invalid state_ means:
/// - Branch node where there is only a single entry;
/// - Extension node followed by anything other than a Branch node.
fn fix(&mut self, node: Node<H>) -> Result<Node<H>, H::Out, C::Error> {
fn fix(&mut self, node: Node<H::Out>) -> Result<Node<H::Out>, H::Out, C::Error> {
match node {
Node::Branch(mut children, value) => {
// if only a single value, transmute to leaf/extension and feed through fixed.
Expand Down Expand Up @@ -825,7 +830,7 @@ where

match self.storage.destroy(handle) {
Stored::New(node) => {
let encoded_root = node.into_encoded::<_, C>(|child| self.commit_child(child) );
let encoded_root = node.into_encoded::<_, C, H>(|child| self.commit_child(child) );
*self.root = self.db.insert(&encoded_root[..]);
self.hash_count += 1;

Expand All @@ -845,14 +850,14 @@ where
/// case where we can fit the actual data in the `Hasher`s output type, we
/// store the data inline. This function is used as the callback to the
/// `into_encoded` method of `Node`.
fn commit_child(&mut self, handle: NodeHandle<H>) -> ChildReference<H::Out> {
fn commit_child(&mut self, handle: NodeHandle<H::Out>) -> ChildReference<H::Out> {
match handle {
NodeHandle::Hash(hash) => ChildReference::Hash(hash),
NodeHandle::InMemory(storage_handle) => {
match self.storage.destroy(storage_handle) {
Stored::Cached(_, hash) => ChildReference::Hash(hash),
Stored::New(node) => {
let encoded = node.into_encoded::<_, C>(|node_handle| self.commit_child(node_handle) );
let encoded = node.into_encoded::<_, C, H>(|node_handle| self.commit_child(node_handle) );
if encoded.len() >= H::LENGTH {
let hash = self.db.insert(&encoded[..]);
self.hash_count +=1;
Expand All @@ -871,7 +876,7 @@ where
}

// a hack to get the root node's handle
fn root_handle(&self) -> NodeHandle<H> {
fn root_handle(&self) -> NodeHandle<H::Out> {
match self.root_handle {
NodeHandle::Hash(h) => NodeHandle::Hash(h),
NodeHandle::InMemory(StorageHandle(x)) => NodeHandle::InMemory(StorageHandle(x)),
Expand All @@ -880,7 +885,7 @@ where
}

impl<'a, H, C> TrieMut<H, C> for TrieDBMut<'a, H, C>
where
where
H: Hasher,
C: NodeCodec<H>
{
Expand Down