nproc: simplify core count#12753
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
max-amb
left a comment
There was a problem hiding this comment.
Is this at risk of an underflow? num_cpus_all() returns a usize, so if we subtract a usize from a usize we risk underflow. It may be completely mitigated by the .max() call but the compiler doesn't like it:
fn main() {
let x: usize = 5;
dbg!((x-6).max(1));
}
fails to compile.
Could use a saturating_sub?
|
|
Ah yeah, could cast to a signed type, or I think the original is the only solution. wdyt? Maybe something more obvious is let initial_cores = std::cmp::min(limit, cores);
let cores = if initial_cores <= ignore {
1
} else {
initial_cores - ignore
} |
This comment was marked as low quality.
This comment was marked as low quality.
|
But will it not panic? |
|
|
|
When compiled in release it will wrap and we get the wrapped number as it is bigger? I'm not too familiar with this though. https://doc.rust-lang.org/book/ch03-02-data-types.html#integer-overflow It panics in debug mode |
9f93a38 to
4331c2a
Compare
|
It seems working on 32bit too. |
|
GNU testsuite comparison: |
Add two `complexity` lints that detect a single-sided clamp written as an
`if`/`else` (or a guarding `if`) and suggest `Ord::max` / `Ord::min`:
let _ = if a < b { b } else { a }; // -> a.max(b)
if cores < b { cores = b; } // -> cores = cores.max(b);
This is the sound, generalizable form of the manual clamp simplified by
hand in uutils/coreutils#12753 (`nproc`). Unlike `manual_clamp`, which
only fires when both a lower and an upper bound are applied, these catch
the common single-bound floor/ceiling case.
Restricted to `Ord` types so float `NaN` semantics are not changed, and
emitted as `MaybeIncorrect` since the branching form re-evaluates the
selected operand.
No description provided.