Run a waited-for pipeline build at once when it is still queued #1129
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
No milestone
No assignees
1 participant
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1129
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/jobs-wait-runs-queued-job"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Some draws wait for their pipeline: those whose targets do not hold an earlier frame's picture. The wait is
jobs_wait(pe->ready). The pipeline's build job sits in the shared worker queue behind every job pushed before it. When a new area queues dozens of pipelines at 0.3-2 s each on AMD's compiler, the frame stops until the queue reaches that one draw's build. One 11.6 s stop I logged had 64 other pipelines finish during it, and the draw's own build was still queued. This is one source of the multi-second freezes when entering new areas (#995).Change
gpu/vk/vk_jobs.h,gpu/vk/vk_jobs.cpp:jobs_pushtakes an optionalflag: what the job sets when done.jobs_waitfinds its flag's job still in the queue, it takes the job out and runs it on the waiting thread, throughrun_guardedas the workers do.RX_VK_JOB_STEAL=0restores the old behaviour, waiting in turn.gpu/vk/vk_draw.cpp: onlygraphics_pipeline's build job is pushed with its flag (&e->ready). Translation jobs, compute jobs and the serial build lane are unchanged. The job runs where the draw that queued it is being recorded, with the same capturedNvnState. No locks are held at eitherwait_forcall indraw().tests/test_jobs_wait_runs_queued.py+.cpp: compilesvk_jobs.cppagainst stand-ins forvk_internal.handvk_draw.hand holds every worker in a job. It then checks that a flagged job queued behind them runs on the thread that waits, and that unflagged jobs still go to the workers. The source check fails onmain; the runtime part would hang there.How it was tested
0x8001a0. Local build on top of v2.0.8-rc.1 plus my fork's AMD changes (in-play pipelines built without the optimizer, 10 workers). This PR carries only the queue change, rebased onmain.python -I -B tests/test_jobs_wait_runs_queued.pypasses (g++ 10, Linux). No crashes or rendering changes in play.Refs #995
🤖 Generated with Claude Code
CI note: the one failing check,
tests/test_program_answer_domains.py(answers_inundeclared in its fixture), fails the same way onmainat8b2e1d3(run 42588468881), so it is not from this change. This PR's own test passes on both Ubuntu and Windows.Tested on NVIDIA (RTX 3070, Windows 11), merged onto v2.0.8-rc.5, with the scripted castle-load run.
Thanks for the no-regression check on NVIDIA! You're right that a warm pipeline cache hides the case. To exercise it, enter an area for the first time after a translator change, or rename
mkw_vk_pipelines.binand clear the driver's shader cache, so the draws there have to wait for builds still queued behind others. Theslow framelines withRX_VK_SLOW_LOG=150show the long stops.Looks right to me. Went through the steal path:
jobs_waitare both undermu, so either a worker already has it (and we fall through to the normal wait) or we own it.pending--/jobs_notify()run withlkreleased, so no self-deadlock onmu.build_graphicstakes noParLock, anddraw()holds nothing at eitherwait_for. Any lock the job needed that the replay thread held would already have deadlocked with the worker version.build_graphicsdoesn't read guest memory orcur_program, anddriver_cache_onlyis false on the replay thread, soub_thread_off()not being set there doesn't matter. Same as theRX_VK_SYNC_PIPELINESpath, which already builds on that thread.guarded_graphicsworks per thread on both Windows (SEH) and Linux (thread-localfault_jmp).CI: the
Python source checksfailures aretests/test_program_answer_domains.py(answers_inundeclared), which fails the same way on the base commit8b2e1d3. The new test passes on both ubuntu and windows.Small things, not blocking:
tests/run_all.pypicks the test up automatically, but it isn't inARGS, so on a box withoutg++it fails instead of being skipped. Adding"tests/test_jobs_wait_runs_queued.py": ["--cxx", "{cxx}"]would match the other compiled tests.@ -0,0 +60,4 @@jobs_push([&] { unflagged.store(true, std::memory_order_release); jobs_notify(); }); // (no flag: left in the queue)release();jobs_wait(unflagged); // (taken by a worker once released)expect(unflagged.load(), "a job pushed without its flag still runs on a worker");nit: this only checks that it ran, and an unflagged job can't be stolen anyway. If you want it to back up the message, record the thread id like the first case and check it's not this thread.