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

remove the dependency on byteorder #5

Closed
wants to merge 1 commit into from
Closed

Conversation

oconnor663
Copy link
Owner

Since that crate takes slices in its API, it tends to introduce unnecessary bounds checks.

Since that crate takes slices in its API, it tends to introduce
unnecessary bounds checks.
@oconnor663
Copy link
Owner Author

oconnor663 commented Oct 1, 2018

This removes a dependency and slightly improves the performance of the portable implementation. But we can't land it until rust-lang/rust#52963 is stable, unless we want to copy those unsafe functions into the crate.

@oconnor663
Copy link
Owner Author

Notably, with the slight performance improvement in this PR, our b2sum hardcoded to use portable.rs edges past the coreutils b2sum in performance.

@oconnor663
Copy link
Owner Author

Actually it turns out that the slight performance drag was entirely due to read_u64_into, and unrolling that loop improves it, without needing to remove the byteorder dependency. I'm going to land some of that now, and we can drop this PR. The to_le_bytes API will be nice, but I think at least in this case it's going to compile to the same code as byteorder.

@oconnor663 oconnor663 closed this Oct 14, 2018
@oconnor663 oconnor663 deleted the no_byteorder branch June 22, 2019 18:57
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.

1 participant