Run a waited-for pipeline build at once when it is still queued #1129

Merged
duck-paradise merged 1 commit from fix/jobs-wait-runs-queued-job into main 2026-11-09 18:56:46 +00:00
duck-paradise commented 2026-11-09 13:33:43 +00:00 (Migrated from github.com)

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_push takes an optional flag: what the job sets when done.
    • When jobs_wait finds its flag's job still in the queue, it takes the job out and runs it on the waiting thread, through run_guarded as the workers do.
    • A job a worker has already started is waited for as before.
    • RX_VK_JOB_STEAL=0 restores the old behaviour, waiting in turn.
  • gpu/vk/vk_draw.cpp: only graphics_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 captured NvnState. No locks are held at either wait_for call in draw().
  • tests/test_jobs_wait_runs_queued.py + .cpp: compiles vk_jobs.cpp against stand-ins for vk_internal.h and vk_draw.h and 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 on main; the runtime part would hang there.

How it was tested

  • Hardware: RX 9070 XT, Windows 11, AMD driver 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 on main.
  • Long stops (slow-frame log, ≥5 s each):
Session Length Stops ≥5 s
Before about 19 min 8 (6-12.5 s)
After about 37 min 2
After about 11 min 0
  • What still stops: builds that were already running on a worker and themselves took long. That is a separate issue.
  • Caveat: the "before" session was also the first after a shader rebuild, so it met more new pipelines than the later ones. Take the numbers as indicative.
  • Tests: python -I -B tests/test_jobs_wait_runs_queued.py passes (g++ 10, Linux). No crashes or rendering changes in play.

Refs #995

🤖 Generated with Claude Code

## 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_push` takes an optional `flag`: what the job sets when done. - When `jobs_wait` finds its flag's job still in the queue, it takes the job out and runs it on the waiting thread, through `run_guarded` as the workers do. - A job a worker has already started is waited for as before. - `RX_VK_JOB_STEAL=0` restores the old behaviour, waiting in turn. - **`gpu/vk/vk_draw.cpp`:** only `graphics_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 captured `NvnState`. No locks are held at either `wait_for` call in `draw()`. - **`tests/test_jobs_wait_runs_queued.py` + `.cpp`:** compiles `vk_jobs.cpp` against stand-ins for `vk_internal.h` and `vk_draw.h` and 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 on `main`; the runtime part would hang there. ## How it was tested - **Hardware:** RX 9070 XT, Windows 11, AMD driver `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 on `main`. - **Long stops (slow-frame log, ≥5 s each):** | Session | Length | Stops ≥5 s | |---|---|---| | Before | about 19 min | 8 (6-12.5 s) | | After | about 37 min | 2 | | After | about 11 min | 0 | - **What still stops:** builds that were already running on a worker and themselves took long. That is a separate issue. - **Caveat:** the "before" session was also the first after a shader rebuild, so it met more new pipelines than the later ones. Take the numbers as indicative. - **Tests:** `python -I -B tests/test_jobs_wait_runs_queued.py` passes (g++ 10, Linux). No crashes or rendering changes in play. Refs #995 🤖 Generated with [Claude Code](https://claude.com/claude-code)
duck-paradise commented 2026-11-09 13:37:17 +00:00 (Migrated from github.com)

CI note: the one failing check, tests/test_program_answer_domains.py (answers_in undeclared in its fixture), fails the same way on main at 8b2e1d3 (run 42588468881), so it is not from this change. This PR's own test passes on both Ubuntu and Windows.

CI note: the one failing check, `tests/test_program_answer_domains.py` (`answers_in` undeclared in its fixture), fails the same way on `main` at 8b2e1d3 (run 42588468881), so it is not from this change. This PR's own test passes on both Ubuntu and Windows.
Nitnek commented 2026-11-09 15:52:17 +00:00 (Migrated from github.com)

Tested on NVIDIA (RTX 3070, Windows 11), merged onto v2.0.8-rc.5, with the scripted castle-load run.

  • The picture matches rc.5. No hang or device lost.
  • Frame times are the same as rc.5: p50 17.4 vs 18.1 ms, p99 36.0 vs 35.5 ms, load hitch 843 vs 823 ms.
  • The driver's pipeline cache was warm, so this does not exercise the queued-build wait the PR targets. It is only a no-regression check on NVIDIA.
Tested on NVIDIA (RTX 3070, Windows 11), merged onto v2.0.8-rc.5, with the scripted castle-load run. - The picture matches rc.5. No hang or device lost. - Frame times are the same as rc.5: p50 17.4 vs 18.1 ms, p99 36.0 vs 35.5 ms, load hitch 843 vs 823 ms. - The driver's pipeline cache was warm, so this does not exercise the queued-build wait the PR targets. It is only a no-regression check on NVIDIA.
duck-paradise commented 2026-11-09 16:44:06 +00:00 (Migrated from github.com)

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.bin and clear the driver's shader cache, so the draws there have to wait for builds still queued behind others. The slow frame lines with RX_VK_SLOW_LOG=150 show the long stops.

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.bin` and clear the driver's shader cache, so the draws there have to wait for builds still queued behind others. The `slow frame` lines with `RX_VK_SLOW_LOG=150` show the long stops.
hckrman101 (Migrated from github.com) approved these changes 2026-11-09 18:55:06 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks right to me. Went through the steal path:

  • Exactly-once holds: the worker's pop and the erase in jobs_wait are both under mu, so either a worker already has it (and we fall through to the normal wait) or we own it. pending-- / jobs_notify() run with lk released, so no self-deadlock on mu.
  • Running the build on the waiting thread doesn't add any lock hazard: build_graphics takes no ParLock, and draw() holds nothing at either wait_for. Any lock the job needed that the replay thread held would already have deadlocked with the worker version.
  • Thread-locals: build_graphics doesn't read guest memory or cur_program, and driver_cache_only is false on the replay thread, so ub_thread_off() not being set there doesn't matter. Same as the RX_VK_SYNC_PIPELINES path, which already builds on that thread. guarded_graphics works per thread on both Windows (SEH) and Linux (thread-local fault_jmp).
  • The GPL-linked case never waits (ready is already set), and compute/serial/translation jobs aren't flagged, so their ordering is unchanged.

CI: the Python source checks failures are tests/test_program_answer_domains.py (answers_in undeclared), which fails the same way on the base commit 8b2e1d3. The new test passes on both ubuntu and windows.

Small things, not blocking:

  • tests/run_all.py picks the test up automatically, but it isn't in ARGS, so on a box without g++ it fails instead of being skipped. Adding "tests/test_jobs_wait_runs_queued.py": ["--cxx", "{cxx}"] would match the other compiled tests.
Looks right to me. Went through the steal path: - Exactly-once holds: the worker's pop and the erase in `jobs_wait` are both under `mu`, so either a worker already has it (and we fall through to the normal wait) or we own it. `pending--` / `jobs_notify()` run with `lk` released, so no self-deadlock on `mu`. - Running the build on the waiting thread doesn't add any lock hazard: `build_graphics` takes no `ParLock`, and `draw()` holds nothing at either `wait_for`. Any lock the job needed that the replay thread held would already have deadlocked with the worker version. - Thread-locals: `build_graphics` doesn't read guest memory or `cur_program`, and `driver_cache_only` is false on the replay thread, so `ub_thread_off()` not being set there doesn't matter. Same as the `RX_VK_SYNC_PIPELINES` path, which already builds on that thread. `guarded_graphics` works per thread on both Windows (SEH) and Linux (thread-local `fault_jmp`). - The GPL-linked case never waits (ready is already set), and compute/serial/translation jobs aren't flagged, so their ordering is unchanged. CI: the `Python source checks` failures are `tests/test_program_answer_domains.py` (`answers_in` undeclared), which fails the same way on the base commit 8b2e1d3. The new test passes on both ubuntu and windows. Small things, not blocking: - `tests/run_all.py` picks the test up automatically, but it isn't in `ARGS`, so on a box without `g++` 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");
hckrman101 (Migrated from github.com) commented 2026-11-09 18:55:06 +00:00

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.

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.
Commenting is not possible because the repository is archived.
No description provided.