Skip to content

Conversation

@ChrisDryden
Copy link
Collaborator

@ChrisDryden ChrisDryden commented Dec 29, 2025

This is the third PR based off of the following prototype: #9866

Now the original SMACK implementation for ls is implemented alongside the CI runner for the SMACK GNU tests and now its time to actually implement SMACK support for the different utilities.

@ChrisDryden ChrisDryden force-pushed the smack-utility-support branch 2 times, most recently from d1808f0 to 070b999 Compare December 29, 2025 16:31
@ChrisDryden ChrisDryden force-pushed the smack-utility-support branch 2 times, most recently from a6003eb to f2fb5e4 Compare December 29, 2025 16:43
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 30, 2025

CodSpeed Performance Report

Merging this PR will degrade performance by 6.37%

Comparing ChrisDryden:smack-utility-support (557f1be) with main (130f780)

Summary

⚡ 1 improved benchmark
❌ 11 regressed benchmarks
✅ 127 untouched benchmarks
⏩ 37 skipped benchmarks1

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

Performance Changes

Benchmark BASE HEAD Efficiency
wc_bytes_synthetic[500] 185.7 µs 192 µs -3.32%
cksum_blake3 224.9 µs 218.2 µs +3.07%
b64_decode_ignore_garbage_synthetic 162.6 µs 168.9 µs -3.77%
b64_encode_synthetic 161.2 µs 168.6 µs -4.41%
b64_decode_synthetic 165.6 µs 171 µs -3.15%
sort_ascii_utf8_locale 20.1 ms 21.4 ms -6.37%
dd_copy_partial 550.5 µs 572.3 µs -3.81%
rm_single_file 118.9 ms 123.8 ms -3.96%
mv_single_file 135 ms 140.9 ms -4.13%
mv_force_overwrite 143.8 ms 149.5 ms -3.79%
factor_multiple_u64s[2] 211.1 ms 224.2 ms -5.88%
single_date_now 184.4 µs 190.7 µs -3.34%

Footnotes

  1. 37 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.

@ChrisDryden ChrisDryden force-pushed the smack-utility-support branch from f2fb5e4 to 05fd505 Compare December 30, 2025 21:43
@ChrisDryden ChrisDryden marked this pull request as ready for review December 30, 2025 22:37
@ChrisDryden
Copy link
Collaborator Author

I'm not sure why the GNU comment is not posting?

I'm seeing this in the workflow but it didn't post the comment
Notice: Congrats! The gnu test tests/id/smack is no longer failing!
Notice: Congrats! The gnu test tests/mkdir/smack-no-root is no longer failing!
Notice: Congrats! The gnu test tests/mkdir/smack-root is no longer failing!

@sylvestre
Copy link
Contributor

i can take some time

also, please fix:

  error: redundant else block
     --> src/uu/id/src/id.rs:186:14
      |
  186 |               } else {
      |  ______________^
  187 | |                 return Err(USimpleError::new(
  188 | |                     1,
  189 | |                     translate!("id-error-cannot-get-context"),
  190 | |                 ));
  191 | |             }
      | |_____________^
      |
      = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#redundant_else
      = note: `-D clippy::redundant-else` implied by `-D warnings`
      = help: to override `-D warnings` add `#[allow(clippy::redundant_else)]`
  help: remove the `else` block and move the contents out
      |
  186 ~             }
  187 +             return Err(USimpleError::new(
  188 +                 1,
  189 +                 translate!("id-error-cannot-get-context"),
  190 +             ));
      |

@ChrisDryden
Copy link
Collaborator Author

As a note about the performance regressions, its from the default locale file growing bigger. This can be improved by duplicating it to the individual utility locales, could go either way, just leaving it in the main locale file for now.

@ChrisDryden ChrisDryden force-pushed the smack-utility-support branch from 05fd505 to a34b2f0 Compare December 30, 2025 22:44
@sylvestre
Copy link
Contributor

As a note about the performance regressions, its from the default locale file growing bigger. This can be improved by duplicating it to the individual utility locales, could go either way, just leaving it in the main locale file for now.

We should work on this at some point, we should not have such perf regressions for a few strings

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/id/smack is no longer failing!
Congrats! The gnu test tests/mkdir/smack-no-root is no longer failing!
Congrats! The gnu test tests/mkdir/smack-root is no longer failing!

@ChrisDryden
Copy link
Collaborator Author

Created an issue to track the shared locale files impacting performance: #9932 I think its a combination of gating the locale files based on feature flag and also coming up with mechanisms to reduce the burden that locales have on performance

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/id/smack is no longer failing!
Congrats! The gnu test tests/mkdir/smack-no-root is no longer failing!
Congrats! The gnu test tests/mkdir/smack-root is no longer failing!

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/id/smack is no longer failing!
Congrats! The gnu test tests/mkdir/smack-no-root is no longer failing!
Congrats! The gnu test tests/mkdir/smack-root is no longer failing!

@oech3
Copy link
Contributor

oech3 commented Jan 2, 2026

@ChrisDryden Would you switch to curl -L https://archlinux.org/packages/core/x86_64/linux/download/?
Curren URL cause many 404 not found.

@oech3
Copy link
Contributor

oech3 commented Jan 2, 2026

It seems this is not a reason for CI breakage :|

@ChrisDryden
Copy link
Collaborator Author

It seems like they updated the default download page, will update that now

@github-actions
Copy link

github-actions bot commented Jan 5, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/id/smack is no longer failing!
Congrats! The gnu test tests/mkdir/smack-no-root is no longer failing!
Congrats! The gnu test tests/mkdir/smack-root is no longer failing!

@ChrisDryden ChrisDryden requested a review from sylvestre January 6, 2026 18:22
@github-actions
Copy link

github-actions bot commented Jan 6, 2026

GNU testsuite comparison:

Congrats! The gnu test tests/id/smack is no longer failing!
Congrats! The gnu test tests/mkdir/smack-no-root is no longer failing!
Congrats! The gnu test tests/mkdir/smack-root is no longer failing!

@sylvestre sylvestre merged commit 20a5c3a into uutils:main Jan 7, 2026
132 of 133 checks passed
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