Skip to content

sort: merge lazily in UTF-8 locales instead of precomputing sort keys#13205

Open
sylvestre wants to merge 1 commit into
uutils:mainfrom
sylvestre:sort-merge-perf
Open

sort: merge lazily in UTF-8 locales instead of precomputing sort keys#13205
sylvestre wants to merge 1 commit into
uutils:mainfrom
sylvestre:sort-merge-perf

Conversation

@sylvestre

Copy link
Copy Markdown
Contributor

Benchmark 1: uutils OLD
  Time (mean ± σ):      82.4 ms ±   3.8 ms    [User: 81.6 ms, System: 7.8 ms]
  Range (min … max):    78.6 ms …  94.7 ms    37 runs

Benchmark 2: uutils NEW
  Time (mean ± σ):       9.2 ms ±   0.6 ms    [User: 12.4 ms, System: 4.5 ms]
  Range (min … max):     8.1 ms …  11.2 ms    312 runs

Benchmark 3: GNU sort
  Time (mean ± σ):       7.5 ms ±   1.3 ms    [User: 6.5 ms, System: 0.9 ms]
  Range (min … max):     5.3 ms …  12.4 ms    421 runs

Summary
  GNU sort ran
    1.23 ± 0.23 times faster than uutils NEW
   10.93 ± 1.99 times faster than uutils OLD

sort -m /usr/share/dict/words in a UTF-8 locale goes from ~70ms to ~9ms (10.5x -> 1.32x vs GNU sort), with byte-identical output. Other sort modes and the C locale are unaffected.

In a UTF-8 locale `sort` precomputes a full ICU collation sort key for
every line while parsing chunks. The regular sort path needs this to
amortize O(n log n) comparisons, but merging only performs O(n log k)
comparisons (and none at all when merging a single already-sorted file),
so the keys are pure overhead there.

Add merge_compare(), which mirrors compare_by() but compares whole-line
locale keys lazily with the collator. The merge reader now runs with
fast_locale_collation disabled so Line::create no longer computes
per-line sort keys, and the merge comparator and dedup use merge_compare.

`sort -m /usr/share/dict/words` in a UTF-8 locale goes from ~70ms to
~9ms (10.5x -> 1.32x vs GNU sort), with byte-identical output. Other
sort modes and the C locale are unaffected.
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/printf/printf-surprise is now passing!

@codspeed-hq

codspeed-hq Bot commented Jun 29, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 6.65%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 5 regressed benchmarks
✅ 322 untouched benchmarks
⏩ 46 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation merge_pre_sorted_files 103 ms 113.9 ms -9.58%
Simulation merge_pre_sorted_files_utf8_locale 102.8 ms 113.6 ms -9.55%
Simulation sort_ascii_utf8_locale 15.4 ms 16.1 ms -4.67%
Simulation sort_ascii_c_locale 16 ms 16.7 ms -4.67%
Simulation sort_key_field[500000] 767.1 ms 804.3 ms -4.63%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing sylvestre:sort-merge-perf (c5c51af) with main (b76d615)

Open in CodSpeed

Footnotes

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

Comment thread src/uu/sort/src/sort.rs

/// Comparison used by the merge path.
///
/// This is result-identical to [`compare_by`], but for the whole-line locale-collation

@cakebaker cakebaker Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The compare_by link makes the Documentation/warnings job fail:

public documentation for `merge_compare` links to private item `compare_by`
    --> src/uu/sort/src/sort.rs:2626:35
     |
2626 | /// This is result-identical to [`compare_by`], but for the whole-line locale-collation
     |                                   ^^^^^^^^^^ this item is private

Comment thread src/uu/sort/src/sort.rs
Comment on lines +2633 to +2639
pub fn merge_compare<'a>(
a: &Line<'a>,
b: &Line<'a>,
settings: &GlobalSettings,
a_line_data: &LineData<'a>,
b_line_data: &LineData<'a>,
) -> Ordering {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The function signature looks a bit odd with settings in the middle, I would have expected it at the end. But I think it's something for a future PR.

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.

2 participants