bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Swap out num_cpus for std::thread::available_parallelism

Open james7132 opened this issue 3 years ago • 6 comments
trafficstars

Objective

As of Rust 1.59, std::thread::available_parallelism has been stabilized. As of Rust 1.61, the API matches num_cpus::get by properly handling Linux's cgroups and other sandboxing mechanisms.

As bevy does not have an established MSRV, we can replace num_cpus in bevy_tasks and reduce our dependency tree by one dep.

Solution

Replace num_cpus with std::thread::available_parallelism. Wrap it to have a fallback in the case it errors out and have it operate in the same manner as num_cpus did.

This however removes physical_core_count from the API, though we are currently not using it in any way in first-party crates.


Changelog

Changed: bevy_tasks::logical_core_count -> bevy_tasks::available_parallelism. Removed: bevy_tasks::physical_core_count.

Migration Guide

bevy_tasks::logical_core_count and bevy_tasks::physical_core_count have been removed. logical_core_count has been replaced with bevy_tasks::available_parallelism, which works identically. If bevy_tasks::physical_core_count is required, the num_cpus crate can be used directly, as these two were just aliases for num_cpus APIs.

james7132 avatar Jun 09 '22 04:06 james7132

I don't know how important this is for Bevy, but std::thread::available_parallelism is erroneous for "environments where the available parallelism is controlled by cgroupv1" (https://github.com/rust-lang/rust/pull/97911), leading to significantly higher memory usage in such environments. num_cpus doesn't have this issue.

vacuus avatar Jun 09 '22 11:06 vacuus

Games generally don't run in containers that limit the amount of usable cpu. Also for bevy increasing the amount of threads doesn't increase the memory usage nearly as much as running more rustc processes at the same time.

bjorn3 avatar Jun 09 '22 12:06 bjorn3

games don't, but a game server using Bevy could

mockersf avatar Jun 09 '22 12:06 mockersf

At worst that would reduce efficiency, not cause crashes due to

Also for bevy increasing the amount of threads doesn't increase the memory usage nearly as much as running more rustc processes at the same time.

For maximum efficiency I think you will have to finetune the taskpool configuration anyway including manual determination of the amount of threads of each kind to use.

bjorn3 avatar Jun 09 '22 13:06 bjorn3

Maybe we should just wait to see how https://github.com/rust-lang/rust/pull/97925 plays out?

cart avatar Jun 10 '22 04:06 cart

Seems like https://github.com/rust-lang/rust/pull/97925 has been merged and was released as a part of 1.63, so I think the issues there have been addressed.

james7132 avatar Sep 10 '22 19:09 james7132

bors r+

alice-i-cecile avatar Sep 19 '22 15:09 alice-i-cecile