debris / tiny-keccak

An implementation of Keccak derived functions specified in FIPS-202, SP800-185 and KangarooTwelve
Creative Commons Zero v1.0 Universal
193 stars 49 forks source link

Replace some unsafe with safe Rust without performance regression #36

Closed brycx closed 5 years ago

brycx commented 5 years ago

There seems to be no performance regression. I actually noticed a small improvement.

With unsafe:

test bench_sha3_256_input_4096_bytes ... bench:      13,141 ns/iter (+/- 890) = 311 MB/s
test keccakf_u64                     ... bench:         413 ns/iter (+/- 31) = 60 MB/s

Without unsafe:

test bench_sha3_256_input_4096_bytes ... bench:      12,986 ns/iter (+/- 1,054) = 315 MB/s
test keccakf_u64                     ... bench:         409 ns/iter (+/- 27) = 61 MB/s
debris commented 5 years ago

I'll double check and merge if it is indeed faster. With an old version of compiler (before rust 1.20) it was muuuch slower

brycx commented 5 years ago

Sounds great!

Also, do you have any news about the big-endian fix (issue: #15 , possible fix: #35)? I'm using tiny-keccak in another library myself and would like to know if you plan on resolving the issue.

burdges commented 5 years ago

Yes, these unsafe loops do not elide bounds checks correctly. I submitted a fix in https://github.com/debris/tiny-keccak/pull/8/commits/ccf709b13ed1e9512068c624e707cdc7d971ed66 that was idiomatic for highly optimized Rust, but I'm happy the compiler has improved its optimizations of truely idiomatic Rust.