Ship the thread pool as its own library so a process has one pool - #21522
Open
shoumikhin wants to merge 85 commits into
Open
Ship the thread pool as its own library so a process has one pool#21522shoumikhin wants to merge 85 commits into
shoumikhin wants to merge 85 commits into
Conversation
Contributor
Author
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21522
Note: Links to docs will display an error until the docs builds have been completed. ❌ 163 New Failures, 2 Cancelled Jobs, 1 Unrelated Failure, 32 Unclassified FailuresAs of commit 4c0a67d with merge base efd6b55 ( NEW FAILURES - The following jobs have failed:
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:
CANCELLED JOBS - The following jobs were cancelled. Please retry:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
shoumikhin
added a commit
that referenced
this pull request
Jul 31, 2026
The thread pool that spreads CPU work across cores is meant to be a process-wide singleton, but it is held in a function-local static inside a static library, so every component that links it gets a private copy. The wheel ships two of them today: the pybindings extension and the training extension each define `get_threadpool`. Loading both into one interpreter gives the process two pools, and because each sizes itself to the machine's core count independently, they oversubscribe the CPU and contend with each other. Build the thread pool as a shared library for the wheel and let both extensions resolve it, so there is one pool per process again. `cpuinfo` and `pthreadpool` are forced static, so they are bundled inside this library rather than left for each consumer to supply, and the runtime is resolved from the shared runtime instead of embedding a second copy of the core. This is gated on the existing `EXECUTORCH_BUILD_SHARED` option, so only the Linux wheel changes; iOS, Android, and embedded builds keep linking the static library exactly as before. Test plan: The wheel smoke test now asserts that exactly one shipped library defines the thread pool accessor, alongside the existing single-backend-registry assertion, so a future change that reintroduces a second pool fails in CI rather than silently degrading performance. The new assertion was confirmed to fail against a wheel built before this change, where it correctly reports the two definers by name. Built the wheel from a clean checkout and verified against a fresh virtual environment with a normal dependency-resolving install: - `nm -DC` across every shipped shared object shows exactly one definition of `get_threadpool`, in the new library; the extensions import it rather than defining their own. - `readelf -d` shows the pybindings extension records the versioned thread pool library in `DT_NEEDED`, with a relocatable RUNPATH so it resolves the runtime as a sibling without `LD_LIBRARY_PATH`. - The single-backend-registry assertion still holds with the new library loaded. - `import executorch`, the registered backend list, and `.pte` execution through the Python bindings are unchanged, with outputs matching eager PyTorch. - With `EXECUTORCH_BUILD_SHARED` off, the thread pool is still a static library and no new shared object is produced, so every build that does not opt in is unaffected. ghstack-source-id: dfdb2bf ghstack-comment-id: 5146636071 Pull-Request: #21522
This was referenced Jul 31, 2026
shoumikhin
added
ciflow/periodic
ciflow/trunk
ciflow/binaries
ciflow/binaries/all
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
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.
The thread pool spreads CPU work across cores, and there is meant to
be exactly one per process. Today it lives in a static library, so every component that links it
gets its own private copy.
The wheel ships two copies right now:
Two pools each sizing themselves to the core count means the process can run roughly twice as
many worker threads as it has cores. They then compete for the same cores, which is slower than
one pool would be, and a setting applied to one pool has no effect on the other.
This change ships the pool as its own shared library that everything links:
How you use it
If you just want the default behaviour, nothing changes. If you want to control the pool from
C++:
Tested
Built the wheel on Linux x86_64 and aarch64 and installed it into a clean environment, then
confirmed by symbol inspection that exactly one shipped library defines the pool accessor,
where before two did. Also linked a C++ application against the component and ran it, and ran
the existing Python tests to confirm behaviour is unchanged for current users.