Implement CGroups Limit Enforcement for CPUs Allocated #2325
Conversation
|
Issue: #2261 |
| if(allocated_cpus_share_int > -1) | ||
| { | ||
| unsigned int allocated_cpus = round (allocated_cpus_share_int / 1000); | ||
| return allocated_cpus; |
There was a problem hiding this comment.
The method should never return zero.
| @@ -15,6 +17,22 @@ unsigned getNumberOfPhysicalCPUCores() | |||
| { | |||
| #if defined(__x86_64__) | |||
|
|
|||
| std::ifstream cgroup_read_in("/sys/fs/cgroup/cpu/cpu.cfs_quota_us"); | |||
There was a problem hiding this comment.
Better to wrap into something like #if defined(__linux__) as cgroups exist only on Linux.
| // If a valid value is present | ||
| if(allocated_cpus_share_int > -1) | ||
| { | ||
| unsigned int allocated_cpus = round (allocated_cpus_share_int / 1000); |
There was a problem hiding this comment.
Bug: round does nothing as you use integer division.
| @@ -1,5 +1,7 @@ | |||
| #include <Common/getNumberOfPhysicalCPUCores.h> | |||
| #include <thread> | |||
| #include <fstream> | |||
| #include <math.h> | |||
There was a problem hiding this comment.
No need for round.
You may round up, like this: (allocated_cpus_share_int + 999) / 1000
| cgroup_read_in.close(); | ||
|
|
||
| // If a valid value is present | ||
| if(allocated_cpus_share_int > -1) |
There was a problem hiding this comment.
> -1 looks less clear than >= 0
There was a problem hiding this comment.
The default value if the limit is not set is -1, hence used the value. However, using > 0 makes sense since that will allow us to avoid corner cases where the CPU limit is read as 0
|
@alexey-milovidov Please review the second version |
|
If you want to use throttling, be aware that it's broken: |
|
OpenJDK is using both shares and determine CPU limit: |
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en