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

Itertools are no longer reexported from util, optimized triedb iter #6322

Merged
merged 5 commits into from
Aug 21, 2017

Conversation

debris
Copy link
Collaborator

@debris debris commented Aug 17, 2017

Triedb benches

old:

test trie_iter                      ... bench:   6,571,603 ns/iter (+/- 732,647)

new:

test trie_iter                      ... bench:   6,181,846 ns/iter (+/- 461,696)

@debris debris added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Aug 17, 2017
@@ -293,7 +292,17 @@ impl<'a> TrieDBIterator<'a> {
/// The present key.
fn key(&self) -> Bytes {
// collapse the key_nibbles down to bytes.
self.key_nibbles.iter().step(2).zip(self.key_nibbles.iter().skip(1).step(2)).map(|(h, l)| h * 16 + l).collect()
unsafe {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it really require such big block of unsafe code to be fast?
what about:

let nibbles = self.key_nibbles;
let len = nibbles.len() - 1;
let mut i = 0;
while i < len {
  result.push(nibbles[i] * 16 + nibbles[i + 1]);
  i += 2;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe the checked indexing is too slow? i would guess that using unsafe only for get_unchecked would be good enough.

@rphmeier
Copy link
Contributor

LGTM other than @tomusdrw grumble

@rphmeier rphmeier 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 18, 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 21, 2017
let nibbles = &self.key_nibbles;
let mut i = 0;
let mut result = Bytes::with_capacity(nibbles.len() / 2);
let len = nibbles.len() - 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a guarantee that nibbles is never empty and this can't underflow?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, just pushed a fix for that

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 21, 2017
@debris debris merged commit 6f03c2b into master Aug 21, 2017
@debris debris deleted the itertools branch August 21, 2017 13:58
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.

4 participants