perf(parallelism): default the pool to physical cores, not SMT siblings - #2380
Open
ArthurZucker wants to merge 1 commit into
Open
ArthurZucker wants to merge 1 commit into
ArthurZucker wants to merge 1 commit into
Conversation
`num_threads()` defaulted to `available_parallelism()`, which counts SMT siblings. Filling them costs throughput on the encode path: on medium documents (~8 KiB) throughput peaks at the physical core count and then falls off, -20% going 88 -> 176 threads on aarch64 and -47% going 128 -> 512 on a two-socket Granite Rapids. Two workers on one physical core share a front-end and an L1, and this path is branchy and L1i-hungry, so it feels that more than most workloads do. Default to physical cores instead, read from sysfs on Linux and sysctl on macOS, and clamp to `available_parallelism()` so a cgroup quota or an affinity mask still wins -- this only ever lowers the count. Platforms that will not report a physical count keep the old behaviour. `set_num_threads()` still overrides. The sibling-list parser is compiled and tested on every platform, not just Linux: a misparse there would divide the pool size by the wrong number everywhere, and it is the only real logic here. (cherry picked from commit e1cd411)
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
1 participant
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.
Split out of #2361, which bundled it with the flat batch path. It is a change to pool
sizing and shares no code with that work, so it reviews and measures on its own.
available_parallelism()counts SMT siblings, and filling them costs throughput on thisencode path. Two workers on one physical core share a front-end and an L1, and this path
is branchy and L1i-hungry, so it feels that sharing more than most workloads do.
Capped by
available_parallelism(), so a cgroup quota or an affinity mask still wins —this only ever lowers the thread count, never raises it.
set_num_threadsstill overrides.Brings three tests, including
parses_thread_siblings_lists, which is compiled everywhererather than only on Linux: the kernel writes
thread_siblings_listas a list ("0,88"),a range (
"0-1"), or a mix ("0-1,4-5"), and a silent misparse would divide the poolsize by the wrong number on every box.
Numbers are in the commit message. I have not re-measured them — this is an extraction, not
a new claim, and I do not have the two machines they were taken on.