-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(sort): GNU sort-continue.sh test #9107
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.
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
base: main
Are you sure you want to change the base?
Conversation
…mits - Add `effective_merge_batch_size()` function to calculate batch size considering fd soft limit, with minimums and safety margins. - Generalize fd limit handling from Linux-only to Unix systems using `fd_soft_limit()`. - Update merge logic to use dynamic batch size instead of fixed `settings.merge_batch_size` to prevent fd exhaustion.
…dling Replace direct call to get_rlimit()? with fd_soft_limit(), adding a check for None value to return a usage error if rlimit cannot be fetched. This improves robustness on Linux by ensuring proper error handling when retrieving the file descriptor soft limit.
CodSpeed Performance ReportMerging #9107 will degrade performances by 2.02%Comparing Summary
Benchmarks breakdown
Footnotes
|
|
GNU testsuite comparison: |
|
@mattsu2020 hey, thanks for the PR. to fix the docs related issues, you'll need to escape the brackets as in the error log. The rest of the CI fails are unrelated. |
|
GNU testsuite comparison: |
This has been improved in another commit. |
|
@sylvestre can you restart the CI, this seems ready to merge :) |
|
GNU testsuite comparison: |
|
@mattsu2020 RLIM_INFINITY doesn't seem to be available there. |
Update conditional compilation attributes from #[cfg(unix)] to #[cfg(target_os = "linux")] for the nix::libc import and fd_soft_limit function implementations, ensuring these features are only enabled on Linux systems to improve portability and avoid issues on other Unix-like platforms.
Modified to restrict it to Linux |
|
GNU testsuite comparison: |
Adjust fd_soft_limit helper to expose the soft RLIMIT for reuse across the codebase.
Respect the soft limit when determining merge batch sizes so the external merge never exceeds the available file descriptors.
Fallback to a useful error message when retrieving the limit fails while parsing --batch-size.
Tests:
cargo build -p uu_sort
ulimit -n 7; ./target/debug/sort -n -m __test.* … (GNU sort-continue scenario)
ulimit -n 7; ./target/debug/sort -n -m __test.* - …