Make System.coreCount aware of cgroup2 #2394
Conversation
Sources/NIOCore/Linux.swift
Outdated
| @@ -70,5 +72,15 @@ enum Linux { | |||
| else { return nil } | |||
| return (quota - 1 + period) / period // always round up if fractional CPU quota requested | |||
| } | |||
|
|
|||
| // cgroup2 | |||
| static func coreCount(maxPath: String) -> Int? { | |||
There was a problem hiding this comment.
Can I propose we rename these functions to be substantially more explicative of what they do? The little comments doesn't help much.
There was a problem hiding this comment.
I've tried a rename, we'll see how people like the names. Making the name unique also allows defaulting the arguments so I've done that too - let me know if you think that's a bad idea.
Sources/NIOCore/Utilities.swift
Outdated
| @@ -93,7 +93,9 @@ public enum System { | |||
| .map { $0.ProcessorMask.nonzeroBitCount } | |||
| .reduce(0, +) | |||
| #elseif os(Linux) || os(Android) | |||
| if let quota = Linux.coreCount(quota: Linux.cfsQuotaPath, period: Linux.cfsPeriodPath) { | |||
| if let quota2 = Linux.coreCount(maxPath: Linux.cfsCpuMaxPath) { | |||
There was a problem hiding this comment.
Should we adapt the documentation of coreCount to also include details of how c groups are used?
There was a problem hiding this comment.
I've added a brief bit of extra commentary. There is scope for broader review here. The idea of using this number to setup your threads loop group is mentioned - in most cases this is probably the wrong thing to do.
I also wonder if the logic could be improved - I think it's probably possible to be both in a cgroup and a cpuset. I do wonder if CPUSet combined with multiple threads is a good idea or not - suspect anyone doing this has probably put in quite a lot of thought.
Lukasa
added
🔨 semver/patch
PeterAdams-A
force-pushed
the
cgroups2
branch
2 times, most recently
from
a9751b8 to
67b6eba
Compare
|
@swift-nio-bot test this please |
Motivation: The count already appears to be aware of cgroup1 but doesn't have logic to support cgroup2. Modifications: Add logic to parse /sys/fs/cgroup/cpu.max For details see - https://stackoverflow.com/questions/65551215/get-docker-cpu-memory-limit-inside-container/65554131#65554131 Result: Core count will adapt if constrained by a cgroup on more modern kernels.
Co-authored-by: Max Desiatov <m_desiatov@apple.com>
Co-authored-by: Max Desiatov <m_desiatov@apple.com>
Co-authored-by: Gwynne Raskind <gwynne@darkrainfall.org>
Co-authored-by: Gwynne Raskind <gwynne@darkrainfall.org>
Motivation:
The count already appears to be aware of cgroup1
but doesn't have logic to support cgroup2.
Modifications:
Add logic to parse /sys/fs/cgroup/cpu.max
For details see - https://stackoverflow.com/questions/65551215/get-docker-cpu-memory-limit-inside-container/65554131#65554131
Result:
Core count will adapt if constrained by a cgroup on more modern kernels.