Skip to content

Conversation

@sylvestre
Copy link
Contributor

nice early results:
Still lower but less

Benchmark 1: /usr/bin/cksum /tmp/oneline_4G.txt
  Time (mean ± σ):     602.7 ms ±  25.7 ms    [User: 88.0 ms, System: 514.3 ms]
  Range (min … max):   584.5 ms … 620.8 ms    2 runs

Benchmark 2: ./target/release/coreutils cksum /tmp/oneline_4G.txt
  Time (mean ± σ):      1.918 s ±  0.121 s    [User: 0.426 s, System: 1.491 s]
  Range (min … max):    1.832 s …  2.004 s    2 runs

Benchmark 3: ./target/release/coreutils.prev cksum /tmp/oneline_4G.txt
  Time (mean ± σ):      9.502 s ±  0.186 s    [User: 8.893 s, System: 0.608 s]
  Range (min … max):    9.370 s …  9.634 s    2 runs

Summary
  /usr/bin/cksum /tmp/oneline_4G.txt ran
    3.18 ± 0.24 times faster than ./target/release/coreutils cksum /tmp/oneline_4G.txt
   15.77 ± 0.74 times faster than ./target/release/coreutils.prev cksum /tmp/oneline_4G.txt

@github-actions
Copy link

github-actions bot commented Sep 9, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@sylvestre sylvestre marked this pull request as ready for review September 9, 2025 08:28
@github-actions
Copy link

github-actions bot commented Sep 9, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

Comment on lines 154 to +155
"crc32fast",
"crc-fast",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need two crc dependencies in the same feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think crc-fast doesn't support crc32b... :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but yeah, i don't like it either

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and i will document this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A detail: for consistency reasons I would name the var crc1, as you do in the other tests when using two Crc objects.

@sylvestre sylvestre mentioned this pull request Sep 9, 2025
@RenjiSann
Copy link
Collaborator

RenjiSann commented Sep 12, 2025

By using the Digest struct of the crc_fast crate, I was able to reach GNU performances.
The remaining bottleneck of your implementation @sylvestre is that you bufferize the whole file in a single vec (data_buffer), and wait until hash_finalize to actually compute the digest.

Code

It 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

Benchmark 1: ./cksum.new oneline_4G.txt
  Time (mean ± σ):     546.8 ms ±   4.8 ms    [User: 127.2 ms, System: 419.5 ms]
  Range (min … max):   540.0 ms … 551.2 ms    5 runs
 
Benchmark 2: ./cksum.buffered oneline_4G.txt
  Time (mean ± σ):      1.919 s ±  0.010 s    [User: 0.410 s, System: 1.509 s]
  Range (min … max):    1.905 s …  1.932 s    5 runs
  
Summary
  ./cksum.new oneline_4G.txt ran
    3.51 ± 0.04 times faster than ./cksum.buffered oneline_4G.txt

@sylvestre
Copy link
Contributor Author

@RenjiSann i have 0 attachment to this PR :) don't hesitate to submit your proposition as it seems much better :)

@sylvestre
Copy link
Contributor Author

merged here:
#8643

@sylvestre sylvestre closed this Sep 20, 2025
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.

3 participants