Further Refactoring

[QuestofIranon]

I did some minor refactoring on top of #14

Includes the API improvements, fixes, and most of the performance boosts from #14 but uses newer dependencies based on recent advances in the Rust ecosystem.

• replaces FxHashMap with hashbrown for ~2x the performance boost (~8x performance boost on top of the original hashmap). hashbrown is also going to be merged into the rust std so it will allow for the dependency to be dropped in the future.

• removes ring as a dependency in favor of smaller specialized crypto libs that have came out of rust crypto recently. No longer depends on ssl implementations.

[steveatinfincia]

Hey this looks good, I'll take a closer look tomorrow and get it merged :)

[maciejhirsz]

So I just learned about hashbrown from this PR and it looks super impressive, but just running the benchmarks here it seems to be a regression:

test from_entropy ... bench:         517 ns/iter (+/- 414)
test new_mnemonic ... bench:         563 ns/iter (+/- 1)
test new_seed     ... bench:   1,706,894 ns/iter (+/- 85,852)
test validate     ... bench:         705 ns/iter (+/- 5)

Compared to results on #14:

test from_entropy ... bench:         395 ns/iter (+/- 226)
test new_mnemonic ... bench:         477 ns/iter (+/- 24)
test new_seed     ... bench:   1,173,651 ns/iter (+/- 69,909)
test validate     ... bench:         566 ns/iter (+/- 5)

new_seed is slower because the pbkdf2 crate is slower than the C (AFAIK?) implementation that ring binds to. This is probably quite fine, and having a pure Rust implementation might be beneficial for me since I want to target multiple architectures (Android and iOS specifically).

The other degradation I'm not sure about. Things I've checked thus far:

• hashbrown vs fxhash is an improvement (albeit marginal), so that's fine.
• No difference between lazy_static and one_cell that I can tell from benchmarks.
• The new error type is smaller on stack (Result<(), Error> is 2 words, previously with error_chain it was 7 words), so that should makes things faster regardless of what the error handling logic is since we aren't benching error branches anyway.
• Having to initialize the Vec with bytes for the entropy before filling it with randomness seems to not matter (and avoiding initialization with unsafe set_len only produces few ns of a difference, does not explain 100ns loss).

I might try to go at it from the other end - start at my branch and slowly re-introduce changes made here to see if I can figure out where the degradation happens.

Edit: it has to be sha2 crate, since all functions have to grab the first byte of the digest from it.

[QuestofIranon]

The sha2 crate has an asm feature. Using the asm implementation sees a better performance boost. I disabled it by default because users were having trouble building another crate that depended on my fork in Windows and anyone who wants that improvement can enable it in their project.

pbkdf2 is likely the main culprit of the performance drop, but it would probably be better to focus on improving the performance of that crate which would benefit other crates using it as well.

[QuestofIranon]

I should mention that one of my intentions with the changes in this PR was for better support across platforms, acknowledging the performance cost.

[maciejhirsz]

That makes sense. The performance degradation isn't bad at all (and it's still way faster than what it used to be prior to #14) so that seems like a reasonable trade-off.

[QuestofIranon]

I made another change that makes the non-English languages compile time features, though I left them all enabled by default. I thought this would be nice as an optional enhancement because it reduces the size of the compiled library by almost half if you disable the additional languages.

With default features disabled (only English) the size of the .rlib is 664 KB compared to 1390 KB with all languages.

The main downside is that currently English is always included even if you specify other languages. I needed a default language due to the current implementation and it is the standard language defined in bip. I might look into ways around this but thought it might be worth discussing first. @maciejhirsz @steveatinfincia

[maciejhirsz]

@steveatinfincia can we get this merged, pretty please? :heart:

[steveatinfincia]

@steveatinfincia can we get this merged, pretty please? ❤️

Yep, was just giving some room for the earlier discussion to play out :)

I'll merge and tag a beta release for Cargo tonight or tomorrow morning.

[steveatinfincia]

This is released on Cargo now as the v0.6.0-beta.1 tag