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

Trie optimizations #6389

Merged
merged 6 commits into from
Aug 28, 2017
Merged

Trie optimizations #6389

merged 6 commits into from
Aug 28, 2017

Conversation

debris
Copy link
Collaborator

@debris debris commented Aug 26, 2017

changes:

  • triedb seek and descend is not recursive
  • optimized memorydb insert, remove and emplace
  • optimized hashdb keys()

all trie insert benchmarks are about 10% faster

@debris debris added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Aug 26, 2017
@debris debris requested a review from rphmeier August 26, 2017 17:13
match self.data.entry(key) {
Entry::Occupied(mut entry) => {
let &mut (ref mut old_value, ref mut rc) = entry.get_mut();
if *rc >= -0x80000000i32 && *rc <= 0 {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

rc >= -0x80000000i32 is probably not needed, right @rphmeier ?

@@ -181,7 +181,10 @@ impl HashDB for MemoryDB {
}

fn keys(&self) -> HashMap<H256, i32> {
self.data.iter().filter_map(|(k, v)| if v.1 != 0 {Some((k.clone(), v.1))} else {None}).collect()
self.data.iter()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why remove a filter_map? Clippy produces a warning on .filter().map() sequence

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah, ok

I just found it more readable than this long one-liner

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

node: node.clone().into(),
});
}
fn seek_descend<'key>(&mut self, mut node_data: DBValue, mut key: NibbleSlice<'key>) -> super::Result<()> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be renamed to just seek since it is not recursive anymore.

fn seek_descend<'key>(&mut self, mut node_data: DBValue, mut key: NibbleSlice<'key>) -> super::Result<()> {
enum Step {
Descend(DBValue, usize),
Return,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be just a return statement

@arkpar arkpar added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 28, 2017
@debris debris added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Aug 28, 2017
@debris
Copy link
Collaborator Author

debris commented Aug 28, 2017

@arkpar fixed

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 28, 2017
@debris
Copy link
Collaborator Author

debris commented Aug 28, 2017

js failure is unrelated

@debris debris merged commit 8ead806 into master Aug 28, 2017
@debris debris deleted the trie branch August 28, 2017 16:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants