Skip to content

Conversation

@naoNao89
Copy link
Contributor

@naoNao89 naoNao89 commented Oct 17, 2025

Bug existed since Sept 2024 but variance was below CodSpeed's threshold (~1-3%). On Oct 11, CI changed to run 19 benchmarks in parallel (f19d7ec, 6df73f0) instead of sequentially. This amplified /tmp contention, pushing variance to 2.64%—above detection threshold. The parallel execution exposed what was always there.

Solution

Move NamedTempFile::new() outside loop, matching sort_bench.rs pattern:

Eliminates filesystem noise while preserving correct semantics (sort overwrites file each iteration, no caching)

Notes
Affected PRs: #8935, #8936, #8927, #8926, #8930, #8932.

Locale benchmarks were creating temp files inside the bench loop (from d1cd999), causing filesystem noise and false CodSpeed regressions. The same commit's sort_bench.rs got it right with file creation outside the loop. This fix aligns with that pattern.
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 17, 2025

CodSpeed Performance Report

Merging #8939 will degrade performances by 3.36%

Comparing naoNao89:fix/sort-benchmark-variance (607383a) with main (0258583)

Summary

❌ 2 regressions
✅ 104 untouched
⏩ 73 skipped1

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

Benchmarks breakdown

Benchmark BASE HEAD Change
du_balanced_tree[(5, 4, 10)] 9.1 ms 9.3 ms -2.01%
du_human_balanced_tree[(5, 4, 10)] 10.1 ms 10.5 ms -3.36%

Footnotes

  1. 73 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
Copy link

GNU testsuite comparison:

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

@sylvestre sylvestre merged commit a255a1f into uutils:main Oct 17, 2025
120 of 121 checks passed
@naoNao89 naoNao89 deleted the fix/sort-benchmark-variance branch October 17, 2025 18:06
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