Repository navigation
cpu: preserve full thread affinity on Windows and Linux - #626
Closed
Yasei-no-otoko wants to merge 8 commits into
Closed
Yasei-no-otoko wants to merge 8 commits into
Yasei-no-otoko wants to merge 8 commits into
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Windows 11’s implicit all-group affinity cannot be restored from the saved primary group and mask.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Updates Windows CPU affinity handling to preserve processor-group identities during worker and host pinning.
Changes:
- Adds group-aware topology discovery and thread pinning.
- Introduces structured affinity state and restoration.
- Adds a Windows pin/restore integration test.
| File | Description |
|---|---|
src/kernels/cpu/pool.cpp |
Implements group-aware topology, pinning, and restoration. |
src/kernels/cpu/pool_affinity_win.hpp |
Adds the shared Windows affinity helper. |
src/kernels/cpu/pool_affinity_test.cpp |
Tests Windows group pinning and restoration. |
src/core/session.cpp |
Tracks whether host pinning succeeded. |
include/strata/kernels/cpu/pool.hpp |
Defines structured affinity state. |
include/strata/core/session.hpp |
Stores the new affinity state type. |
CMakeLists.txt |
Registers the Windows affinity test. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Niko1221
pushed a commit
that referenced
this pull request
Oct 4, 2026
Windows: pool workers pin with SetThreadGroupAffinity (the processor group was dropped: a 3990X's 63 workers collapsed to 32 group-local masks); the host thread selects a CPU Set and restores the previous selection. Linux: the complete, dynamically sized affinity mask is kept and restored (CPU IDs past 63). pool_affinity_test (Windows) and pool_affinity_linux_test. Squashed from the PR's 8 commits (net diff). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
|
Thanks! Taken into 0.1.39, the net diff squashed into one commit. pool_affinity_test passes here (one processor group). On our 6-core Ryzen, 10 interleaved decode pairs were within noise on Q2_0 and IQ3_XXS with the host on CPU Sets. The >64-CPU behavior is your 3990X and QEMU testing. |
sarashin65
added a commit
to sarashin65/XeStrata
that referenced
this pull request
Oct 4, 2026
…nse::load layer range) so v0.1.39 compiles The sycl port went in as 5047172 ("sycl: port 0.1.39"). Two engine-side changes were merged into the release branch after it (cb97e6c, 1189218) and nothing in sycl/ was updated for them, so the v0.1.39 tag does not compile: - Niko1221#626 (ec35cf0) made the thread affinity a value: pin_current_thread() returns strata::kernels::cpu::ThreadAffinity and restore_thread_affinity() takes it. sycl/include/strata/core/session.hpp still stored a `long long` (no conversion exists, and it did not include the header that names the type); sycl/src/core/session.cpp still assigned the returned struct to it and passed the `long long` back. - Niko1221#559 (ba707d8) added the two layer-range arguments to NativeDense::load. The sycl out-of-line definition was still the 4-argument one and no longer matched the declaration in include/strata/core/native_dense.hpp. Both are fixed by mirroring the CUDA sources: the same member type, the same `outside()` predicate, and the same place it is applied in the tensor loop. `pinned` now follows CUDA as well: it is pinned_core.valid, so a pin that failed is not "restored". Nothing changes when the pin succeeds. No caller passes layer_lo/layer_hi yet, so `outside()` is never true today: this restores the build and does not change what the engine computes. Measured here: Arch Linux, Intel oneAPI DPC++ 2026.0.0 + oneMKL 2026.0, SPIR-V (JIT), cmake -S sycl -B build-sycl -G Ninja -DCMAKE_C_COMPILER=icx -DCMAKE_CXX_COMPILER=icpx, then `cmake --build build-sycl --target strata -- -k 0` -> 0 errors, links. The two errors are a type and a declaration mismatch, so the compiler version does not matter. Not built here: the other targets (kernel parity tests, benches) and the AOT path. Not run on an Arc in this change.
samsonnguyen
pushed a commit
to samsonnguyen/Strata_B70
that referenced
this pull request
Oct 5, 2026
…o1221#559's per-stage dense layers in the port's copies The 0.1.39 re-migration (5047172) predates two upstream changes merged for the release, and the port's copies of the files they touch did not get them, so `sycl/` stopped compiling: - Niko1221#626 (ec35cf0): pin_current_thread returns a ThreadAffinity; sycl/src/core/session.cpp and session.hpp still held a long long core (3-way merged from ec35cf0^ -> ec35cf0). - Niko1221#559 (ba707d8, e50f266): NativeDense::load takes layer_lo / layer_hi, and the layer-range filter; the port's native_dense.cpp kept the old signature (3-way merged from 37fcef2 -> 0.1.39; dpct's function-try-block kept). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rb4SRxFuLpffQAAXBf4HxA
Niko1221
pushed a commit
that referenced
this pull request
Oct 6, 2026
…range) so v0.1.39 compiles The sycl port went in as 5047172 ("sycl: port 0.1.39"). Two engine-side changes were merged into the release branch after it (cb97e6c, 1189218) and nothing in sycl/ was updated for them, so the v0.1.39 tag does not compile: - #626 (ec35cf0) made the thread affinity a value: pin_current_thread() returns strata::kernels::cpu::ThreadAffinity and restore_thread_affinity() takes it. sycl/include/strata/core/session.hpp still stored a `long long` (no conversion exists, and it did not include the header that names the type); sycl/src/core/session.cpp still assigned the returned struct to it and passed the `long long` back. - #559 (ba707d8) added the two layer-range arguments to NativeDense::load. The sycl out-of-line definition was still the 4-argument one and no longer matched the declaration in include/strata/core/native_dense.hpp. Both are fixed by mirroring the CUDA sources: the same member type, the same `outside()` predicate, and the same place it is applied in the tensor loop. `pinned` now follows CUDA as well: it is pinned_core.valid, so a pin that failed is not "restored". Nothing changes when the pin succeeds. No caller passes layer_lo/layer_hi yet, so `outside()` is never true today: this restores the build and does not change what the engine computes. Measured here: Arch Linux, Intel oneAPI DPC++ 2026.0.0 + oneMKL 2026.0, SPIR-V (JIT), cmake -S sycl -B build-sycl -G Ninja -DCMAKE_C_COMPILER=icx -DCMAKE_CXX_COMPILER=icpx, then `cmake --build build-sycl --target strata -- -k 0` -> 0 errors, links. The two errors are a type and a declaration mismatch, so the compiler version does not matter. Not built here: the other targets (kernel parity tests, benches) and the AOT path. Not run on an Arc in this change.
Niko1221
pushed a commit
that referenced
this pull request
Oct 6, 2026
Windows: pool workers pin with SetThreadGroupAffinity (the processor group was dropped: a 3990X's 63 workers collapsed to 32 group-local masks); the host thread selects a CPU Set and restores the previous selection. Linux: the complete, dynamically sized affinity mask is kept and restored (CPU IDs past 63). pool_affinity_test (Windows) and pool_affinity_linux_test. Squashed from the PR's 8 commits (net diff).
Niko1221
pushed a commit
that referenced
this pull request
Oct 6, 2026
…range) so v0.1.39 compiles The sycl port went in as 5ffcce4 ("sycl: port 0.1.39"). Two engine-side changes were merged into the release branch after it (4f5e6c8, f5410ae) and nothing in sycl/ was updated for them, so the v0.1.39 tag does not compile: - #626 (5a8b292) made the thread affinity a value: pin_current_thread() returns strata::kernels::cpu::ThreadAffinity and restore_thread_affinity() takes it. sycl/include/strata/core/session.hpp still stored a `long long` (no conversion exists, and it did not include the header that names the type); sycl/src/core/session.cpp still assigned the returned struct to it and passed the `long long` back. - #559 (9461f5e) added the two layer-range arguments to NativeDense::load. The sycl out-of-line definition was still the 4-argument one and no longer matched the declaration in include/strata/core/native_dense.hpp. Both are fixed by mirroring the CUDA sources: the same member type, the same `outside()` predicate, and the same place it is applied in the tensor loop. `pinned` now follows CUDA as well: it is pinned_core.valid, so a pin that failed is not "restored". Nothing changes when the pin succeeds. No caller passes layer_lo/layer_hi yet, so `outside()` is never true today: this restores the build and does not change what the engine computes. Measured here: Arch Linux, Intel oneAPI DPC++ 2026.0.0 + oneMKL 2026.0, SPIR-V (JIT), cmake -S sycl -B build-sycl -G Ninja -DCMAKE_C_COMPILER=icx -DCMAKE_CXX_COMPILER=icpx, then `cmake --build build-sycl --target strata -- -k 0` -> 0 errors, links. The two errors are a type and a declaration mismatch, so the compiler version does not matter. Not built here: the other targets (kernel parity tests, benches) and the AOT path. Not run on an Arc in this change.
anon761
pushed a commit
to anon761/Strata-upstream
that referenced
this pull request
Oct 6, 2026
Windows: pool workers pin with SetThreadGroupAffinity (the processor group was dropped: a 3990X's 63 workers collapsed to 32 group-local masks); the host thread selects a CPU Set and restores the previous selection. Linux: the complete, dynamically sized affinity mask is kept and restored (CPU IDs past 63). pool_affinity_test (Windows) and pool_affinity_linux_test. Squashed from the PR's 8 commits (net diff).
anon761
pushed a commit
to anon761/Strata-upstream
that referenced
this pull request
Oct 6, 2026
…nse::load layer range) so v0.1.39 compiles The sycl port went in as 5ffcce4 ("sycl: port 0.1.39"). Two engine-side changes were merged into the release branch after it (4f5e6c8, f5410ae) and nothing in sycl/ was updated for them, so the v0.1.39 tag does not compile: - Niko1221#626 (5a8b292) made the thread affinity a value: pin_current_thread() returns strata::kernels::cpu::ThreadAffinity and restore_thread_affinity() takes it. sycl/include/strata/core/session.hpp still stored a `long long` (no conversion exists, and it did not include the header that names the type); sycl/src/core/session.cpp still assigned the returned struct to it and passed the `long long` back. - Niko1221#559 (9461f5e) added the two layer-range arguments to NativeDense::load. The sycl out-of-line definition was still the 4-argument one and no longer matched the declaration in include/strata/core/native_dense.hpp. Both are fixed by mirroring the CUDA sources: the same member type, the same `outside()` predicate, and the same place it is applied in the tensor loop. `pinned` now follows CUDA as well: it is pinned_core.valid, so a pin that failed is not "restored". Nothing changes when the pin succeeds. No caller passes layer_lo/layer_hi yet, so `outside()` is never true today: this restores the build and does not change what the engine computes. Measured here: Arch Linux, Intel oneAPI DPC++ 2026.0.0 + oneMKL 2026.0, SPIR-V (JIT), cmake -S sycl -B build-sycl -G Ninja -DCMAKE_C_COMPILER=icx -DCMAKE_CXX_COMPILER=icpx, then `cmake --build build-sycl --target strata -- -k 0` -> 0 errors, links. The two errors are a type and a declaration mismatch, so the compiler version does not matter. Not built here: the other targets (kernel parity tests, benches) and the AOT path. Not run on an Arc in this change.
anon761
pushed a commit
to anon761/Strata-upstream
that referenced
this pull request
Oct 6, 2026
Windows: pool workers pin with SetThreadGroupAffinity (the processor group was dropped: a 3990X's 63 workers collapsed to 32 group-local masks); the host thread selects a CPU Set and restores the previous selection. Linux: the complete, dynamically sized affinity mask is kept and restored (CPU IDs past 63). pool_affinity_test (Windows) and pool_affinity_linux_test. Squashed from the PR's 8 commits (net diff). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Syakyr
pushed a commit
to Syakyr/Strata
that referenced
this pull request
Oct 7, 2026
…o1221#559's per-stage dense layers in the port's copies The 0.1.39 re-migration (5047172) predates two upstream changes merged for the release, and the port's copies of the files they touch did not get them, so `sycl/` stopped compiling: - Niko1221#626 (ec35cf0): pin_current_thread returns a ThreadAffinity; sycl/src/core/session.cpp and session.hpp still held a long long core (3-way merged from ec35cf0^ -> ec35cf0). - Niko1221#559 (ba707d8, e50f266): NativeDense::load takes layer_lo / layer_hi, and the layer-range filter; the port's native_dense.cpp kept the old signature (3-way merged from 37fcef2 -> 0.1.39; dpct's function-try-block kept). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rb4SRxFuLpffQAAXBf4HxA
sskver
pushed a commit
to sskver/Strata
that referenced
this pull request
Oct 7, 2026
…nse::load layer range) so v0.1.39 compiles The sycl port went in as 5ffcce4 ("sycl: port 0.1.39"). Two engine-side changes were merged into the release branch after it (4f5e6c8, f5410ae) and nothing in sycl/ was updated for them, so the v0.1.39 tag does not compile: - Niko1221#626 (5a8b292) made the thread affinity a value: pin_current_thread() returns strata::kernels::cpu::ThreadAffinity and restore_thread_affinity() takes it. sycl/include/strata/core/session.hpp still stored a `long long` (no conversion exists, and it did not include the header that names the type); sycl/src/core/session.cpp still assigned the returned struct to it and passed the `long long` back. - Niko1221#559 (9461f5e) added the two layer-range arguments to NativeDense::load. The sycl out-of-line definition was still the 4-argument one and no longer matched the declaration in include/strata/core/native_dense.hpp. Both are fixed by mirroring the CUDA sources: the same member type, the same `outside()` predicate, and the same place it is applied in the tensor loop. `pinned` now follows CUDA as well: it is pinned_core.valid, so a pin that failed is not "restored". Nothing changes when the pin succeeds. No caller passes layer_lo/layer_hi yet, so `outside()` is never true today: this restores the build and does not change what the engine computes. Measured here: Arch Linux, Intel oneAPI DPC++ 2026.0.0 + oneMKL 2026.0, SPIR-V (JIT), cmake -S sycl -B build-sycl -G Ninja -DCMAKE_C_COMPILER=icx -DCMAKE_CXX_COMPILER=icpx, then `cmake --build build-sycl --target strata -- -k 0` -> 0 errors, links. The two errors are a type and a declaration mismatch, so the compiler version does not matter. Not built here: the other targets (kernel parity tests, benches) and the AOT path. Not run on an Arc in this change.
MistVVK
added a commit
to MistVVK/XeStrata
that referenced
this pull request
Oct 14, 2026
物理コアごとに1つの論理プロセッサを GetLogicalProcessorInformationEx から取り、 番号はグループ × 64 + グループ内の番号にする。プールのワーカーは SetThreadGroupAffinity で固定する。 ホストのスレッドは upstream の Niko1221#626 と同じく CPU Set で選び、元に戻せるようにする。 グループのアフィニティでは、Windows 11 の暗黙の全グループの割り当てを戻せない ため。前の状態はマスク(long long)に収まらないので、pin_current_thread は ThreadAffinity を返す形に変える。Linux では中身は 64 ビットのマスクのままで、 動作は変わらない。
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.

Strata's CPU pinning lost part of the processor identity on Windows and part of the caller's saved CPU mask on Linux. Windows workers discarded the processor-group number; Linux host restoration retained only CPU IDs 0-63. This change preserves the full placement on both platforms without changing automatic worker sizing or explicit
--pool-workersvalues.On Windows, use
SetThreadGroupAffinityfor pool-owned workers, enumerate every core group mask and preserve group identity in fallback enumeration. On the tested 3990X, the previous 63-worker list collapsed to 32 distinct group-local masks becauseSetThreadAffinityMask(..., 1 << (core & 63))discarded the group.The caller-owned Windows host thread uses reversible CPU Set selection. Save its full explicit selection and restore it, or clear it when originally empty. This preserves process-default inheritance and Windows 11's implicit all-group eligibility without changing existing hard affinity. CPU Sets are soft placement constraints. See Microsoft's CPU Set API.
On Linux, retain the complete native affinity mask in
ThreadAffinity. UseCPU_ALLOCand the sized CPU-set macros, retry a query with a larger buffer onEINVAL, and share the dynamic-mask handling with topology enumeration and worker pinning. Restore the complete saved mask, including high-only and sparse selections, and report failed affinity operations using pthread's returned error number. See Linux CPU-set APIs. No Windows-style group IDs are needed on Linux.Validation:
pool_affinity_testafter the Linux addition; CTest passed. It covers worker pins, reversible host selections, process defaults, and continued execution in both processor groups. The earlier Windows change also completed a full Release HIP engine build; details are in bench: report Coder IQ1_M on Threadripper 3990X and RX 6900 XT #628.strata_kernels_cpuandpool_affinity_testwith GPU/native-ggml support disabled.-Wall -Wextra -Werrorsyntax checks passed for the changed Linux implementation and test. CTest passed with 64 available CPUs, and a separate run withtaskset -c 0,2,15passed. The high-CPU path correctly reports a skip on these WSL runs.6.6.36.3-microsoft-standard-WSL2exposed CPUs 0-127. The same statically linked regression test passed with--require-high-cpu, both with all 128 CPUs and with affinity restricted to CPUs 64-127. Tests include full-mask and sparse/nested restoration, a high-only saved mask, topology respecting the allowed CPU set, worker execution on CPU 127, invalid targets, and cleanup.Linux reproduction on a normal Linux checkout:
The Linux results establish affinity correctness in WSL / an emulated guest, not native Linux inference performance. CPU IDs above 1023 were not exercised. The AVX-512 pool self-test and
pool_stressskip their workloads on this 3990X; a zero exit code from those skips is not a stress-workload pass.Benchmark data remains in the separate PR #628. Its speed figures belong to the recorded earlier Windows revisions and predate both the Windows host CPU Set review follow-up and this Linux addition; inference throughput was not remeasured for this revision. The machine's one NUMA node spans two processor groups, so this is an application affinity correction, not a Windows Containers or OS NUMA fix.