Skip to content

Refactor uniq field skipping to avoid allocations #8703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Sep 29, 2025
Merged

Refactor uniq field skipping to avoid allocations #8703

merged 7 commits into from
Sep 29, 2025

Conversation

mattsu2020
Copy link
Contributor

Summary

Reworked skip_fields to walk the input buffer directly and return the borrowed tail slice, eliminating the repeated Vec allocations while preserving the existing whitespace/field semantics.

Updated cmp_key to operate on the borrowed slice so that byte-based --skip-chars handling and the invalid-UTF-8 fallback continue to match the original behavior without extra copies.

Testing

✅ cargo test uniq --features=default

Benchmark

Baseline target/debug/uniq_baseline --skip-fields=3 --skip-chars=10 uniq_bench_input.txt > /dev/null: 18.475 s, 18.113 s, 17.271 s (avg 17.95 s).

Refactored target/debug/uniq --skip-fields=3 --skip-chars=10 uniq_bench_input.txt > /dev/null: 1.348 s, 1.288 s, 1.339 s (avg 1.33 s, ≈13.5× faster)

@sylvestre
Copy link
Contributor

Could you please the benchmark with hyperfine? With gnu, baseline and your version.
It is much better for benchmarks

@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented Sep 22, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #8703 will improve performances by ×7.3

Comparing mattsu2020:performance_tuning (1e25202) with main (f7490ca)

Summary

⚡ 2 improvements
✅ 42 untouched
⏩ 64 skipped1

Benchmarks breakdown

Benchmark BASE HEAD Change
⚡ uniq_case_insensitive[10000] 6 ms 2 ms ×3
⚡ uniq_with_count[10000] 13.4 ms 1.8 ms ×7.3

Footnotes

  1. 64 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 GitHub Actions
Copy link

GNU testsuite comparison:

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

@mattsu2020
Copy link
Contributor Author

performance improvement

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

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

sylvestre
sylvestre requested changes Sep 22, 2025
View reviewed changes
Copy link
Contributor

@sylvestre sylvestre left a comment

Choose a reason for hiding this comment

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

please fix the various job failures

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

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

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

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

@sylvestre
Copy link
Contributor

Could you please share/run the benchmark with hyperfine? With gnu, baseline and your version.

@mattsu2020
Copy link
Contributor Author

mattsu2020 commented Sep 23, 2025 •
edited
Loading

benchmark Report: uniq (uutils) vs system uniq

Objective: Compare the performance of the uutils uniq against the system /usr/bin/uniq using hyperfine.
Environment:
uutils binary: target/release/uniq
system binary: /usr/bin/uniq
　　　　　　　mac os macOS 26.0
datasets: /tmp/uniq_bench/{small.txt, medium.txt, large_runs.txt, sorted.txt}
runner: hyperfine -w 3 -m 10 (Markdown outputs saved as /tmp/uniq_bench/*.md)

Method:

Command form: cat | > /dev/null

Four datasets:

small.txt (~5k lines, two values)
medium.txt (200k unique-ish 16-char tokens)
large_runs.txt (1M lines with long runs of A/B/C)
sorted.txt (500k sorted random tokens)

Results (mean time, relative speed):

small.txt: System uniq is ~1.11x faster (both ~5 ms; shell overhead significant)
medium.txt: uutils uniq is ~2.16x faster
large_runs.txt: uutils uniq is ~2.63x faster
sorted.txt: uutils uniq is ~2.69x faster

small.txt
sorted.txt
large_runs.txt
medium.txt

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

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

@sylvestre
Copy link
Contributor

could you please share the full output of hyperfine ?

@sylvestre
Copy link
Contributor

i think your usage of codex is a bit too heavy
it completely rewrote the tool make it is hard to review.

currently, this won't be merged given all the changes you have made

@mattsu2020
Copy link
Contributor Author

small.md
sorted.md
medium.md
large_runs.md

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

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

@github-actions GitHub Actions
Copy link

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/misc/tee (fails in this run but passes in the 'main' branch)

@sylvestre sylvestre force-pushed the performance_tuning branch 2 times, most recently from f6d61aa to f415c6d Compare September 28, 2025 20:21
@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (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)

mattsu2020 and others added 7 commits September 29, 2025 20:29
@mattsu2020 @sylvestre
refactor uniq skip_fields to avoid allocations
5f7a8cd
@mattsu2020 @sylvestre
feat(uniq): add read error handling and optimize line processing
a757450 
- Add read error localization keys to en-US and fr-FR locales
- Refactor print_uniq to use buffered line reading with LineMeta for case-insensitive comparisons, avoiding memory issues with large inputs
- Improve error handling by detecting read failures and exiting appropriately
@mattsu2020 @sylvestre
fix,build_count_prefix
4d8aa5a
@mattsu2020 @sylvestre
ci test
fe2b1a8
@mattsu2020 @sylvestre
fix &self
ae84112
@mattsu2020 @sylvestre
fix fmt
b0a31c3
@mattsu2020 @sylvestre
fix fmt
1e25202
@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

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

@sylvestre
Copy link
Contributor

bravo for the perf improvements!

image

@sylvestre sylvestre merged commit 31d1f06 into uutils:main Sep 29, 2025
97 of 98 checks passed
@mattsu2020 mattsu2020 deleted the performance_tuning branch October 3, 2025 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sylvestre sylvestre Awaiting requested review from sylvestre

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mattsu2020 @sylvestre