-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(sort): auto-tune buffer sizing from available memory #8959
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
|
GNU testsuite comparison: |
CodSpeed Performance ReportMerging #8959 will degrade performances by 3.03%Comparing Summary
Benchmarks breakdown
Footnotes
|
Added 'sysconf' to the CSpell jargon dictionary to include the system configuration function term, preventing false positives in spell-checking for technical code.
|
i added comments in the other PR |
|
GNU testsuite comparison: |
Moved buffer size calculation functions (e.g., automatic_buffer_size, file_size_hint) to a new buffer_hint module for better code organization and modularity. Removed the Linux-specific nix dependency as the memory hint functionality is now handled internally without external crates.
Updated the `physical_memory_bytes_unix` function to explicitly cast `pages` and `page_size` to `u128` before multiplication, ensuring safe arithmetic and preventing potential overflow. Also added "libc" dependency to `fuzz/Cargo.lock` to support the changes.
…ory_bytes_unix Reformatted the multi-line `#[cfg(...)]` attribute and reordered imports in `physical_memory_bytes_unix()` for better code clarity and consistency.
…bytes_unix Minor code style cleanup to improve readability and consistency in the buffer hint module.
…y_bytes The explicit return is redundant in Rust, as the last expression in a block is implicitly returned, improving code style and adherence to idiomatic practices.
Corrected "overcommitting" to "overcommit" in the comment for accurate spelling.
|
https://github.com/nix-rust/nix/blob/e1e630f1b942a222f59c42cee63af16b2a925f15/src/unistd.rs#L3138C1-L3141C40 |
|
GNU testsuite comparison: |
Added standard copyright and license header to the buffer_hint.rs file in the sort utility to comply with project licensing requirements and ensure proper attribution.
|
GNU testsuite comparison: |
Removal of unnecessary variables Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>
src/uu/sort/src/sort.rs
Outdated
| #[cfg(target_os = "linux")] | ||
| { | ||
| match get_rlimit() { | ||
| Ok(limit) => limit.saturating_div(4).clamp(32, 256), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please explain what it is doing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8000The batch size is dynamically adjusted based on the number of file descriptors. Comments and processing have been modified for clarity.
Added a comment in the `physical_memory_bytes` function for non-Unix or Redox targets to clarify why `None` is returned, improving code readability and documenting the absence of a portable API for detecting total physical memory.
Updated cfg conditions in `physical_memory_bytes` to explicitly handle Linux and Android targets. Refined `physical_memory_bytes_unix` with improved error handling for sysconf calls, ensuring safer memory page and size calculations while removing fallback libc implementation for non-Linux/Android Unix systems.
|
GNU testsuite comparison: |
… batch size Removed the libc crate dependency from Cargo.toml and Cargo.lock to reduce unnecessary dependencies. Refactored default_merge_batch_size() in sort.rs to use named constants (LINUX_BATCH_DIVISOR, LINUX_BATCH_MIN, LINUX_BATCH_MAX) for better code readability and maintainability, while preserving the same dynamic batch size calculation logic based on file descriptor limits.
Reordered the imports in the `use nix::unistd` statement to place `SysconfVar` before `sysconf`, improving code readability and adhering to alphabetical import ordering conventions.
|
GNU testsuite comparison: |
Summary
Testing