[rocjitsu] Add --cpu-thread-budget to rocjitsu and mirage by RattataKing · Pull Request #12245 · ROCm/rocm-systems

RattataKing · 2026-09-25T04:51:56Z

Motivation

Add --cpu-thread-budget to the Rocjitsu and Mirage CLIs to launch Rocjitsu with the desired thread budget without modifying the target JSON configuration file.

Technical Details

Missing --cpu-thread-budget or --cpu-thread-budget=0 uses the default thread allocation.
--cpu-thread-budget N creates effective_config.json in the invocation runtime directory and launches from that copy.
The original config is never modified; the effective config is written atomically and reclaimed by the existing runtime reaper.
The rewritten config is reparsed against the embedded schema and the applied budget is verified before launch.
The override works for table display, local launch, daemon mode, and VFIO paths.
--thread-budget-table applies the override in memory and does not create a file.
--attach rejects --cpu-thread-budget because the daemon already owns its configuration.
Configs with dbt_guest.simulator_config reject the flag; the budget must be set in the referenced host config.
mirage run supports the same flag. It is shorthand for -o cpu_thread_budget=N and takes precedence over that option.
Mirage leaves a supplied --config file unchanged and uses a session copy. Non-plain JSON configs are rejected.

Two refusals rather than silent wrong answers:

--attach joins a daemon that has already built its machine from its own config, so there is nothing left for a budget to apply to.
A dbt_guest.simulator_config is resolved relative to the file declaring it,
and that file, not this one, is what the host VM is built from — so a copy
elsewhere would both break the path and miss the config the budget has to
reach.

Issue Tracking

#12234

Test Plan

Config rewrite tests: replacement, insertion, comments, quoted/escaped keys, invalid input, and file preservation.
Native launch test: verify config_path, budget propagation, and unchanged source config.
CLI tests: both flag forms, invalid value, and --attach refusal.
Mirage tests plus build, CTest, Rust tests, formatting, Clippy, and pre-commit.

Test Result

Tests passed.

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

therock-pr-bot · 2026-09-25T04:52:46Z

✅ All Checks Passed — Ready for Review

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🔎 pre-commit	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—
🤖 therock-pr-bot	✅ Pass	—

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

therock-pr-bot · 2026-09-25T04:53:04Z

🎉 All checks passed! This PR is ready for review.

kuhar

We should preserve the loader's accepted syntax when applying --cpu-thread-budget, and add regression cases for adjacent comments and quoted field names. In emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/config/effective_config.cpp:101, scalar scanning consumes comment openers: "cpu_thread_budget":32// ceiling followed by a newline and comma, or 32/*,}*/, parses without the override but fails after replacement. At line 128, single-quoted keys are rejected and "cpu_thread_budge\u0074":32 is compared without decoding, so the rewrite inserts a duplicate budget field. Each form works with --thread-budget-table on the shipped gfx942 config but fails when --cpu-thread-budget 4 is added. Compatible tokenization and decoded key matching would let the launch-only override work with these existing configs.

…mirage Give one launch its own execution-thread ceiling without editing the config file it was given. The launcher copies the config to effective_config.json in that invocation's runtime directory, sets cpu_thread_budget there, and runs from the copy, so the override reaches the native table, VFIO, daemon and local routes alike and is reclaimed with the rest of that directory. The budget is a property of a simulation's config rather than of a process, so nothing process-wide carries it: the rewritten JSON is reparsed and the field read back before use, which turns any mistake into a refusal instead of a run under the wrong ceiling. The flag is refused with --attach, which joins a daemon that already built its machine, and with a config naming a dbt_guest.simulator_config, which is resolved relative to the file declaring it and is where the budget would have to land. mirage run gains the same --cpu-thread-budget. Unlike -o, it composes with --config, because upstream rocjitsu takes the two together; the supplied file is left alone and the run loads a session copy carrying the budget. Co-authored-by: Cursor <cursoragent@cursor.com>

kuhar

LGTM

systems-assistant · 2026-09-28T04:39:59Z

TheRock Submodule Bump Activity

Newest first

2026-09-28 — Included in TheRock via ROCm/TheRock#8533.

Fix #12245. Add e2e tests and fix the parser. Issue ID: #12234 --- <sub>Stack created with <a href="https://github.com/github/gh-stack">GitHub Stacks CLI</a> • <a href="https://gh.io/stacks-feedback">Give Feedback 💬</a></sub>

…-only tests PR #12475 added the first host-only coverage of `src/gin/gin_host.cc` (the GIN proxy NUMA affinity pin) and PR #12245 the GIN_PROXY_NTHREADS progress-thread behaviour, which together left the file at 59% of lines and 34% of branches. This builds out the rest, so every function in the file now has a test. The test TU's `ncclGin_t` fake grows a uniform `ScriptedFailure` per plugin entry point ("fail call N of this function"), recorders for the `ginConfig`s and `regMrSym`/`deregMrSym` arguments, and knobs for a NULL-returning `createContext` and a `queryLastError` that reports an error on a chosen call. That makes each failure arm reachable without a bespoke fake per test. The same mechanism now also covers `ncclTopoGetLocalGinDevs` and `bootstrapAllGather`, and the rail team's stride is a knob so the stride-validation branches are reachable. New suites, one per entry point: `GinHostTypeQueryMicrotest`, `GinHostSignalRequestMicrotest`, `GinHostConnectOnceMicrotest`, `GinHostBackendSelectMicrotest`, `GinHostDevCommSetupMicrotest`, `GinHostDevCommFreeMicrotest`, `GinHostFinalizeMicrotest`, `GinHostRegisterMicrotest` and `GinHostQueryLastErrorMicrotest`. They all start with `GinHost`, so the existing `GinHost*` CTest pattern picks them up without touching the categories yaml beyond its description. Coverage of `gin_host.cc` in `rccl-UnitTestsMicroGinHost` goes from 59.01% to 100% of lines, 66.67% to 100% of functions, and 33.79% to 74.43% of branches; no source-level branch is left uncovered, the remainder being `NCCLCHECK`/`WARN` macro expansions at call sites whose failure arm no test drives. Testing: `run_host_tests.sh all` passes (80 GinHost tests, shuffled); ten mutations of the hipified `gin_host.cc` -- dropping the NULL-window check, the multi-segment DMABUF requirement, the stride-multiple check, the empty-slot skip, the early return on a reported error, the context round-up, the connectivity check in `ncclGetGinType`, the VA-signal requirement check, the devComm reset on failure, and the fail-path `closeListen` -- each turn exactly the matching test red. Co-authored-by: Pi Agent

…13225) Brings `src/gin/gin_host.cc` to full host-only test coverage in `rccl-UnitTestsMicroGinHost`, and tidies the surrounding microtest infrastructure the work exposed. PR #12475 added the first host-only coverage of this file (the GIN proxy NUMA affinity pin) and PR #12245 the `GIN_PROXY_NTHREADS` progress-thread behaviour, which together left it at 59% of lines and 34% of branches. This builds out the rest: every function in the file now has a test, across nine new suites (one per entry point) covering type query, signal requests, connect-once, backend select, devComm setup/free, finalize, register and last-error query. Coverage of `gin_host.cc` goes from 59.01% to 100% of lines, 66.67% to 100% of functions, and 33.79% to 74.43% of branches; no source-level branch is left uncovered, the remainder being `NCCLCHECK`/`WARN` macro expansions at call sites whose failure arm no test drives. The test TU's `ncclGin_t` fake gains a uniform `ScriptedFailure` per plugin entry point ("fail call N of this function") plus argument recorders, so each failure arm is reachable without a bespoke fake per test. `FakeGin` hands out tagged handles rather than relying on a file-scope singleton, so tests that hold more than one context stay independent. Four reusable utilities that had accumulated inside the test TU are promoted to shared headers for the other host-only suites: `ScopeExit.h`, `ScriptedFailure.h`, `WaitUntil.h` and `JoinThreadsOnExit.h`. Externals the suite had been defining locally move onto the fakes file named for the production TU that defines each, per `MICROTEST_README.md`: `bootstrapAllGather` to `fakes/bootstrap_stubs.cc`, `ncclOsCpuCount`/`ncclOsSetAffinity` to `fakes/os_fakes.cc`, `ncclSetThreadName` to `fakes/nccl_fakes.cc`, and the `ncclTeam*` accessors to a new `fakes/nccl_device_core_fakes.cc`. Each moved seam picks up `ASSERT_HOOK_MATCHES_PROD` signature-drift checking from its new home. Testing: `run_host_tests.sh all` passes — all 13 host-only binaries green, 87 `GinHost` tests, shuffled. Ten mutations of the hipified `gin_host.cc` (dropping the NULL-window check, the multi-segment DMABUF requirement, the stride-multiple check, the empty-slot skip, the early return on a reported error, the context round-up, the connectivity check in `ncclGetGinType`, the VA-signal requirement check, the devComm reset on failure, and the fail-path `closeListen`) each turn exactly the matching test red. JIRA ID : AICOMRCCL-2642

github-actions Bot added documentation Improvements or additions to documentation project: rocjitsu labels Sep 25, 2026

RattataKing mentioned this pull request Sep 25, 2026

[rocjitsu][CI] Bound each corpus simulator's CPU thread budget #12236

Merged

RattataKing marked this pull request as ready for review September 25, 2026 05:01

RattataKing requested review from a team, amd-arosa and atgutier as code owners September 25, 2026 05:01

RattataKing added this pull request to stack #12246 September 25, 2026 05:01

systems-assistant Bot added the organization: ROCm label Sep 25, 2026

kuhar reviewed Sep 25, 2026

View reviewed changes

Comment thread emulation/rocjitsu/tests/CMakeLists.txt Resolved

RattataKing changed the title ~~[rocjitsu] Add a per-launch --cpu-thread-budget to rocjitsu and mirage~~ [rocjitsu] Add --cpu-thread-budget to rocjitsu and mirage Sep 25, 2026

RattataKing force-pushed the users/RattataKing/cpu-thread-budget-cli branch from 965315a to 243fb33 Compare September 25, 2026 17:24

kuhar approved these changes Sep 25, 2026

View reviewed changes

RattataKing merged commit b38e78f into develop Sep 25, 2026
84 of 86 checks passed

RattataKing deleted the users/RattataKing/cpu-thread-budget-cli branch September 25, 2026 22:59

amd-vivekag mentioned this pull request Sep 26, 2026

fix(mirage): warn when the session's ROCm cannot enumerate the profile's target #11367

Merged

1 task done

RattataKing mentioned this pull request Sep 28, 2026

[rocjitsu] Clean up --cpu-thread-budget #12387

Merged

tianxiaojiang4 mentioned this pull request Sep 28, 2026

fix(rocprofiler-sdk): report LDS_Block_Size from the dispatch, not the code-object symbol #12311

Closed

kuhar mentioned this pull request Sep 29, 2026

[rocjitsu] Fix floating-point and operand semantics #12467

Merged

mch mentioned this pull request Oct 10, 2026

test(rccl): cover every gin_host.cc entry point with host-only tests #13225

Merged

Navigation Menu

Repository navigation

[rocjitsu] Add --cpu-thread-budget to rocjitsu and mirage - #12245

[rocjitsu] Add --cpu-thread-budget to rocjitsu and mirage#12245
RattataKing merged 1 commit into
developfrom
users/RattataKing/cpu-thread-budget-cli

RattataKing commented Sep 25, 2026 •

edited

Loading

Uh oh!

therock-pr-bot Bot commented Sep 25, 2026 •

edited

Loading

Uh oh!

therock-pr-bot Bot commented Sep 25, 2026

Uh oh!

kuhar left a comment

Uh oh!

Uh oh!

kuhar left a comment

Uh oh!

Uh oh!

systems-assistant Bot commented Sep 28, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

RattataKing commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Issue Tracking

Test Plan

Test Result

Submission Checklist

Uh oh!

therock-pr-bot Bot commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All Checks Passed — Ready for Review

Uh oh!

therock-pr-bot Bot commented Sep 25, 2026

Uh oh!

kuhar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kuhar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

systems-assistant Bot commented Sep 28, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

RattataKing commented Sep 25, 2026 •

edited

Loading

therock-pr-bot Bot commented Sep 25, 2026 •

edited

Loading