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

Optimized PlainHasher hashing. Trie insertions are >15% faster #6321

Merged
merged 4 commits into from Aug 28, 2017

Conversation

debris
Copy link
Collaborator

@debris debris commented Aug 17, 2017

PlainHasher::write before:

test write ... bench:      18,025 ns/iter (+/- 1,425)

PlainHasher::write after:

test write ... bench:          58 ns/iter (+/- 10)

before:

test trie_insertions_32_mir_1k      ... bench:   3,956,331 ns/iter (+/- 577,529)
test trie_insertions_32_ran_1k      ... bench:   4,009,316 ns/iter (+/- 567,403)
test trie_insertions_random_mid     ... bench:   2,900,765 ns/iter (+/- 283,349)
test trie_insertions_six_high       ... bench:   2,981,152 ns/iter (+/- 331,667)
test trie_insertions_six_low        ... bench:   5,828,811 ns/iter (+/- 593,180)
test trie_insertions_six_mid        ... bench:   3,889,366 ns/iter (+/- 110,769)
test trie_iter                      ... bench:   6,755,724 ns/iter (+/- 1,199,892)

after:

test trie_insertions_32_mir_1k      ... bench:   3,262,684 ns/iter (+/- 118,147)
test trie_insertions_32_ran_1k      ... bench:   3,290,951 ns/iter (+/- 78,756)
test trie_insertions_random_mid     ... bench:   2,654,520 ns/iter (+/- 744,629)
test trie_insertions_six_high       ... bench:   2,492,217 ns/iter (+/- 236,359)
test trie_insertions_six_low        ... bench:   4,989,133 ns/iter (+/- 86,834)
test trie_insertions_six_mid        ... bench:   3,314,200 ns/iter (+/- 84,884)
test trie_iter                      ... bench:   6,749,411 ns/iter (+/- 6,490,631)

@debris debris added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Aug 17, 2017
}

fn random_value(seed: &mut H256) -> Bytes {
*seed = seed.sha3();
match seed[0] % 2 {
1 => vec![seed[31];1],
_ => seed.into_vec(),
_ => seed.to_vec(),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unrelated benchmark fix

@debris debris changed the title Optimized PlainHasher hashing. Trie insertions are ~10% faster Optimized PlainHasher hashing. Trie insertions are >15% faster Aug 17, 2017
@debris
Copy link
Collaborator Author

debris commented Aug 17, 2017

I moved PlainHasher to a separate crate. here is the source

@rphmeier
Copy link
Contributor

rphmeier commented Aug 18, 2017

indentation wrong here: https://github.com/debris/plain_hasher/blob/master/src/lib.rs#L45
BTW, shouldn't it be a paritytech crate? @arkpar and I are also authors of PlainHasher

@rphmeier rphmeier added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 18, 2017
@gavofyork gavofyork added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Aug 20, 2017
@debris
Copy link
Collaborator Author

debris commented Aug 21, 2017

@rphmeier updated readme and moved the repo to https://github.com/paritytech/plain_hasher

I will also add core-devs as owners on crates.io once they fix this bug: rust-lang/crates.io#997

@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
@arkpar
Copy link
Collaborator

arkpar commented Aug 28, 2017

I don't really see why PlainHasher needs to be on crates.io. It is too parity-specific to be generally useful.

@arkpar arkpar added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them 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 A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them 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 debris merged commit c6b3fac into master Aug 28, 2017
@debris debris deleted the plain_hasher branch August 28, 2017 16:46
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.

None yet

4 participants