Repository navigation
Limit the default thread pool to the allowed CPUs on Linux (not Android) - #24395
Conversation
get_num_performant_cores() reads two files that only ARM systems have: the SoC image version under /sys/devices/soc0, to spot a phone model with a known core count, and each CPU's MIDR register, to find small ARM cores. On x86 and other CPUs neither file exists. Each call logs the failed reads, including "Failed to open midr file", and returns 0, meaning the count is unknown. Skip both probes on those CPUs and return 0 directly. The result is the same and those log lines are gone. ARM runs both probes as before.
…re count is unknown On x86 Linux the default thread pool starts one thread per CPU in the machine, even when the process may only run on a few of them, for example under taskset or in a container with a CPU set. The extra threads compete for the same CPUs and slow CPU inference down. The pool size comes from get_num_performant_cores(), which returns 0 when it cannot tell fast cores from slow ones, as on x86. The 0 was passed on to pthreadpool, which then counted every online CPU. That count ignores the CPU affinity mask, and it also skips the 63-thread cap applied here. On Linux, when the count is unknown, use the number of CPUs in the affinity mask of the thread that creates the pool, which the pool's threads inherit, and then apply the cap. A machine with more than 63 CPUs and no limit now gets 63 threads instead of one per CPU. The threadpool.h notes describe the fallback. Nothing else changes. A known performance core count, which ARM usually has, is used as before. Android keeps pthreadpool's count, because it moves apps between CPU sets as they go to the background and back, while the pool is sized only once. Other systems keep pthreadpool's count as well. It is the right one there: on Emscripten, for example, pthreadpool can only make a pool of one thread. The new test sizes the pool in a fresh process limited to one CPU and expects one thread. It fails without this change.
shoumikhin
added
the
release notes: runtime
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24395
Note: Links to docs will display an error until the docs builds have been completed. ❌ You can merge normally! (6 Unrelated Failures), 2 Unclassified FailuresAs of commit 180db83 with merge base 05092c1 ( UNCLASSIFIED FAILURES - DrCI could not classify the following jobs because the workflow did not run on the merge base. The failures may be pre-existing on trunk or introduced by this PR:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
There was a problem hiding this comment.
🟡 Changes recommended
The regression test can falsely pass for incorrect pool sizes because Linux truncates exit statuses to eight bits, so the child must compare the count with 1 and return distinct success/failure codes.
1 open issue
What changed in this PR
This PR makes ExecuTorch’s default thread pool respect Linux CPU affinity when the performance-core count is unknown, avoiding excess threads in CPU-restricted environments.
Changes:
- Uses the allowed CPU count as the fallback on non-Android Linux, then applies the existing 63-thread cap.
- Skips ARM-only core detection on other architectures.
- Documents the fallback and adds a single-CPU affinity regression test.
| File | Description |
|---|---|
extension/threadpool/threadpool.h |
Documents the Linux affinity fallback. |
extension/threadpool/threadpool.cpp |
Adds affinity-based default pool sizing. |
extension/threadpool/test/threadpool_test.cpp |
Tests default sizing in a process restricted to one CPU. |
extension/threadpool/cpuinfo_utils.cpp |
Guards ARM-specific core detection by architecture. |
Review effort: Balanced
| // The pool is created once per process, so it is checked in a fresh one whose | ||
| // exit code is the pool's thread count. Failing to limit the CPUs aborts, so | ||
| // it cannot pass for a pool size. | ||
| TEST(ThreadPoolDeathTest, DefaultSizeHonorsCpuAffinity) { | ||
| executorch::runtime::runtime_init(); | ||
|
|
||
| if (::executorch::extension::cpuinfo::get_num_performant_cores() != 0) { | ||
| GTEST_SKIP() << "The performance core count sizes the pool here."; | ||
| } | ||
| ::testing::GTEST_FLAG(death_test_style) = "threadsafe"; | ||
| EXPECT_EXIT( | ||
| { | ||
| cpu_set_t cpus; | ||
| CPU_ZERO(&cpus); | ||
| CPU_SET(sched_getcpu(), &cpus); | ||
| if (sched_setaffinity(0, sizeof(cpus), &cpus) != 0) { | ||
| std::abort(); | ||
| } | ||
| std::exit(static_cast<int>( | ||
| ::executorch::extension::threadpool::get_threadpool() | ||
| ->get_thread_count())); | ||
| }, | ||
| ExitedWithCode(1), |
There was a problem hiding this comment.
Thanks, good point. This is fixed in a later commit. The child process no longer returns the thread count as its exit status. It prints the pool size and the performance core count, exits with success, and the test matches the exact text "pool 1 ...". A pool of 257 threads now fails the test.
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 19m 48s —— View job Needs Discussion: the affinity fallback only runs when the core count is 0, so a known count still ignores Detailed reviewPR Review: #24395SummaryOn non-Android Linux, the PR sizes the default pool by the affinity mask when The main problem is scope. A known count is never clamped by the mask, so the oversubscription fixed here for x86 CMake builds is still there on aarch64 Linux servers and in Buck builds. The minor findings:
The cpuinfo and pthreadpool submodules aren't checked out in this run, so statements about how they behave come from their known behavior, not from reading their source. Code Quality
Testing
API Design
Performance
RecommendationNeeds Discussion. There are no blockers, but the scope should be settled before merging: should known counts also be clamped by the mask, including under ALL_LOGICAL_CORES (and so every Buck build)? That change is a few lines, and it would also let the new test run on aarch64 and in Buck. The other findings are minor. |
A cpu_set_t holds 1024 CPUs. On a host whose kernel allows more, sched_getaffinity() rejects it with EINVAL, so the allowed-CPU count was 0 and the pool fell back to pthreadpool's count, ignoring both the mask and the 63-thread cap. Allocate the mask with CPU_ALLOC and double its size until the kernel accepts it.
Until now only an unknown core count was replaced by the number of CPUs in the affinity mask. A known count was used as is, so homogeneous ARM servers, and builds that size the pool by all logical cores (the Buck default), still started one thread per CPU in the machine under taskset or a CPU set. On Linux other than Android, take the smaller of the core count and the number of CPUs in the mask, and use the mask alone when the count is unknown. When the process may run on every online CPU, a known count stays as it was, so big.LITTLE systems keep their performance core count. The new test now runs in every Linux build, not only in x86 CMake ones. ParallelAdd and ParallelReduce expect more than one thread only when the process may use more than one CPU, so they pass under a one-CPU limit. The threadpool.h notes and the FAQ now say what the pool reads.
The soc0 image version and the MIDR registers are read from /sys, which only Linux has. On macOS and iOS on ARM, and on Windows on ARM, the probes still ran, failed, logged the failure, and could only return 0. Run them only on ARM Linux, which still includes Android. The core counts on the other systems do not change.
…s unknown When get_num_performant_cores() returns 0, the runner reset the pool to std::min(std::thread::hardware_concurrency(), 8). hardware_concurrency() counts every online CPU, so under taskset the runner started more threads than the process may use, undoing the default pool's limit. Cap the default pool's own size at 8 instead. It keeps the runner's limit and follows the CPU affinity mask.
The default pool now stays within the CPU affinity mask, but the example runners and other callers size the pool themselves with get_num_performant_cores(), which ignored the mask. On ARM Linux they still started one thread per core under taskset or a CPU set. Move the allowed-CPU count into cpuinfo_utils as get_num_allowed_cpus(), next to get_num_performant_cores(), and have the latter return the smaller of its count and the allowed CPUs on Linux other than Android. An unknown count (0) stays 0, so callers keep falling back to the default pool. When the process may run on every online CPU, the count is unchanged. The affinity test now also checks get_num_performant_cores() and limits itself to its current CPU with a mask sized for any CPU index. ParallelAdd and ParallelReduce read the allowed CPUs from get_num_allowed_cpus(), which handles hosts with more than 1024 CPUs.
The threadpool.h notes now say that the mask is read once, so the pool does not follow later changes to it, and that a child created by fork() keeps its parent's size. The FAQ now says that the default is one thread per performance core where core types are known and one per logical core otherwise, limited on Linux by the allowed CPUs and 63, and that CPU quotas are not detected.
…where it sizes threads get_num_allowed_cpus() returned 0 on Android to keep thread counts there unchanged, which mixed a sizing rule into a query whose name promises the CPU count. It now returns the count in the affinity mask on every Linux system, Android included, and 0 only where the mask is not available. Its comment says why the mask is a poor size for a pool on Android. get_num_performant_cores() and the default pool apply the limit everywhere except Android. Thread counts are the same as before. The get_num_performant_cores() comment now also lists the other cases that return 0, and the Gemma 4 runner comment says the default pool follows the mask on Linux other than Android only.
The death test now prints the pool size and the performance core count, and matches them exactly, so a failure shows which one was wrong, and a pool that failed to start cannot pass. It also uses gtest's default fork style: the threadsafe style runs the child on a small clone() stack that ASAN overflows now and then. The fork style works here because gtest runs death tests before the other tests, so the forked child creates the pool itself. ParallelAdd and ParallelReduce count the allowed CPUs with sched_getaffinity() themselves instead of asking the code under test.
Under --gtest_repeat, later runs of the death test fork after other tests created the pool, and the child keeps that pool's size, so the check fails although the code is right. Only the first run checks the pool now. The threadsafe death test style would start a fresh process instead, but it runs gtest's child on a small clone() stack that ASAN overflows now and then. ParallelAdd and ParallelReduce also keep checking for more than one thread when the affinity mask cannot be read, as on other systems.
…d that the mask is read once The comment in get_threadpool() now says the pool follows the affinity mask of the thread that creates it, which the pool's threads inherit. The get_num_performant_cores() comment says the count is capped at the allowed CPUs without checking which cores the mask allows, and the FAQ says the limit is read once, from the thread that first uses the thread pool.
…d cap as current in the FAQ The death test accepted a performance core count of 0 or 1 in the child, so on ARM a limit that wrongly dropped the count to 0 still passed. The parent now works out which one to expect: 1 when the count is known, 0 when it is not. The FAQ now says the default is "currently" at most 63 threads, because that cap comes from a sanitizer limit the code may drop later.
shoumikhin
changed the title
|
Thanks, this is updated:
|
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 21m 37s —— View job Approve: two minor items remain. The 63-thread cap on unrestricted x86 Linux hosts was only measured on a busy machine, and the Android exception is written twice under guards that don't mention Linux. Detailed reviewPR Review: #24395SummaryOn Linux other than Android, the PR limits the default thread pool and Code Quality
Performance
RecommendationApprove. Neither finding blocks merging. Both can be handled here or in a follow-up. |
Both places that apply the affinity limit now use the same guard as the test, Linux other than Android, instead of only excluding Android and relying on get_num_allowed_cpus() returning 0 elsewhere. The reason for leaving Android out is now given only in the get_num_allowed_cpus() comment, and <algorithm> moved to the Linux-only includes it serves.
populate_available_cpu_mids() parsed each midr_el1 file with std::stoi. A MIDR whose implementer byte is 0x80 or higher, such as AmpereOne (0xC0), is above INT_MAX, so std::stoi threw std::out_of_range and the process ended the first time the thread pool or a runner asked for the performance core count. std::stoul reads the full value.
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 22m 40s —— View job Approve: two minor items remain. On Linux the pool now stays small for the rest of the process if the CPU set grows after the pool is first used, and a test helper still can't read the mask on hosts with more than 1024 possible CPUs. Detailed reviewPR Review: #24395SummaryOn Linux other than Android, the PR limits the default thread pool and What this review could not check:
Testing
Performance
RecommendationApprove. Both findings are minor, and either can be handled here or in a follow-up. |
…4 CPUs may_use_more_than_one_cpu() read the mask into a fixed cpu_set_t, which the kernel rejects when it has more possible CPUs than the set holds. It then assumed more than one CPU, so ParallelAdd and ParallelReduce failed on such a host under a one-CPU mask. It now grows the mask the same way get_num_allowed_cpus() does. threadpool.h also says what to do when the mask grows after the pool is created: call _unsafe_reset_threadpool().
…grow The previous note told users to call _unsafe_reset_threadpool() after the mask grows. Loaded XNNPACK delegates keep the pthreadpool they were loaded with, so resetting the pool at that point leaves them with a freed pool. The note now says to set the needed count before loading a model, which matches the FAQ.
std::stoul still threw std::invalid_argument when a midr_el1 file opened but read back empty or not as hex. std::strtoul with an end check treats that the same as a file that does not open: the probe gives up and the performance core count stays unknown. ExecuTorch does not use exceptions.
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 23m 32s —— View job Approve: one low-confidence item remains. The new death test calls Detailed reviewPR Review: #24395SummaryOn Linux other than Android, the PR caps the default thread pool and What this review could not check:
Infrastructure
RecommendationApprove. The only finding is low confidence, and one internal coverage run would settle it. |
|
@pytorchbot cherry-pick --onto release/1.6 -c critical |
Cherry picking #24395The cherry pick PR is at #24449 and it is recommended to link a critical cherry pick PR with an issue. Details for Dev Infra teamRaised by workflow job |
What is wrong today
On Linux, the default ExecuTorch thread pool can start one thread per CPU in the machine, even when the process may only use a few, as under
tasksetor in a container with a CPU set. The extra threads compete for those CPUs and made a small XNNPACK model 1.4x slower.Why it happens
The pool size comes from
get_num_performant_cores()or, in Buck builds, the logical core count. Neither looks at the affinity mask. On x86 the first returns 0 (unknown), so with standard glibcpthreadpool_create(0)counts every online CPU and skips the 63-thread cap. On ARM servers with one core type, it counts every core.What this change does
On Linux other than Android, the default pool never starts more threads than the CPUs in the affinity mask, and uses that number when the core count is unknown.
get_num_performant_cores()applies the same limit, so runners that use it follow the mask too. In CMake builds, x86 Linux hosts with more than 63 allowed CPUs now get 63 threads by default, as ARM servers did, and the FAQ shows how to set the count in C++. Android and other systems keep their old sizing, because Android moves apps between CPU sets after the pool is sized.The new
get_num_allowed_cpus()counts the CPUs in the mask on Linux, even past 1024. The/sysprobes now run only on ARM Linux. With an unknown core count, the Gemma 4 runner uses the default pool size, nothardware_concurrency(), still capped at 8. Reading ARM core IDs no longer throws, as it did on AmpereOne.Tests
ParallelAddandParallelReducenow pass when only one CPU is allowed.taskset -c 0-3: 168 to 4 on x86, 14 to 4 on aarch64.