archer884 / harsh

Hashids implementation in Rust
Apache License 2.0
63 stars 4 forks source link

Stabilize #9

Closed archer884 closed 4 years ago

archer884 commented 6 years ago

Outstanding items:

Note: the first part above might require fuzzing. Just saying. Hell if I know.

archer884 commented 6 years ago

@Dr-Emann's patch adds quickcheck, which has largely resolved my questions regarding the use of unwrap and panic in the code. There was, in fact, a bug relating to attempts to decode invalid input (that is, inputs containing letters not found in the alphabet).

As a side note, if you're reading, what's the story on calls to black_box outside the benchmark iteration context? I didn't remove those, but I kind of got the idea they wouldn't do anything.

archer884 commented 6 years ago

I have decided against decode_into because harsh's performance is simply not marvelous enough to justify the additional API complexity.

archer884 commented 6 years ago

Leaning toward not stabilizing right now and instead releasing a 0.2.0-rc. I would prefer to present a less disjointed interface, with everything returning result values rather than some things returning options and others returning results. This is especially true in light of what I have learned recently about the performance characteristics of option values...

...Even if those can hardly be considered relevant, particularly for encoding. :)

As another side note, if I'm reading those benchmarks @Dr-Emann provided correctly, encoding takes much, much longer than decoding. Any thoughts? :|

Dr-Emann commented 6 years ago

There's no benefit to using black_box outside the iter function, however, because I benchmarked creation using the init function, I wanted to make sure it wasn't going to do any smart inlining of salts or anything.

Dr-Emann commented 6 years ago

My results don't look like there's a huge difference between encoding and decoding, and if anything encoding is slightly faster than decoding:

running 4 tests
test custom_creation  ... bench:       5,566 ns/iter (+/- 2,769)
test decode           ... bench:     179,099 ns/iter (+/- 61,987)
test default_creation ... bench:       3,059 ns/iter (+/- 1,301)
test encode           ... bench:     144,143 ns/iter (+/- 36,351)

with rustc version rustc 1.25.0-nightly (56733bc9f 2018-02-01) on a windows machine

archer884 commented 6 years ago

Aw crap, you're right... I saw the blackbox thing around the encode and assumed that it was measuring the encode that leads into the decode call. I'll get around this eventually. :)

On Fri, Feb 2, 2018 at 1:25 PM Zachary Dremann notifications@github.com wrote:

My results don't look like there's a huge difference between encoding and decoding, and if anything encoding is slightly faster than decoding:

running 4 tests test custom_creation ... bench: 5,566 ns/iter (+/- 2,769) test decode ... bench: 179,099 ns/iter (+/- 61,987) test default_creation ... bench: 3,059 ns/iter (+/- 1,301) test encode ... bench: 144,143 ns/iter (+/- 36,351)

with rustc version rustc 1.25.0-nightly (56733bc9f 2018-02-01) on a windows machine

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/archer884/harsh/pull/9#issuecomment-362681634, or mute the thread https://github.com/notifications/unsubscribe-auth/AApeRmmEvUHFTMJxyCJ890DEYdc_jrfPks5tQ2E3gaJpZM4Rh5zc .

archer884 commented 4 years ago

Looks like this is finally going to get done, but I rewrote the error code a different way.