Use *mut u8 to write FromHex bytes

[tankyleo]

• <[u8; LEN]>::from_hex now runs in 2x fewer "Estimated Cycles" on [email protected].
• <Vec<u8>>::from_hex now runs in 30% fewer "Estimated Cycles" on [email protected].

Please see https://github.com/tankyleo/hex-benchmarks for benchmark code.

Partially addresses #115

[tankyleo]

@RCasatta I try to address partially the issue you raised here, let me know what you think.

I am aware this is a lot of unsafe code that you might not be comfortable with. Happy to take a less aggressive approach.

[tcharding]

Any chance you could explain why this is faster, IIUC you are replacing a call to collect with a manual implementation - that is a pretty low level thing to need to do, I would have thought the Rust language could handle that.

[tankyleo]

Thank you for your question.

As far as I understand:

For a T: ExactSizeIterator, unsafe code "must not rely on the correctness of Iterator::size_hint": https://doc.rust-lang.org/stable/std/iter/trait.ExactSizeIterator.html

So while the collect call does allocate the total memory used in a single call, the hot loop that writes each element keeps checking that the vector has enough capacity to fit one more element:

https://github.com/rust-lang/rust/blob/fb32dd41ed278e2d691900248d94aa9bc4409f99/library/alloc/src/vec/mod.rs#L3510

Further below, you can see the implementation for T: TrustedLen:

https://github.com/rust-lang/rust/blob/fb32dd41ed278e2d691900248d94aa9bc4409f99/library/alloc/src/vec/mod.rs#L3538

The hot loop also bumps the length on each element write. See below:

• https://github.com/rust-lang/rust/blob/fb32dd41ed278e2d691900248d94aa9bc4409f99/library/alloc/src/vec/mod.rs#L3526-L3527
• https://github.com/rust-lang/rust/blob/fb32dd41ed278e2d691900248d94aa9bc4409f99/library/alloc/src/vec/mod.rs#L3553-L3554

[tankyleo]

I think I am looking at the correct location in the rust library, here's how I arrived at it:

• https://github.com/rust-lang/rust/blob/fb32dd41ed278e2d691900248d94aa9bc4409f99/library/alloc/src/vec/mod.rs#L3409
• https://github.com/rust-lang/rust/blob/fb32dd41ed278e2d691900248d94aa9bc4409f99/library/alloc/src/vec/spec_from_iter.rs#L33
• https://github.com/rust-lang/rust/blob/fb32dd41ed278e2d691900248d94aa9bc4409f99/library/alloc/src/vec/spec_from_iter_nested.rs#L19
• https://github.com/rust-lang/rust/blob/fb32dd41ed278e2d691900248d94aa9bc4409f99/library/alloc/src/vec/spec_extend.rs#L18
• https://github.com/rust-lang/rust/blob/fb32dd41ed278e2d691900248d94aa9bc4409f99/library/alloc/src/vec/mod.rs#L3510

[tankyleo]

Does a u8 need to be "dropped" ?

At the moment, if we return an InvalidCharError, we drop a vector that has zero length, but we might have written a few u8's.

My guess is u8 does not need to be "dropped", so the current behavior there is ok.

[RCasatta]

Thanks @tankyleo, the performance achievement looks great! In particular in #115 most of the time is spent in decoding hex txids, thus it would gain the 2x speedup of fixed size array, Also unsafe code looks ok to me, I really would like this.

That said, I am no expert in unsafe code, and given the name conservative in the crate we should be really confident on this.

I think reasons explained in @tankyleo commente here should be added in code comments.

I would like benches in https://github.com/tankyleo/hex-benchmarks to be added in this crate one day (as a sub-crate)

[tcharding]

Thanks, your function call chain looks correct. Its a bit sad that collect on a Vec does not seem to take ExactSizeIterator into consideration. Iterators are supposed to be pure magic, if they don't collect fast then they are not that magic.

[tcharding]

Could maybe add a comment to save the next guy wondering why we implement collect manually.

Use your own words, but if you want you could use

        // Calling `collect` on `self` is slower than this manual implementation because Rust does not
        // seem to utilize the fact that `HexToBytesIter` implements `ExactSizeIterator`.

[tankyleo]

Rebase: add a comment explaining why we implement collect manually.