Skip to content

shuf: Reduce malloc#10998

Open
oech3 wants to merge 1 commit intouutils:mainfrom
oech3:shuf-malloc
Open

shuf: Reduce malloc#10998
oech3 wants to merge 1 commit intouutils:mainfrom
oech3:shuf-malloc

Conversation

@oech3
Copy link
Contributor

@oech3 oech3 commented Feb 17, 2026

No description provided.

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 17, 2026 •
edited
Loading

Merging this PR will improve performance by 15.19%

⚡ 2 improved benchmarks
✅ 286 untouched benchmarks
⏩ 38 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Simulation shuf_input_range[1000000] 92.5 ms 87 ms +6.39%
⚡ Memory shuf_input_range[1000000] 8.9 MB 7.7 MB +15.19%

Comparing oech3:shuf-malloc (d6715e6) with main (1e95554)

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. ↩

xtqqczze
xtqqczze reviewed Feb 17, 2026
View reviewed changes
src/uu/shuf/src/nonrepeating_iterator.rs Outdated
Copy link
Contributor

@xtqqczze xtqqczze Feb 17, 2026

Choose a reason for hiding this comment

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

extract 8_192 to a constant and document

Copy link
Contributor

@xtqqczze xtqqczze Feb 17, 2026

Choose a reason for hiding this comment

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

its doesn't really help, why is 8K chosen, is it related to page size?

Copy link
Contributor Author

@oech3 oech3 Feb 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

No. Just to avoid huge allocation

Copy link
Contributor Author

@oech3 oech3 Feb 17, 2026

Choose a reason for hiding this comment

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

Also I'm not clear why don't we use Vec at 1st.
Maybe, size with few times hashbrown scan?

@oech3
Copy link
Contributor Author

oech3 commented Feb 17, 2026

shuf_lines[100000]
-3%
27.6 ms -> 28.3 ms

@oech3 oech3 force-pushed the shuf-malloc branch 4 times, most recently from 3df1603 to 622c06d Compare February 17, 2026 18:37
@oech3
Copy link
Contributor Author

oech3 commented Feb 17, 2026

Hmm, I can't fix shuf_lines[100000]. Should I undraft this as a trade-off?

@oech3 oech3 force-pushed the shuf-malloc branch 3 times, most recently from e00e478 to 92205f2 Compare February 17, 2026 19:09
@github-actions
Copy link

github-actions bot commented Feb 17, 2026

GNU testsuite comparison:

GNU test failed: tests/cut/bounded-memory. tests/cut/bounded-memory is passing on 'main'. Maybe you have to rebase?
Note: The gnu test tests/printf/printf-surprise is now being skipped but was previously passing.
Congrats! The gnu test tests/cp/link-heap is now passing!

@oech3 oech3 marked this pull request as ready for review February 17, 2026 19:43
xtqqczze
xtqqczze reviewed Feb 17, 2026
View reviewed changes
xtqqczze
xtqqczze reviewed Feb 17, 2026
View reviewed changes
src/uu/shuf/src/nonrepeating_iterator.rs
impl<'a> NonrepeatingIterator<'a> {
pub(crate) fn new(range: RangeInclusive<u64>, rng: &'a mut WrappedRng) -> Self {
let values = Values::Sparse(range, HashMap::default());
let avoid_reallocation_bound = 128; // todo: optimize this
Copy link
Contributor

@xtqqczze xtqqczze Feb 17, 2026

Choose a reason for hiding this comment

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

should be const, maybe MAX_CAPACITY?

Copy link
Contributor Author

@oech3 oech3 Feb 17, 2026

Choose a reason for hiding this comment

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

How about hardcoding and leaving // set initial capacity to avoid reallocation. But current capacity might not optimal?

@github-actions
Copy link

github-actions bot commented Feb 17, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/pr/bounded-memory (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/cp/link-heap is now passing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@xtqqczze xtqqczze xtqqczze left review comments

Reviewers whose approvals may not affect merge requirements

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

@oech3 @xtqqczze

Comments