[bug] get_cpus is not supporting cgroup2 #11635 #11667
Conversation
|
|
|
Should I document the changes to the Conan docs? |
Better wait until it is merged, and then yes, a PR to the docs, if necessary, would be nice, just a couple of lines if it makes sense |
|
Do I need to do something for this PR to be merged? |
No, it is good, it just takes some time to review, too many ongoing things. It doesn't have tests, but honestly all tests are going to be pure mocks, so probably it is good. Maybe @maikelvdh want to comment something more, if not, this will be merged and released in 1.51 (still a few days, Conan 2.0-beta.2 is in the next release) |
conan/tools/build/cpu.py
Outdated
| if os.path.exists("/sys/fs/cgroup/cgroup.controllers"): | ||
| cpu_max = load("/sys/fs/cgroup/cpu.max").split() | ||
| if cpu_max[0] == "max": | ||
| return 1 |
There was a problem hiding this comment.
In case of max it means basically no limits are established and therefore we should be able to rely on the multiprocessing.cpu_count(). In the current logic you would otherwise effectively in case of docker when cgroup V2 present and without any limiting of CPU cores applied just run with 1 jobs always.
There was a problem hiding this comment.
But is it not an issue that multiprocessing.cpu_count() is used? because it will not match the docker assigned cores, and it can also overflow the container capacity?
There was a problem hiding this comment.
That is exactly what you want in case of docker if you are not using the runtime options to limit the CPU (https://docs.docker.com/config/containers/resource_constraints/#cpu). In case you are using this options to limit you will get not max as value.
There was a problem hiding this comment.
Great, thanks very much for the clarification. @MariaMozgunova could you please apply this in your PR? Thanks!
|
Thanks a lot @MariaMozgunova for addressing this problem 🚀 ! |
When $MAX == "max", there is no limit on the cpu usage set Use multiprocessing.cpu_count() function in that case
MariaMozgunova
force-pushed
the
feature/cgroup2_support
branch
from
61a5d80
to
9e43328
Compare
|
Merged, thanks for your contribution! It will be part of 1.51 If you now want to do a PR to the docs, something very simple about the behavior for cgroups2, a couple of lines would be enough, that would be great. |
|
@memsharded Can you please hint to me what file precisely should be modified? |
|
@MariaMozgunova The documentation already describes cgroup usage, so with your hotfix, the follow part should updated: The website will look like this: https://docs.conan.io/en/latest/reference/tools.html#tools-cpu-count Thank you for your contribution! |
Changelog: Fix: Get the cpu count for cgroup2 from
/sys/fs/cgroup/cpu.max.Docs: conan-io/docs#2658
Fixes #11635
Gets the cpu count for cgroup2 from
/sys/fs/cgroup/cpu.maxfile. From docs,/sys/fs/cgroup/cpu.maxfile has the following format:$MAX $PERIOD.In case
$MAXis "max", return 1.If there is no
$PERIOD, use only$MAXas the return value.When both values are integers, use them as before.
developbranch, documenting this one.Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.