Skip to content

Sort: bench ahash#10783

Open
oech3 wants to merge 1 commit intouutils:mainfrom
oech3:sort-ahash
Open

Sort: bench ahash#10783
oech3 wants to merge 1 commit intouutils:mainfrom
oech3:sort-ahash

Conversation

@oech3
Copy link
Contributor

@oech3 oech3 commented Feb 7, 2026

No description provided.

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 7, 2026

Merging this PR will improve performance by 13.34%

⚡ 5 improved benchmarks
✅ 279 untouched benchmarks
⏩ 38 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation sort_dictionary_order[500000] 1.1 s 1 s +9.79%
Simulation sort_numeric[500000] 1.2 s 1 s +13.34%
Simulation sort_general_numeric[200000] 721.1 ms 659.5 ms +9.33%
Simulation sort_key_field[500000] 853.4 ms 783.2 ms +8.96%
Simulation sort_case_insensitive[500000] 257.2 ms 249 ms +3.28%

Comparing oech3:sort-ahash (970f6b2) with main (3d56517)

Open in CodSpeed

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.

uu_wc,
uu_factor,
uu_date
uu_sort
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you remove them ? it isn't helping much ?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Useful to call codspeed quickly.

@sylvestre
Copy link
Contributor

Merging this PR will improve performance by 14.56%

impressive!

@xtqqczze
Copy link
Contributor

xtqqczze commented Feb 7, 2026

Merging this PR will improve performance by 14.56%

Interesting, using rustc-hash showed no change in #10690

@oech3
Copy link
Contributor Author

oech3 commented Feb 7, 2026

Interesting, using rustc-hash showed no change in

Because I kept 1 Fnv to unbreak sort -R. I will test rustc-hash at internal Hash* (non-RNG part).

@oech3
Copy link
Contributor Author

oech3 commented Feb 7, 2026

Also we reciently applied codegen-units=1 to release profile.
So previous 9.* % is not used for comparison.

@xtqqczze
Copy link
Contributor

xtqqczze commented Feb 7, 2026

it's probably worth trying foldhash as well?

@oech3 oech3 force-pushed the sort-ahash branch 6 times, most recently from b4aa98b to bd12116 Compare February 7, 2026 12:26
use std::ffi::{OsStr, OsString};
use std::fs::{File, OpenOptions};
use std::hash::{Hash, Hasher};
use std::hash::BuildHasher as _;
Copy link
Contributor

Choose a reason for hiding this comment

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

why "as _" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to avoid confliction in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applying clippy removed it...

Copy link
Contributor

Choose a reason for hiding this comment

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

i would prefer we don't have this
it is a pretty ugly in the code base
i would prefer you remove the " as " in use ahash::AHashMap as HashMap;
instead
and change the name
it isn't like we are going not change the hashmap on a regular basis

Copy link
Contributor Author

@oech3 oech3 Feb 7, 2026

Choose a reason for hiding this comment

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

@xtqqczze recommended shadowing. Should we revert also for other utils?

I switched to AHash* at here anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, i don't like shadowing

@github-actions
Copy link

github-actions bot commented Feb 7, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/tail/follow-name (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)

@github-actions
Copy link

github-actions bot commented Feb 7, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/tail/follow-name (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)

@github-actions
Copy link

github-actions bot commented Feb 7, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/tail/follow-name (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)

@oech3 oech3 marked this pull request as ready for review February 7, 2026 14:37
@github-actions
Copy link

github-actions bot commented Feb 7, 2026

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/usage_vs_getopt (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)

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