Skip to content

feat(agc): opt-in raised priority for the queue workers and the presenter - #1873

Merged
adriadam10 merged 6 commits into
boykopovar:mainfrom
fisherjoey:feat/queue-thread-priority
Oct 10, 2026
Merged

adriadam10 merged 6 commits into
boykopovar:mainfrom
fisherjoey:feat/queue-thread-priority

Conversation

@fisherjoey

@fisherjoey fisherjoey commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What

Adds an opt-in APS5_THREAD_PRIORITY that raises the AGC queue workers and the VideoOut presenter above the title's threads. Guest priorities don't reach host scheduling, so a title whose job system busy-waits on every hardware thread can keep the queue 0 worker off a core and make the driver the bottleneck (#1788).

  • =high: nice APS5_THREAD_NICE (default -10). On Linux through rtkit (MakeThreadHighPriorityWithPID) over a dlopen'd libdbus-1, the way SDL does it. On Windows, THREAD_PRIORITY_HIGHEST.
  • =rt: real-time priority APS5_THREAD_RT (default 10) through rtkit's MakeThreadRealtimeWithPID. On Windows, THREAD_PRIORITY_TIME_CRITICAL.
  • The mode picks one path before any call, with no fallback between them (fix(libkernel): pread, read and APR reads into write-tracked guest memory through a staging buffer #182). If rtkit or SetThreadPriority refuses, a queue worker sends the refusal through Driver::ReportFailure, so the next submit, wait or shutdown rethrows it, and the presenter passes it back to the VideoOut constructor.
  • Only off, high and rt are accepted. Any other value (hgih, 1, realtime) is rejected the same way as a refusal.
  • Each queue worker calls it, and so does the presenter, through a new C export from the driver. Each raised thread logs its priority once as [thread-priority]. Unset means no change.

RLIMIT_RTTIME handling for =rt (second commit, after @MarcZX100's review in #1788): rtkit's verify_process_rttime reads the hard limit from /proc/<pid>/limits and refuses when it is above RTTimeUSecMax (source). Lowering only the soft limit is not enough. With soft 100000 us and hard unlimited, MakeThreadRealtimeWithPID returns "Access denied"; with hard 200000 it succeeds. So the code now:

  • reads rtkit's MaxRealtimePriority and RTTimeUSecMax first. If rtkit is unreachable or the priority is above its maximum, the limit is not touched.
  • keeps a hard limit that is already at most min(APS5_THREAD_RTTIME_US, RTTimeUSecMax), and otherwise lowers it to that value. The soft limit goes to at most half of the hard one.
  • logs when it lowers the hard limit.

An unprivileged process cannot raise the hard limit again, so once lowered it stays lowered even if rtkit then refuses (burst limit, user or start-time checks). That is listed in TechnicalDebt and in BUILD.md. The log also reports the policy rtkit applied: SCHED_RR by default, not SCHED_FIFO as the issue says.

Why opt-in: it changes host scheduling for every title, rtkit is not on every system, and under =rt a queue worker that runs past RLIMIT_RTTIME without blocking gets the process a SIGXCPU. #626 (P-core pinning on hybrid CPUs) is complementary.

Closes #1788.

Tested

Latest revision (f01f95d), on the same base a29ef59: the NVIDIA GPU on this machine is down for now, so these runs are on RADV (Ryzen 5 7600X iGPU, Mesa 26.2.1).

  • ctest -R agc_ -j1: 325 of 326. The one failure, agc_driver, segfaults on main bf6e25a on RADV as well.
  • agc_driver_pm4_priority_refused (new) submits a write with APS5_THREAD_PRIORITY=high and no rtkit, and expects the refusal from WaitIdle and from shutdown, with the write not executed. Without the Submission.cpp change the process aborts in std::terminate. agc_thread_priority fails without the parser change on APS5_THREAD_PRIORITY=hgih must be rejected.

Earlier revisions:
Linux 7.2 (Nobara 44), GCC 16, RTX 5070 Ti, driver 595.91.07, Ryzen 5 7600X. Rebased on main a29ef59.

  • ctest -R agc_ -j1 on a29ef59 with the variable unset: 324 of 325. The one failure, agc_driver_dcc_keys_first_write, also fails on main. On the earlier base d70b899: 259 of 260 unset, and the same with APS5_THREAD_PRIORITY=high and with =rt.
  • agc_thread_priority_refused_rt and _high run the raise with the system bus pointed at a missing socket and pass only if it throws.
  • Earlier run, before the fallbacks were removed: ctest -R agc_driver -j1 202 of 202, unset and with =rt. In the rt run the queue workers log SCHED_RR 10 via rtkit, and the hard limit is lowered from unlimited to 200000 us once per process.
  • A standalone harness around RaiseWorkerThreadPriority (=rt): hard limit unlimited is lowered to 200000 (soft 100000); a preset soft 30000 / hard 150000 is left as is and rtkit grants; APS5_THREAD_RT=99 (above MaxRealtimePriority 20) and an unreachable system bus leave the limit untouched (that run predates the throw).
  • python3 tools/check_conventions.py --base origin/main: passes.
  • Demon's Souls (PPSA01341 v1.07), from Driver queue workers and the presenter lose their cores to a title's spinning job threads #1788 on an earlier base: with =high the PS Studios movie went from 254-255 to 345 presents per 10 s, and the attract movie from 429-447 to 520-536 presents per 10 s. With the priority on, the boot-time capture race from AGC: a compute dispatch is captured before the title has written its tables when a cross-queue wait passes on a recorded label #1787 became more likely; with that fix, 15 of 15 runs were clean.
  • Windows (by @popcoder565, on the earlier revision): Windows 10, RTX 3060 Laptop, driver 616.92. agc_thread_priority passes, and the threads log highest / time-critical in the two modes.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@fisherjoey

Copy link
Copy Markdown
Contributor Author

Added a unit test, agc_thread_priority (c46440b). The env parsing and the RLIMIT_RTTIME decision are now plain functions (ParseThreadPriorityMode, PlanRealtime) that PrepareRealtime calls, so the test runs without rtkit or real limits. It checks that an unreachable rtkit, a priority above MaxRealtimePriority and an RTTimeUSecMax of 0 all refuse without touching the limit, that an unlimited hard limit drops to min(APS5_THREAD_RTTIME_US, RTTimeUSecMax) with the soft limit at half, and that a limit rtkit already accepts is left alone.

main has no ThreadPriority.cpp, so there is no run on main. With PlanRealtime replaced by the first commit's rule (always set the hard limit to APS5_THREAD_RTTIME_US), it fails:

an unreachable rtkit must refuse real-time priority without touching RLIMIT_RTTIME

On the branch it passes. Also rebased on f145139.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

ThreadPriority.cpp falls back step by step when a request is refused: rtkit real-time, then an rtkit nice level, then setpriority. Fallbacks aren't allowed here (#182); please decide the path from the mode before the call (rt → rtkit real-time only, high → one nice-level path) and throw with the refusal when it fails. It also conflicts with main in VideoOutDriver.cpp since #1367; please rebase.

@popcoder565

Copy link
Copy Markdown
Contributor

Windows run of the SetThreadPriority branch, since the description lists Windows as not tested.

Setup: Windows 10 (19045), RTX 3060 Laptop (NVIDIA 616.92), GCC 15.2.0 (WinLibs r7), Release. Head abe8eb69 merged onto main d9484c7.

The merge conflicts in VideoOutDriver.cpp, because since #1367 the presenter thread gets a started promise. I resolved it by keeping both: the priority call first, then presentLoop(token, started).

  • agc_thread_priority passes.
  • APS5_THREAD_PRIORITY=high: every queue worker logs [thread-priority] queue 0x… worker: highest in agc_driver, agc_driver_pm4 and agc_driver_flip, and the VideoOut presenter logs presenter: highest. With rt they log time-critical. The tests pass in both modes.
  • video_out_flip_tests: controls and decode pass in both modes. The default lifetime mode aborts with VideoOut: port is closed with and without the variable; that's the main bug fix(videoout): report GPU driver shutdown failure from the present thread at Shutdown #1225 fixes.

No title was run.

🤖 Addressed by Claude Code

@fisherjoey
fisherjoey force-pushed the feat/queue-thread-priority branch from abe8eb6 to 2af260f Compare October 9, 2026 15:18
@fisherjoey

Copy link
Copy Markdown
Contributor Author

Done in 2af260f. The mode now picks one path up front, and a refusal throws:

  • rt asks rtkit for real-time priority and nothing else.
  • high asks rtkit for the nice level and nothing else. The setpriority step is gone.
  • On Windows, a failed SetThreadPriority throws too.

Two new tests, agc_thread_priority_refused_rt and _high, point the system bus at a missing socket and pass only if the raise throws. agc_thread_priority still checks the RLIMIT_RTTIME plan, including a priority above MaxRealtimePriority. Rebased onto main d70b899. The VideoOutDriver.cpp conflict keeps both sides: the presenter raises its priority first, then runs presentLoop(token, started). If the raise throws, the exception goes back through the started promise, so the VideoOut constructor fails instead of the thread terminating. In a queue worker there is nothing to hand it to, so a refusal there ends the process. That only happens when APS5_THREAD_PRIORITY is set.

Linux, RTX 5070 Ti, 595.91.07: ctest -R agc_ 259 of 260 unset, with =high and with =rt. The one failure, agc_driver_dcc_keys_first_write, also fails on main d70b899. Conventions pass.

@popcoder565 thanks for the Windows run. I added it to the description.

AI-assisted: yes

@popcoder565

Copy link
Copy Markdown
Contributor

Rechecked the new head 2af260f9 on Windows, since the Windows path now throws on a refusal. Setup: Windows 10 (19045), RTX 3060 Laptop (NVIDIA 616.92), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release, merged onto main d70b899.

APS5_THREAD_PRIORITY ctest -R "agc_|video_out|guest_video_out"
unset 260 of 260
high 260 of 260
rt 260 of 260
  • With high, 15 queue workers and 2 presenters logged [thread-priority] ...: highest. With rt they logged time-critical. So SetThreadPriority succeeded and nothing threw.
  • agc_thread_priority passes. The two refused tests aren't registered on Windows, as intended.

🤖 Addressed by Claude Code

@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 9, 2026
@fisherjoey
fisherjoey force-pushed the feat/queue-thread-priority branch from 2af260f to 12f2dec Compare October 9, 2026 22:22
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 9, 2026
@fisherjoey
fisherjoey force-pushed the feat/queue-thread-priority branch from 12f2dec to af32113 Compare October 10, 2026 02:26
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked on Linux (merged onto main): AGC/VideoOut ctest is green apart from display-dependent video_out tests, and agc_thread_priority, _refused_rt and _refused_high pass. The fallback removal asked for earlier looks right. Three things to fix:

  • Submission.cpp:317-322: RaiseWorkerThreadPriority runs in the thread lambda, outside the try in Driver::run. When rtkit refuses, the exception escapes the std::thread and calls std::terminate, while the presenter reports its refusal through the constructor. Catch it there and send it through Driver::ReportFailure like the other worker failures, so a refusal surfaces the same way for both threads.
  • ThreadPriority.cpp:288: any unrecognised APS5_THREAD_PRIORITY value (a typo like hgih) silently becomes high. Accept the documented values and throw on anything else, with a test row for it.
  • tests/ThreadPriority.cpp:50: remove the code comment.

Not verified here: the real rtkit path and the Demon's Souls numbers (no title or rtkit on our side); the Windows path rests on @popcoder565's runs.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
Read rtkit's MaxRealtimePriority and RTTimeUSecMax before touching the
limit, keep a hard limit that already satisfies rtkit, lower the soft
limit to at most half of the hard one, and log when the hard limit is
lowered. rtkit checks the hard limit, so it cannot be left alone when it
is above RTTimeUSecMax; the side effect is listed in TechnicalDebt.md.
Log the policy rtkit applied, SCHED_RR by default.
@fisherjoey
fisherjoey force-pushed the feat/queue-thread-priority branch from af32113 to 0fe61e4 Compare October 10, 2026 10:20
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@orishamir

Copy link
Copy Markdown
Contributor

0fe61e41 is af321131 rebased: the PR's own changes are identical in all ten files, so the three points from the review are still open on this head:

  • Submission.cpp:320 still calls RaiseWorkerThreadPriority in the thread lambda, outside the try in Driver::run. Reproduced here (Linux, D-Bus system bus present, no rtkit): with APS5_THREAD_PRIORITY=high or =rt, agc_driver and agc_driver_pm4 die with terminate called after throwing an instance of 'std::runtime_error', what(): APS5_THREAD_PRIORITY=high: queue 0x0 worker nice -10 refused: MakeThreadHighPriorityWithPID: The name org.freedesktop.RealtimeKit1 was not provided by any .service files (the =rt message names MaxRealtimePriority). Unset, both pass.
  • ThreadPriority.cpp:285-289 still maps any other value to High: with APS5_THREAD_PRIORITY=hgih the same abort happens and its message says =high.
  • The comment at tests/ThreadPriority.cpp:50 is still there.

Merged onto current main (57f96fd2) it builds, and agc_thread_priority, _refused_rt, _refused_high and the agc driver set pass with the variable unset (PR CI does not build the libraries).

AI-assisted: yes (Claude Code Fable 5.1 xhigh)

…and reject unknown APS5_THREAD_PRIORITY values
@fisherjoey

Copy link
Copy Markdown
Contributor Author

@adriadam10 Thanks. All three points are in f01f95d:

  • Submission.cpp: the worker lambda now catches what RaiseWorkerThreadPriority throws and sends it through ReportFailure, then returns without running the queue. New test agc_driver_pm4_priority_refused (Linux only, like the other refusal tests) submits a write with APS5_THREAD_PRIORITY=high and the system bus pointed at a missing socket. It expects the refusal from AgcDriverWaitIdle_nid_postfix and from shutdown, and checks that the write did not run. Without the Submission.cpp change it aborts: terminate called after throwing an instance of 'std::runtime_error'.
  • ThreadPriority.cpp: ParseThreadPriorityMode accepts unset, empty, off, high and rt, and throws std::invalid_argument on anything else. The rejected rows are hgih, HIGH, realtime, on, 1, 0 and high. realtime, on, 1 and 0 were accepted before, but BUILD.md never listed them, so they go too. BUILD.md now says that only off, high and rt are valid. A bad value fails the first raised thread the same way a refusal does. Without the parser change, agc_thread_priority fails with APS5_THREAD_PRIORITY=hgih must be rejected.
  • tests/ThreadPriority.cpp: comment removed.

Tested on RADV (Ryzen 5 7600X iGPU, Mesa 26.2.1), because the NVIDIA GPU on this machine is down for now. The branch is still on a29ef59. ctest -R agc_ -j1 passes 325 of 326. The one failure is agc_driver, which segfaults on RADV on main bf6e25a as well. check_conventions.py passes. I updated the PR description.

AI-assisted: yes

@orishamir

Copy link
Copy Markdown
Contributor

Checked f01f95d1 merged onto current main (57f96fd2; clean merge), Linux, D-Bus system bus present, no rtkit:

  • With APS5_THREAD_PRIORITY=high or =rt, agc_driver and agc_driver_pm4 now end in a reported failure instead of std::terminate: shutdown: APS5_THREAD_PRIORITY=high: queue 0x0 worker nice -10 refused: MakeThreadHighPriorityWithPID: The name org.freedesktop.RealtimeKit1 was not provided by any .service files (=rt names MaxRealtimePriority). With =hgih: APS5_THREAD_PRIORITY=hgih is not off, high or rt. Unset, the agc driver set passes.
  • agc_driver_pm4_priority_refused, agc_thread_priority, _refused_rt and _refused_high pass. Each new check pins its change: with Submission.cpp back at 0fe61e41, agc_driver_pm4_priority_refused aborts with terminate called after throwing an instance of 'std::runtime_error'; with ThreadPriority.cpp back at 0fe61e41, agc_thread_priority fails with APS5_THREAD_PRIORITY=hgih must be rejected.

AI-assisted: yes (Claude Code Fable 5.1 xhigh)

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked f01f95d merged onto main on Linux (no rtkit): agc_thread_priority, agc_thread_priority_refused_rt/_high, agc_driver_pm4 and agc_driver_pm4_priority_refused pass. Without the Submission.cpp try/catch, agc_driver_pm4_priority_refused aborts, so the test covers the fix. Build and conventions are green on the head; Windows run by @popcoder565 on the previous revision. Merging.

@adriadam10
adriadam10 merged commit 379d8b4 into boykopovar:main Oct 10, 2026
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Driver queue workers and the presenter lose their cores to a title's spinning job threads

5 participants