cksum: small improvements, l10n #9532

RenjiSann · 2025-12-01T00:05:45Z

This MR brings a few changes to simplify and optimize some parts of the checksum computation

It depends on the #9511 MR

github-actions · 2025-12-01T00:32:57Z

GNU testsuite comparison:

Congrats! The gnu test tests/cksum/cksum-c is no longer failing!

codspeed-hq · 2025-12-01T02:12:46Z

CodSpeed Performance Report

Merging #9532 will degrade performances by 16.41%

_{Comparing RenjiSann:cksum-improvements (935f0a9) with main (22a8732)}

Summary

⚡ 1 improvement
❌ 13 regressions
✅ 112 untouched
⏩ 6 skipped¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
❌	`wc_bytes_synthetic[500]`	163.9 µs	182.4 µs	-10.16%
❌	`b64_decode_synthetic`	149.1 µs	168.9 µs	-11.74%
❌	`b64_decode_ignore_garbage_synthetic`	149.2 µs	166.8 µs	-10.55%
❌	`b64_encode_synthetic`	146.3 µs	164.4 µs	-11.04%
❌	`cksum_blake3`	190.7 µs	207.7 µs	-8.18%
⚡	`sort_ascii_c_locale`	22.7 ms	21.7 ms	+4.51%
❌	`sort_reverse_mixed`	38.2 ms	39.2 ms	-2.57%
❌	`ls_recursive_wide_tree[(10000, 1000)]`	51.3 ms	52.5 ms	-2.33%
❌	`cp_large_file[16]`	341.3 µs	359 µs	-4.94%
❌	`mv_single_file`	129.7 ms	136 ms	-4.63%
❌	`mv_force_overwrite`	125.3 ms	138.4 ms	-9.51%
❌	`factor_multiple_u64s[2]`	178.3 ms	213.3 ms	-16.41%
❌	`split_number_chunks`	275.4 µs	287.7 µs	-4.27%
❌	`rm_single_file`	106.6 ms	119.6 ms	-10.91%

6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

github-actions · 2025-12-01T02:28:07Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/cksum/cksum-c is no longer failing!

github-actions · 2025-12-01T10:56:05Z

GNU testsuite comparison:

Congrats! The gnu test tests/cksum/cksum-c is no longer failing!

github-actions · 2025-12-01T18:49:10Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

RenjiSann · 2025-12-03T10:05:08Z

It looks like the localized messages I added add a significant overhead to many other tools :/ Not sure how to improve this.

tests/by-util/test_cksum.rs

cakebaker · 2025-12-04T09:56:18Z

src/uucore/src/lib/features/checksum/compute.rs

-            sum.parse::<u16>().unwrap(),
+    match (options.algo_kind, sum) {
+        (SizedAlgoKind::Sysv, DigestOutput::U16(sum)) => print!(
+            "{prefix}{sum} {}",


Is the printing of prefix here, and in the other arms of this match, correct? I'm asking because it is already printed on line 155.

Indeed it's wrong, I forgot to remove the print above

src/uucore/src/lib/features/checksum/validate.rs

github-actions · 2025-12-04T13:40:34Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

cakebaker · 2025-12-04T14:02:57Z

src/uucore/src/lib/features/sum.rs

-use hex::encode;
+use std::io::{self, Write};
+
+use data_encoding::BASE64;


You forgot to add data_encoding to the sum feature in src/uucore/Cargo.toml. Currently there is an "unresolved import data_encoding" error when I try to run cargo test --features=sum --no-default-features

cakebaker · 2025-12-04T14:26:33Z

src/uucore/src/lib/features/sum.rs

+impl_digest_shake!(Shake128, 256);
+impl_digest_shake!(Shake256, 512);


Are the values 256 and 512 correct? With the impl_digest_common macro, the values correspond to the bits in the algorithm name.

From what I saw, Shake algorithms are not available in GNU, so I have no point of comparison. I just used the values that kept the test passing while still removing the code that previously handled the specific case of Shake.

cakebaker · 2025-12-04T14:32:40Z

src/uucore/src/lib/features/sum.rs

 // Implements the Digest trait for sha2 / sha3 algorithms with variable output
 macro_rules! impl_digest_shake {
-    ($algo_type: ty) => {
+    ($algo_type: ty, $output_bits: literal) => {


What's the reason that the definition here is different than the one used for impl_digest_common (literal vs expr)?

literal is more restrictive than expr, and it's enough since I'm writing the number directly. I can switch to expr in case we want to change this later

I would use literal in both cases. In the other macro the number is also written directly.

cakebaker · 2025-12-04T15:55:26Z

src/uucore/src/lib/features/checksum/validate.rs

-                .all(|&b| b.is_ascii_alphanumeric() || b == b'+' || b == b'/')
-        {
-            return None;
+        while index < checksum.len() {


I would use a for loop so you don't have to increase index yourself.

Suggested change

while index < checksum.len() {

for index in 0..checksum.len() {

cakebaker · 2025-12-04T15:57:23Z

src/uucore/src/lib/features/checksum/validate.rs

+                // ASCII alphanumeric
+                [b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9', ..] => {
+                    index += 1;
+                    continue;


The continue is not necessary.

Suggested change

continue;

cakebaker · 2025-12-04T15:57:45Z

src/uucore/src/lib/features/checksum/validate.rs

+                [b'+' | b'/', ..] => {
+                    is_base64 = true;
+                    index += 1;
+                    continue;


Same here.

Suggested change

continue;

cakebaker · 2025-12-04T16:00:48Z

src/uucore/src/lib/features/checksum/validate.rs

            // division by 2 converts the length of the Blake2b checksum from
            // hexadecimal characters to bytes, as each byte is represented by
            // two hexadecimal characters.


The comment no longer matches the code.

Suggested change

// division by 2 converts the length of the Blake2b checksum from

// hexadecimal characters to bytes, as each byte is represented by

// two hexadecimal characters.

cakebaker · 2025-12-04T16:01:53Z

src/uucore/src/lib/features/checksum/validate.rs

+            (AlgoKind::Blake2b, Some(expected_checksum.len()))
        }
        algo @ (AlgoKind::Sha2 | AlgoKind::Sha3) => {
            // multiplication by 4 to get the number of bits


Suggested change

// multiplication by 4 to get the number of bits

// multiplication by 8 to get the number of bits

cakebaker

Done :)

... to prevent a preemptive computation of the hex encoding.

…um_format

to avoid decoding base64 and directly re-encoding it in hexadecimal

…-input filename print

github-actions · 2025-12-04T23:23:32Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

RenjiSann self-assigned this Dec 1, 2025

RenjiSann force-pushed the cksum-improvements branch from 5461873 to c6b9693 Compare December 1, 2025 01:59

RenjiSann changed the title ~~cksum: small improvements~~ cksum: small improvements, l10n Dec 1, 2025

RenjiSann force-pushed the cksum-improvements branch from c6b9693 to be080a2 Compare December 1, 2025 02:12

RenjiSann force-pushed the cksum-improvements branch 2 times, most recently from 4e9151b to 086d610 Compare December 1, 2025 10:40

RenjiSann mentioned this pull request Dec 1, 2025

cksum: Fix GNU test cksum-c.sh after bump to 9.9 #9511

Merged

RenjiSann force-pushed the cksum-improvements branch 2 times, most recently from c96ed79 to dc8063d Compare December 1, 2025 18:32

RenjiSann marked this pull request as ready for review December 3, 2025 10:05

cakebaker reviewed Dec 4, 2025

View reviewed changes

tests/by-util/test_cksum.rs Outdated Show resolved Hide resolved

cakebaker reviewed Dec 4, 2025

View reviewed changes

src/uucore/src/lib/features/checksum/validate.rs Outdated Show resolved Hide resolved

RenjiSann force-pushed the cksum-improvements branch from dc8063d to 7ba5109 Compare December 4, 2025 13:24

cakebaker reviewed Dec 4, 2025

View reviewed changes

RenjiSann added 6 commits December 4, 2025 18:16

checksum: Introduce a DigestOutput type...

3d23a02

... to prevent a preemptive computation of the hex encoding.

checksum(validate): Simplify and optimize LineFormat::validate_checks…

ae9162a

…um_format

checksum(validate): Check calculated checksum against raw expected

7f09fd6

to avoid decoding base64 and directly re-encoding it in hexadecimal

test(cksum): Add test for ignore-missing standard input

f6dcae5

checksum(validate): Remove called-once simple functions, fix standard…

b823ac2

…-input filename print

l10n(uucore::checksum): Implement l10n for English + French

935f0a9

RenjiSann force-pushed the cksum-improvements branch from 7ba5109 to 935f0a9 Compare December 4, 2025 23:07

		impl_digest_shake!(Shake128, 256);
		impl_digest_shake!(Shake256, 512);

	while index < checksum.len() {
	for index in 0..checksum.len() {

	// division by 2 converts the length of the Blake2b checksum from
	// hexadecimal characters to bytes, as each byte is represented by
	// two hexadecimal characters.

	// multiplication by 4 to get the number of bits
	// multiplication by 8 to get the number of bits

Uh oh!

cksum: small improvements, l10n #9532

Are you sure you want to change the base?

cksum: small improvements, l10n #9532

Conversation

RenjiSann commented Dec 1, 2025

Uh oh!

github-actions bot commented Dec 1, 2025

Uh oh!

codspeed-hq bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #9532 will degrade performances by 16.41%

Summary

Benchmarks breakdown

Footnotes

Uh oh!

github-actions bot commented Dec 1, 2025

Uh oh!

github-actions bot commented Dec 1, 2025

Uh oh!

github-actions bot commented Dec 1, 2025

Uh oh!

RenjiSann commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Dec 4, 2025

Uh oh!

cakebaker Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cakebaker Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cakebaker Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cakebaker left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codspeed-hq bot commented Dec 1, 2025 •

edited

Loading

RenjiSann commented Dec 3, 2025 •

edited

Loading

cakebaker Dec 4, 2025 •

edited

Loading

cakebaker Dec 4, 2025 •

edited

Loading

cakebaker Dec 4, 2025 •

edited

Loading