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

Rust Scrypt deprecation #292

Merged
merged 1 commit into from
Aug 1, 2018
Merged

Rust Scrypt deprecation #292

merged 1 commit into from
Aug 1, 2018

Conversation

mersinvald
Copy link
Contributor

This PR aims to replace rust-scrypt crate with the pure rust scrypt crate implementation for all platforms.

@r8d8 is there a reason to maintain rust-scrypt and not do this transition on unix?

@mersinvald mersinvald requested a review from r8d8 July 30, 2018 14:26
@ghost ghost assigned mersinvald Jul 30, 2018
@ghost ghost added the review label Jul 30, 2018
@mersinvald
Copy link
Contributor Author

mersinvald commented Jul 30, 2018

Ran the banchmarks, encoding takes forever with both crates somehow.

Performance

scrypt

decode: 2,088,595 ns/iter (+/- 6,930)
encode: Time Out

ryst-scrypt

decode: 2,149,693 ns/iter (+/- 9,936)
encode: Time Out

@r8d8
Copy link
Contributor

r8d8 commented Jul 31, 2018

rust built-in benchmark was giving unreal results.
The most accurate real-life bench I had, is measurements of generation time for account through emerald-cli interface.

@mersinvald
Copy link
Contributor Author

mersinvald commented Aug 1, 2018

@r8d8 I've made custom bench for that, built-in benches couldn't chew through encoding.

Performance

scrypt

decrypt: 2090547 ns/iter
encrypt: 1279643 us/iter

rust-scrypt

decrypt: 2150338 ns/iter
encrypt: 1326455 us/iter

Though the difference is not very noticeable, scrypt is a bit faster then rust-scrypt

UPD: in debug builds encryption takes 150+ seconds, should we consider opting-in the rust-scrypt for debug builds only or postponing this change at all until the rust-lang/cargo#1359 and rust-lang/rust#48683 are resolved?

Do not merge this until #291 is merged, this PR depends on #291

@r8d8 r8d8 merged commit 464b0b7 into master Aug 1, 2018
@ghost ghost removed the review label Aug 1, 2018
@splix splix deleted the rust-scrypt-deprecation branch April 9, 2019 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants