Skip to content

fix(io): make io worker cpu pinning opt-in via MORI_CORE_OFFSET#364

Merged
maning00 merged 1 commit into
ROCm:mainfrom
isytwu:fix/io-pinning-opt-in
Jun 5, 2026
Merged

fix(io): make io worker cpu pinning opt-in via MORI_CORE_OFFSET#364
maning00 merged 1 commit into
ROCm:mainfrom
isytwu:fix/io-pinning-opt-in

Conversation

@isytwu

@isytwu isytwu commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #358 (cpuset-aware worker pinning).

Changes

  • IO worker CPU pinning is now opt-in: workers are only pinned when
    MORI_CORE_OFFSET is set. When it is unset, workers inherit the process
    cpuset and the scheduler places them (matches the default no-explicit-pin
    behavior used elsewhere, e.g. SGLang's opt-in SGLANG_SET_CPU_AFFINITY).
  • Parse MORI_CORE_OFFSET via mori::env::GetInt instead of std::stoi,
    so an invalid value is ignored (treated as unset) instead of throwing and
    terminating the worker thread. Added a small signed-int helper
    (ParseInt / GetInt) alongside the existing ParsePositiveInt /
    GetPositiveIntOr in env_utils.hpp.

Why opt-in

Implicitly pinning every worker to allowed[0..n] can collide with other hot
threads sharing the same cpuset (e.g. the SGLang scheduler/compute threads).
Pinning is a tuning decision, so the default is now neutral; operators opt in
when they have a deliberate core layout.

Test

On 086/087 under taskset -c 8-15 with --num-worker-threads 4 --disable-chunking:

  • MORI_CORE_OFFSET=0: workers bound to cores 8-11 (within the allowed set).
  • unset: no pinning, workers run on scheduler-chosen allowed cores (e.g. 11-14).
  • MORI_CORE_OFFSET=abc (invalid): no crash, treated as unset, workers run normally.
    Plus a unit check of GetInt parsing (valid / 0 / negative / garbage / overflow).

Only pin workers when MORI_CORE_OFFSET is set; otherwise inherit the process cpuset and let the scheduler place them. Read the value via mori::env::GetInt to avoid std::stoi throwing on invalid input.
@isytwu isytwu requested a review from maning00 June 5, 2026 04:08
@isytwu isytwu self-assigned this Jun 5, 2026
@maning00 maning00 merged commit d87651c into ROCm:main Jun 5, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants