cli: size worker pool by cgroup-aware cpu count#976
Open
GrapeBaBa wants to merge 8 commits into
Open
Conversation
std.Thread.getCpuCount() only reads the CPU affinity mask, so under a
cgroup CPU quota (docker --cpus / k8s limits.cpu) it reports the host
core count rather than the quota, over-sizing the shared worker thread
pool and causing CFS throttling in CPU-limited containers.
Add pkgs/cli/src/cpu_count.zig (getNumCpus) returning min(cgroup quota,
affinity) by reading /proc/self/{cgroup,mountinfo} and cpu.max /
cpu.cfs_quota_us (cgroup v1 + v2), and use it when sizing the worker pool
in both the simulation (main.zig) and single-node (node.zig) paths.
/proc files report size 0, so they are streamed to EOF (readerStreaming +
allocRemaining); ceilDiv is overflow-safe; non-Linux falls back to the
affinity count.
CPU-count logic ported from the num_cpus crate; test vectors adapted from
seanmonstar/num_cpus (MIT).
g11tech
previously approved these changes
Jun 5, 2026
ch4r10t33r
reviewed
Jun 5, 2026
ch4r10t33r
left a comment
ch4r10t33r
left a comment
Contributor
There was a problem hiding this comment.
Three medium notes inline — none blocking. The rayon thread sizing also benefits from this change (rayon_threads derives from desired_workers), which is worth calling out in the PR body since it's a bigger win than "worker pool sizing" alone.
Sync cpu_count.zig with the refined error contract: distinguish a genuine absence of quota from a detection failure, instead of collapsing every failure into a silent affinity fallback that could mask a real quota and over-size the worker pool in a CPU-limited container. - null (-> affinity) only when no quota can be located: non-Linux, no cpu controller, no cgroup mount, unresolvable path, quota file absent, or unlimited (cpu.max "max" / cfs_quota_us "-1"). - error otherwise: unreadable /proc or quota file, unopenable cgroup dir, malformed/zero content, or an unavailable affinity count (getCpuCount no longer `catch 1`) -- now propagated to the caller. - match the v1 "-1" / v2 "max" unlimited sentinels before parseUnsigned so they are not taken for malformed input now that parse errors propagate. - document the 8 MB mountinfo read ceiling. getNumCpus is now fallible; both call sites (main.zig, node.zig) `try` it.
- Keep cgroup paths containing ':' intact (containerd's ...slice:cri-containerd:<id> cgroupfs naming): take the path with rest() instead of truncating at the next ':'. - Treat '..' cgroup paths (process outside its cgroupns root) as unresolvable -> affinity fallback instead of a hard error. - Take the minimum quota along the cgroup ancestor chain (leaf up to the mount point), matching Rust available_parallelism: the kernel enforces every level but each level's file reports only its own limit, so leaf-only reads missed LXC/Proxmox cpulimit and systemd CPUQuota= on a parent slice. - Polish per review: assert ceilDiv preconditions, dedup quota-file reads, drop unused pubs, 100-col wraps, hybrid-mount and buffer-limit test coverage (23 tests). Synced verbatim from ChainSafe/lodestar-z#386 (commits 7c84fab7..f504039f); verified there end-to-end in containers including the ancestor and colon-path scenarios.
Inline the logicalCpus wrapper (unnecessary abstraction over std.Thread.getCpuCount(); the n >= 1 assert moves into getNumCpus), drop the redundant 'never exceeds logical' test (the postcondition assert checks it on every call), and document the consumer in the module doc (adapted to this repo: the CLI worker pool).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
std.Thread.getCpuCount()only reads the CPU affinity mask, so under a cgroup CPU quota (docker --cpus/ k8slimits.cpu) it reports the host core count rather than the quota, over-sizing the shared worker thread pool and causing CFS throttling in CPU-limited containers. (node.zigalready has a comment acknowledging this exact issue near the--rayon-threadsguard.)Changes
pkgs/cli/src/cpu_count.zig—getNumCpus(gpa, io)=min(cgroup quota, affinity), via/proc/self/{cgroup,mountinfo}+cpu.max/cpu.cfs_quota_us(cgroup v1 + v2). Ported from thenum_cpuscrate.main.zig) and the single-node path (node.zig). Both pass the processIo(init.io);Node.initgains anioparam so it uses the caller's io rather than a global/throwaway one./procfiles report size 0 → streamed to EOF (readerStreaming+allocRemaining);ceilDivoverflow-safe; non-Linux falls back to affinity.Testing
cpu_count.zig; vectors adapted byte-for-byte fromseanmonstar/num_cpus(MIT).zig build installbuilds the cli; OrbStack Linux container:--cpus=2→2,--cpus=4→4, unlimited→host count.🤖 Developed with AI assistance (Claude Code).