Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support cpu limit in docker #1827

Merged
merged 3 commits into from
Apr 30, 2022
Merged

Support cpu limit in docker #1827

merged 3 commits into from
Apr 30, 2022

Conversation

okhowang
Copy link
Contributor

close #1824

on windows docker with process isolation, use job api to get cpu limit.
on linux docker, use cgroups to get cpu limit.

@jhasse
Copy link
Collaborator

jhasse commented Aug 18, 2020

Any idea why 1bcc689 did work for me?

src/util.cc Outdated Show resolved Hide resolved
@okhowang okhowang force-pushed the cpu-limit branch 2 times, most recently from bf20848 to 8b2dd9d Compare August 19, 2020 02:05
@okhowang
Copy link
Contributor Author

okhowang commented Aug 19, 2020

Any idea why 1bcc689 did work for me?

I notice that there are many arguments about cpu-limit.
https://docs.docker.com/config/containers/resource_constraints/
--cpus=<value> is using cgroup to limit cpu usage.
--cpuset-cpus is using cpu-affinity api to limit process run on specific cpus

so 1bcc689 work only when --cpuset-cpus is actived.
my patch work when --cpus is actived.

we should use minimal args when they both active.
for example
docker run --cpus 3 --cpuset-cpus 1,2,3,4 mean we have 3 core available.
docker run --cpus 3 --cpuset-cpus 1,2 mean we have 2 core available.

windows docker don't support --cpuset-cpus currently.

docker: Error response from daemon: invalid option: Windows does not support CpusetCpus.

@okhowang okhowang changed the title Support cpu limit in docker [WIP] Support cpu limit in docker Aug 19, 2020
@okhowang okhowang changed the title [WIP] Support cpu limit in docker Support cpu limit in docker Aug 19, 2020
@jhasse
Copy link
Collaborator

jhasse commented Aug 23, 2020

Thanks!

The detection code is quite long (especially the cgroups version). If we include this, we should definitely not run it, if the user specified a job count with -j.

@okhowang
Copy link
Contributor Author

Thanks!

The detection code is quite long (especially the cgroups version). If we include this, we should definitely not run it, if the user specified a job count with -j.

I have modified ninja.cc to bypass GuessParallelism when -j set.

@jhasse jhasse added this to the 1.11.0 milestone Aug 24, 2020
@jhasse
Copy link
Collaborator

jhasse commented Aug 24, 2020

Thanks and nice approach using the destructor to set it at the end of the scope :)

Looking forward to some further comments by others.

@rufinio
Copy link

rufinio commented Feb 3, 2022

Successfully tested --cpus=<value>

with following configuration:

  • OS: Microsoft Windows Server 2022 Standard 10.0.20348
  • Docker: version 20.10.7, build 40ef3b6
  • Image: mcr.microsoft.com/dotnet/framework/sdk:4.8-windowsservercore-ltsc2022

@jhasse
Copy link
Collaborator

jhasse commented Mar 17, 2022

Anyone tested on Linux, especially start up time in different scenarios? (no idea how taxing those /proc opens can be)

@okhowang
Copy link
Contributor Author

Anyone tested on Linux, especially start up time in different scenarios? (no idea how taxing those /proc opens can be)

I tested with time ninja --version(--version will read cpu limit config) on my WSL2 with version 1.10.0 and this MR.

my version

$ time ./ninja --version
1.10.2.git

real    0m0.023s
user    0m0.000s
sys     0m0.004s

system ninja

time ninja --version
1.10.0

real    0m0.002s
user    0m0.000s
sys     0m0.001s

@rufinio
Copy link

rufinio commented Mar 30, 2022

@jhasse: Anything else to get this PR merged?

@jhasse jhasse merged commit 7905dee into ninja-build:master Apr 30, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support cpu limit in docker
3 participants