cksum: improve performance (Closes: #8573)#8579
cksum: improve performance (Closes: #8573)#8579sylvestre wants to merge 1 commit intouutils:mainfrom
Conversation
|
GNU testsuite comparison: |
f402fed to
3c79c05
Compare
|
GNU testsuite comparison: |
| "crc32fast", | ||
| "crc-fast", |
There was a problem hiding this comment.
Do we really need two crc dependencies in the same feature?
There was a problem hiding this comment.
i think crc-fast doesn't support crc32b... :)
There was a problem hiding this comment.
but yeah, i don't like it either
There was a problem hiding this comment.
and i will document this
There was a problem hiding this comment.
Ok, I simply read in the readme of crc-fast that they support
[...] all known CRC-32 and CRC-64 variants
and assumed that covers what crc32fast does.
|
|
||
| #[test] | ||
| fn test_crc_hash_update_edge_cases() { | ||
| let mut crc = Crc::new(); |
There was a problem hiding this comment.
A detail: for consistency reasons I would name the var crc1, as you do in the other tests when using two Crc objects.
|
By using the CodeIt also simplifies the code a lot: pub struct Crc(crc_fast::Digest);
impl Digest for Crc {
fn new() -> Self {
Self(crc_fast::Digest::new_with_params(get_posix_cksum_params()))
}
fn hash_update(&mut self, input: &[u8]) {
self.0.update(input);
}
fn hash_finalize(&mut self, out: &mut [u8]) {
let xout = self.0.finalize();
out.copy_from_slice(&xout.to_ne_bytes());
}
fn result_str(&mut self) -> String {
let mut out: [u8; 8] = [0; 8];
self.hash_finalize(&mut out);
u64::from_ne_bytes(out).to_string()
}
fn reset(&mut self) {
self.0.reset();
}
fn output_bits(&self) -> usize {
256
}
}Benchmark |
|
@RenjiSann i have 0 attachment to this PR :) don't hesitate to submit your proposition as it seems much better :) |
|
merged here: |
nice early results:
Still lower but less