Multithreaded component refresh #1232
Conversation
src/common.rs
Outdated
| @@ -3822,9 +3822,19 @@ impl Components { | |||
| /// components.refresh(); | |||
| /// ``` | |||
| pub fn refresh(&mut self) { | |||
| for component in self.list_mut() { | |||
| component.refresh(); | |||
| #[cfg(feature = "multithread")] | |||
There was a problem hiding this comment.
Instead of doing it like this, could you instead create an iter_mut() function into utils.rs (which would be the same as into_iter except it would call par_iter_mut/iter_mut) please? The idea is to prevent the duplication of .for_each(|component| component.refresh()). It might seem super small but it's easy to lose track of such things quite easily.
|
Apart from my suggestion, this is a great improvement, thanks! |
|
Thank you for the suggestion. I have made the change. While implementing |
| feature = "unknown-ci", | ||
| all(target_os = "macos", feature = "apple-sandbox") | ||
| ))] | ||
| pub(crate) fn into_iter_mut<T>(val: T) -> T::IntoIter |
There was a problem hiding this comment.
Shouldn't it be:
| pub(crate) fn into_iter_mut<T>(val: T) -> T::IntoIter | |
| pub(crate) fn into_iter_mut<T>(val: &mut T) -> T::IntoIter |
to enforce that val is mutably borrowed? It would also make it more coherent with the rayon function.
There was a problem hiding this comment.
It is slightly confusing but in the multithreaded case the &mut is saying the parallel iterator is mutable. In the non-multithreaded case the &mut is already part of T and specifying it will result in &mut &mut T.
There was a problem hiding this comment.
Can you add a code comment to explain that please? Like that future me will not wonder about it. :)
There was a problem hiding this comment.
Good idea. Future me will also appreciate the clarity.
| feature = "multithread" | ||
| ), | ||
| feature = "multithread", | ||
| not(feature = "unknown-ci"), |
… non-multithreaded implementations
|
|
||
| // In the multithreaded version of `into_iter_mut` above, the `&mut` on the argument is indicating | ||
| // the parallel iterator is an exclusive reference. In the non-multithreaded case, the `&mut` is | ||
| // already part of `T` and specifying it will result in the argument being `&mut &mut T`. |
* Multithreaded component refresh * Create utils::into_iter_mut to complement utils::into_iter. * Fix invalid parallelization `cfg`, preventing multi-threading on linux
This PR causes the bench_refresh_components benchmark to go from 1,517,282 ns/iter to 142,299 ns/iter when the "multithread" feature is enabled, which it is on by default. This massive improvement in speed is because reading from the
/sys/class/hwmon/files in Linux can be slow. In particular reading the CPU and chipset temperatures were taking about 60 ms each on my system.