num_cpus
num_cpus copied to clipboard
seanmonstar
Use a Result for get_physical() ?
The current signature is pub fn get_physical() -> usize. The documentation says that it will return the same as get() if it isn't able to get the number of physical cores on that architecture. I'm wondering if it would be a good idea to have get_physical() return a Result<usize, usize> to inform the user of whether that fallback was utilized or not.
I don't know if that is useful for not, just pointing out that that would be a breaking change to the API, so would require going to v2 to make that change.
I don't know if it would be useful or not either; the thought just crossed my mind and I figured I would share
Perhaps there could be a try_get_physical function that returns an Option<usize>, and the get_physical function stays the same, while calling try_get_physical under the hood. That way the current API stays the same.
I don't know if it's useful either - if this library is unable to find the number of physical cores, I doubt the user would have a better fallback.
I'd argue that there might be a usecase (although I'm not able to give one from the top of my head) in which user would like to be certain about number of physical cores, and if the query fails, they might make an informed decision instead of automatic fallback.
There's been quite a bit of discussion on this one (and the best return types for these functions in general) at the Libz Blitz evaluation thread: https://internals.rust-lang.org/t/out-of-band-crate-evaluation-for-2017-10-13-num-cpus/5986/2
Have linked to the evaluation tracking issue.
Does anyone have any more thoughts on this? If we can't find a compelling use-case then I'd be inclined to leave the status-quo unchanged, and avoid the possible confusion of users not knowing whether they should care.
What do you think?
I'm struggling to come up with a compelling use case for this, to be totally honest. It definitely feels like the right thing to do is return a Result or Option here, but if there's nothing the library user can usefully do with the extra information, we might as well not change the API.
@seanmonstar any thoughts to add?
The user probably can find a way to use this extra info -- for instance, only displaying only logical CPUs. It might also matter for very low level stuff, or hardware discovery. IMO if you can embed more information unobtrusively, then you should. The only downside is having to bump to 2.0... but that's what SemVer's for, right? No use in having major versions if you don't use 'em 😛
Just throwing in my two cents here: I think get() should also return a Result. Right now it is hardcoded to return 1 on certain platforms (SGX, WASM...) which is super misleading.
