Skip to content

Conversation

@xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Jan 20, 2026

Currently, we construct uucore::Args within the measured section of benchmarks. That adds allocation and conversion overhead and will skew the results.

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/tail/retry. tests/tail/retry is passing on 'main'. Maybe you have to rebase?

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 20, 2026

CodSpeed Performance Report

Merging this PR will degrade performance by 26.29%

Comparing xtqqczze:get_benchmark_args (1936e7b) with main (0f7a7c4)

Summary

⚡ 8 improved benchmarks
❌ 3 regressed benchmarks
✅ 273 untouched benchmarks
⏩ 38 skipped benchmarks1

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

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory sort_ascii_utf8_locale 5.6 MB 6.1 MB -8.02%
Memory sort_reverse_utf8_locale 2.6 MB 3.5 MB -26.29%
Memory sort_numeric[500000] 48.7 MB 44.8 MB +8.72%
Memory sort_key_field[500000] 32.8 MB 28.9 MB +13.45%
Memory cksum_sm3 66.5 KB 64 KB +3.85%
Memory cksum_sha2 70.7 KB 60.8 KB +16.4%
Memory cksum_md5 64 KB 66.5 KB -3.76%
Memory cksum_sha512 59.2 KB 56.6 KB +4.55%
Memory cksum_default 58.5 KB 56.3 KB +3.92%
Memory cksum_blake2b 59.1 KB 55.4 KB +6.71%
Simulation cp_large_file[16] 385.9 µs 366.5 µs +5.3%

Footnotes

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

@xtqqczze
Copy link
Contributor Author

@sylvestre Please could you confirm whether these changes are viable to apply in full?

@sylvestre
Copy link
Contributor

i guess ? :)

@xtqqczze
Copy link
Contributor Author

Need to refactor to handle the following case:

bencher.bench(|| {
fs_utils::remove_path(&dest);
let mut full_args = Vec::with_capacity(args.len() + 2);
full_args.extend_from_slice(args);
full_args.push(source_str);
full_args.push(dest_str);

@oech3
Copy link
Contributor

oech3 commented Jan 26, 2026

Should we wait working about #10486 until your PR was merged?
Any chance for splitting PR?

xtqqczze added a commit to xtqqczze/uutils-coreutils that referenced this pull request Jan 26, 2026
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