feat(vk): quick pipelines for draws whose full build is not done #234

Merged
HumanBean merged 3 commits from TrashPanda/Longshot:feat/quick-pipelines into main 2026-11-13 18:10:19 +00:00
Contributor

Why

While a worker is still building a draw's pipeline, Longshot either leaves the draw out or waits for the build. On AMD's proprietary driver the optimizer took 5-13 s per pipeline (Mako's report, by sharkiix): Volvagia's intro showed the boss as a black silhouette, and draws that had to wait froze it. This PR lets such draws use a pipeline from the caches, or a quick one built without the optimizer, until the full build is done. It is off by default on every driver (RX_VK_QUICK_MS unset) until someone measures it on AMD.

Ported from Mako, by sharkiix, github.com/Shakoquo/Mako@8292d36346 ("quick pipelines for draws whose full build is not done (boss cutscenes no longer black)").

Commits (base main @ 2e1e2e8c; #233 is merged, so nothing is stacked any more):

  • 9cf8b511, the author's: quick pipelines and the brief wait.
  • 4a629554, HumanBean's (review 1124): a busy driver-fault gate is retried, it is off by default everywhere with 8 ms suggested and a real budget, RX_VK_PIPE_SKIP=0 makes no quick pipelines, a failed full build that leaves the quick pipeline is a deliberate choice and logged, and st_pipelines no longer counts quick builds.
  • d49cd033 (review 1136): a quick build that met a busy gate no longer uses up the frame's one build. A scope guard keeps a throw from leaving the thread flags or e.quick set. A draw that must wait polls for 5 s by the clock. The docs say how far the retry reaches.

What changed

  • draw: in draw() (gpu/vk/vk_draw.cpp), a draw whose pipeline isn't ready and that has nothing to draw with calls quick_pipeline first.
    • A draw Longshot may already leave out (may_skip: attachment-only, on earlier frames' targets) is held to the budget. Without a quick pipeline it gets the brief wait (RX_VK_PIPE_WAIT_MS, off unless set) and is then left out.
    • A draw that must wait gets a quick pipeline whatever the budget, and otherwise waits for the full build as before.
    • With RX_VK_PIPE_SKIP=0 (always_wait) a draw waits for the full pipeline as on main, and no quick one is made.
    • Shaders still translating are waited for exactly as before (https://github.com/NotRexed/Hookshot/pull/1573, RX_VK_TRANSLATE_WAIT_MS). Mako's brief wait applied to translations too; here it applies only to pipelines.
  • quick_pipeline (vk_draw.cpp), with its rules in gpu/vk/quick_pipe.h (take, brief, FrameBudget):
    • It first asks the caches for the full pipeline without compiling, as pipeline_from_caches does (ask_start_first, only where the driver has pipelineCreationCacheControl). If that misses, it builds one at once without the driver's optimizer.
    • The result becomes the entry's pipeline but is not marked ready: the worker's full build replaces it (job->e->pipeline = full).
    • Each entry gets one try. Two cases don't use it up: running out of budget, and a busy driver-fault gate. In both the entry stays kNone and a later draw tries again. A quick pipeline that loses the race to the full build is destroyed, since no draw has seen it.
    • The budget: a frame's first quick build on a thread always runs. After that, a build starts only if the thread's last build would fit in what is left. "First" counts builds the gate actually let in. A cache answer doesn't count, and neither does an ask or a build that met the gate busy. So a retry early in the frame can't keep the one build from the draws after it.
    • A draw that must wait polls for a quick pipeline another thread is making, for at most 5 s by the clock (kOtherQuickWaitMs), and stops once that attempt is over.
    • quick_build, ask_start_first and the test's slow_full_build are set by a scope guard (quick_pipe::Raise), and take leaves the entry kTried if the ask or build throws. run_guarded keeps a worker running after its job throws. Without the guard, a throw would leave the flag set for that thread's later builds, and every must-wait draw of the entry would poll the whole 5 s.
  • build_graphics, through thread_local bool quick_build:
    • It adds VK_PIPELINE_CREATE_DISABLE_OPTIMIZATION_BIT and builds into no pipeline cache. It doesn't count towards pcache_built_since_save (an unoptimized pipeline in the saved cache would answer a later run's full build) or in st_pipelines.
    • It returns nothing where the full build is already made without the optimizer (RX_VK_NOOPT, AMD's task and mesh rule, the AMD Windows cloth pipeline), and for stage libraries (part >= 0).
    • A quick build doesn't wait at the driver-fault gate (issue 996). When the gate is busy it sets gate_busy, and the entry is tried again later.
  • A full build that fails, or that the driver faults on, leaves the quick pipeline in use for the session. That is deliberate: before this PR such a draw was left out. It is logged once per program and documented. A quick pipeline the full build replaced is kept for the session: nothing says when the recordings that used it are done on the GPU. The log's quick pipeline lines count these pipelines.
  • jobs: jobs_hurry(flag) (vk_jobs.cpp) moves a queued job to the front of the queue. The brief wait uses it and then waits (jobs_wait_for) within its budget. It doesn't run the build on the recording thread, as Mako did: once started, a build can't be stopped, and AMD's took seconds.
  • RX_VK_TEST_SLOW_MS (Mako's test switch) holds each worker's full build inside the driver-fault gate, so a quick build meets the busy gate as it would on AMD. Off AMD, add RX_VK_DRIVER_FAULTS=2 to make the gate serial.
  • All three switches are read strictly (quick_pipe::read_ms) and logged once when on, and are documented in docs/development.md.
  • tests:
    • New tests/test_quick_pipelines. With fake handles, it checks quick_pipe.h:

      • the budget, including a busy ask and a busy build that cost time, followed by an entry whose expected build is above the budget, which still gets the frame's one build;
      • the caches asked first, and one try per entry;
      • a lost quick pipeline destroyed, and the full build replacing a quick one;
      • a busy gate that leaves the entry to be tried again;
      • a throw that leaves the entry kTried;
      • the 5 s poll measured by the clock, with 1 ms and 15.6 ms polls.

      It also cuts vk_draw.cpp's own quick_budget_ms, pipe_wait_budget_ms and quick_pipeline out and builds them against stand-ins. That covers the caches, the flags, a busy gate at either step, a throw out of build_graphics that clears the flags, and RX_VK_QUICK_MS off by default on AMD and NVIDIA. Finally it checks source rules for build_graphics, the job and draw().

    • test_draw_side_effects: the pipeline's quick path and brief wait, a translation still waited out, and no quick path under RX_VK_PIPE_SKIP=0.

    • test_jobs_wait_runs_queued: jobs_hurry.

    • test_driver_faults, test_gpl_library_cache and test_fallback_pipeline_state follow quick_build / gate_busy / slow_full_build. Their checks are kept.

Flags and settings

Flag What it does Default now Recommendation Helps on Doesn't help on Untested on
RX_VK_QUICK_MS=<ms> Quick pipelines: up to this many ms per frame, on each recording thread, of pipelines from the caches or built without the optimizer, for draws that could be left out. A frame's first build always runs. Draws that must wait get one whatever the budget. 0 or unset turns it off; anything that isn't a number is ignored with a warning Off on every driver 8 to try it (half a 60 fps frame, as RX_VK_TRANSLATE_WAIT_MS). On AMD, where a quick build takes tens of ms, 8 in practice means about one quick build a frame on each recording thread, each a hitch of tens of ms. Turn it on by default for AMD's proprietary driver in a follow-up with numbers AMD's proprietary driver, where the optimizer took 5-13 s per pipeline (Mako's report, RX 6000 on Windows) NVIDIA, RADV, Intel: their full builds take milliseconds. On AMD, draws whose program is already building in the driver (see Risk) Everything in Longshot: not run in the game
RX_VK_PIPE_WAIT_MS=<ms> Brief wait: a skippable draw's queued full build moves to the front of the queue and is waited for, up to this many ms per frame on each recording thread, before the draw is left out. Pipelines only Off Off by default. Try 100 (Mako's default) where draws flicker in for a frame or two on first sight Draws whose full build finishes within a few ms of being reached AMD's slow optimizer (quick pipelines cover that) Everything: not run in the game
RX_VK_TEST_SLOW_MS=<ms> Test: every worker's full build is held this long (at most 60000) inside the driver-fault gate. Off AMD, add RX_VK_DRIVER_FAULTS=2 so the gate is serial as on AMD Off Testers only Testing Play Not run in the game

Measurements

None in the game: there is no game dump on this PC, and no AMD card with the proprietary driver. The motivating report is Mako's (sharkiix, 8292d363): AMD's optimizer took 5-13 s per pipeline in Volvagia's intro, which drew as a black silhouette. With quick pipelines the boss cutscenes were no longer black, and draws that had to wait no longer froze loading.

main: 2e1e2e8c (this PR's base; main is now fbd42024, see Testing)
this PR: feat/quick-pipelines @ d49cd033
Hardware: RTX 4070 Ti, NVIDIA 595.99.02, Linux (game-free checks only)
Scene: none (no game)
Method: python -I -B tests/run_all.py --jobs 4 at each commit, and the touched tests one by one
How to reproduce (in the game, for a tester): see Testing.

main @ 2e1e2e8c this PR @ d49cd033
tests/test_quick_pipelines.py fails (no quick_pipe.h) passes (7 driver/value runs)
tests/run_all.py --jobs 4 498 passed, 1 failed, 30 skipped 499 passed, 1 failed, 30 skipped (the same failure)
PASS (amd, RX_VK_QUICK_MS unset, budget 0 ms): quick pipelines' decision
PASS (nvidia, RX_VK_QUICK_MS unset, budget 0 ms): quick pipelines' decision
PASS (amd, RX_VK_QUICK_MS 0, budget 0 ms): quick pipelines' decision
PASS (nvidia, RX_VK_QUICK_MS 250, budget 250 ms): quick pipelines' decision
PASS (amd, RX_VK_QUICK_MS 8, budget 8 ms): quick pipelines' decision
PASS (amd, RX_VK_QUICK_MS 8ms, budget 0 ms): quick pipelines' decision
PASS (nvidia, RX_VK_QUICK_MS fast, budget 0 ms): quick pipelines' decision
PASS: quick pipelines: budget a frame, caches first, never into the pipeline cache, replaced by the full build

Testing

Tested:

  • python -I -B tests/run_all.py --jobs 4 at d49cd033 and at main @ 2e1e2e8c (Linux, RTX 4070 Ti, 595.99.02): 499 / 498 passed, 30 skipped each. The one failure, test_lift_cache ("the lift's scripts import"), fails the same way on main. The skips need Windows, a Vulkan 1.3 device for readback, numpy or the SPIR-V headers, none of which is set up here.
  • Main has moved on by one commit (fbd42024, #198, which touches vk_draw.cpp). It merges cleanly, and on the merge test_quick_pipelines, test_draw_side_effects, test_driver_faults, test_gpl_library_cache, test_jobs_wait_runs_queued and test_fallback_pipeline_state pass.
  • The new cases were mutation-checked against d49cd033. With 4a629554's counting (a build that met the busy gate counted as started), the busy-build budget case fails. Without the e.quick guard, both throw cases fail. With ask_start_first set by hand, the cut-code throw case fails. A 16x longer poll bound fails the clock case. Before 4a629554, a quick build sent into pcache or quick_build set during the cache ask each made the test fail.

Not tested:

  • Not run in the game: there is no game dump on this PC and no AMD card with the proprietary driver. The PR stays WIP until a tester has run it.
  • Not built for Windows (llvm-mingw isn't installed here). The change is portable C++.
  • Draws using a quick pipeline could look slightly different from the optimized build for a few frames (floating-point differences). Nobody has looked yet.
  • How much memory the kept quick pipelines take hasn't been measured. The log counts them.

Needs other testers (open, but they can't be claimed until a maintainer adds the testing wanted label):

  • T-H86M — AMD RX 6000 or newer, Adrenalin, Windows, cold pipeline cache. Volvagia's intro and a new area, with RX_VK_QUICK_MS=8 and without it, against main. Expect vk: quick pipeline (no optimizer) for ... in X ms lines (report the times and the worst frame time), fewer draws left out, and no longer freezes than before. Watch for the known limit (see Risk): a boss whose programs are already building still draws as a silhouette.
  • T-HE7V — NVIDIA or RADV, cold pipeline cache: RX_VK_QUICK_MS=8 RX_VK_TEST_SLOW_MS=8000 RX_VK_DRIVER_FAULTS=2 into a new area, and the same without RX_VK_QUICK_MS. Expect quick pipeline lines, nothing missing for long while full builds are held, and no 8 s freezes, except for a draw whose own program is already building (that one waits, as on main).
  • T-4982 — any GPU, RX_VK_PIPE_WAIT_MS=100 against unset and main, in a new area. Expect fewer draws flickering in late, and frame-time spikes of at most about 100 ms.
  • Check for all three: geometry with a depth pre-pass must draw whole, with no z-fighting, holes or flicker where a surface switches between the quick pipeline and the full one. A depth-EQUAL colour pass drawn with one, over a pre-pass drawn with the other, needs bit-identical positions. On AMD, gl_Position is decorated Invariant (mma_layout.h), which should cover this across optimisation levels. On NVIDIA it isn't decorated, so T-HE7V is where this would show.

Screenshots

This change should show on screen (the AMD boss intro), but there is no game here to take them. A tester's before and after from the same spot is wanted.

Risk and rollback

  • How far it reaches on AMD. The driver-fault gate (issue 996) is per program, for any pipeline state. While any build of a program is in the driver, every quick attempt for that program finds the gate busy. A worker waiting at the gate also gets it back before a draw's next non-blocking try. So quick pipelines mostly come for entries whose program no worker has reached yet: a backlog, as in a new area. Draws whose program is already building stay out (skippable) or wait (must) for the whole build, as on main. That may be most of Volvagia's intro. The gate can't be skipped: a second build hangs behind a faulted one. T-H86M watches for this.
  • Hitches. With RX_VK_QUICK_MS=8 on AMD, a recording thread can make about one quick build a frame (tens of ms). Draws that must wait get one whatever the budget, as before they waited for the whole full build.
  • Quick pipelines drawn longer than intended. A full build that fails or faults leaves the quick pipeline in use for the session, where the draw was left out before. A quick pipeline could misbehave where the optimizer is what makes a shader work. The known cases (AMD mesh, the cloth pipeline) are already built without the optimizer, so no quick build is made for them.
  • Memory. Each entry that got a quick pipeline keeps one more, unoptimised pipeline for the session.
  • Rollback. Nothing changes unless RX_VK_QUICK_MS or RX_VK_PIPE_WAIT_MS is set (RX_VK_PIPE_SKIP=0 is as on main). Unsetting them, or reverting the three commits, is enough.

Reviewer notes

Start with quick_pipe.h (FrameBudget::fits / started, take, Raise), then quick_pipeline and the quick_build / gate_busy lines in build_graphics, then the pipeline block in draw() and the job in graphics_pipeline. Deliberate differences from Mako:

  • the brief wait hurries the build instead of running it on the recording thread;
  • translations are untouched;
  • no quick builds go into the pipeline cache;
  • no quick build where the full one is already unoptimized;
  • no wait at the driver-fault gate (a busy gate is retried by a later draw);
  • a lost quick pipeline is destroyed;
  • a real per-frame budget, and off by default everywhere.

Issues

None open; motivated by Mako's report (sharkiix, 8292d363). Builds on #233 (merged).

Checklist

For everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main. (Based on 2e1e2e8c. Main is one commit ahead, fbd42024 (#198); it merges cleanly and the touched tests pass on the merge.)
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
  • Tested in the game, or what wasn't is named under Testing. (Not run in the game: no dump on this PC, no AMD card. Named under Testing, with test requests open.)
  • Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (No game to measure in; only game-free results above.)
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Flags noted, and the switch reference updated (docs/development.md's switch table; the README sends test switches there).
  • Screenshots for a change that shows on screen. (No game here; asked of testers.)
  • A game-free test added for the fix, failing on main and passing here.
  • A check under tests/ that reads changed source updated in this PR, not weakened.
  • Docs updated.
## Why While a worker is still building a draw's pipeline, Longshot either leaves the draw out or waits for the build. On AMD's proprietary driver the optimizer took 5-13 s per pipeline (Mako's report, by sharkiix): Volvagia's intro showed the boss as a black silhouette, and draws that had to wait froze it. This PR lets such draws use a pipeline from the caches, or a quick one built without the optimizer, until the full build is done. **It is off by default on every driver** (`RX_VK_QUICK_MS` unset) until someone measures it on AMD. Ported from Mako, by sharkiix, https://github.com/Shakoquo/Mako/commit/8292d363469041cb77d7b4967804a43d5b3f83cd ("quick pipelines for draws whose full build is not done (boss cutscenes no longer black)"). Commits (base `main` @ 2e1e2e8c; #233 is merged, so nothing is stacked any more): - 9cf8b511, the author's: quick pipelines and the brief wait. - 4a629554, HumanBean's (review 1124): a busy driver-fault gate is retried, it is off by default everywhere with 8 ms suggested and a real budget, `RX_VK_PIPE_SKIP=0` makes no quick pipelines, a failed full build that leaves the quick pipeline is a deliberate choice and logged, and `st_pipelines` no longer counts quick builds. - d49cd033 (review 1136): a quick build that met a busy gate no longer uses up the frame's one build. A scope guard keeps a throw from leaving the thread flags or `e.quick` set. A draw that must wait polls for 5 s by the clock. The docs say how far the retry reaches. ## What changed - draw: in `draw()` (`gpu/vk/vk_draw.cpp`), a draw whose pipeline isn't ready and that has nothing to draw with calls `quick_pipeline` first. - A draw Longshot may already leave out (`may_skip`: attachment-only, on earlier frames' targets) is held to the budget. Without a quick pipeline it gets the brief wait (`RX_VK_PIPE_WAIT_MS`, off unless set) and is then left out. - A draw that must wait gets a quick pipeline whatever the budget, and otherwise waits for the full build as before. - With `RX_VK_PIPE_SKIP=0` (`always_wait`) a draw waits for the full pipeline as on main, and no quick one is made. - **Shaders still translating are waited for exactly as before** (https://github.com/NotRexed/Hookshot/pull/1573, `RX_VK_TRANSLATE_WAIT_MS`). Mako's brief wait applied to translations too; here it applies only to pipelines. - `quick_pipeline` (`vk_draw.cpp`), with its rules in `gpu/vk/quick_pipe.h` (`take`, `brief`, `FrameBudget`): - It first asks the caches for the full pipeline without compiling, as `pipeline_from_caches` does (`ask_start_first`, only where the driver has `pipelineCreationCacheControl`). If that misses, it builds one at once without the driver's optimizer. - The result becomes the entry's pipeline but is not marked ready: the worker's full build replaces it (`job->e->pipeline = full`). - Each entry gets one try. Two cases don't use it up: running out of budget, and a busy driver-fault gate. In both the entry stays `kNone` and a later draw tries again. A quick pipeline that loses the race to the full build is destroyed, since no draw has seen it. - The budget: a frame's first quick build on a thread always runs. After that, a build starts only if the thread's last build would fit in what is left. "First" counts builds the gate actually let in. A cache answer doesn't count, and neither does an ask or a build that met the gate busy. So a retry early in the frame can't keep the one build from the draws after it. - A draw that must wait polls for a quick pipeline another thread is making, for at most 5 s by the clock (`kOtherQuickWaitMs`), and stops once that attempt is over. - `quick_build`, `ask_start_first` and the test's `slow_full_build` are set by a scope guard (`quick_pipe::Raise`), and `take` leaves the entry `kTried` if the ask or build throws. `run_guarded` keeps a worker running after its job throws. Without the guard, a throw would leave the flag set for that thread's later builds, and every must-wait draw of the entry would poll the whole 5 s. - `build_graphics`, through `thread_local bool quick_build`: - It adds `VK_PIPELINE_CREATE_DISABLE_OPTIMIZATION_BIT` and builds into **no pipeline cache**. It doesn't count towards `pcache_built_since_save` (an unoptimized pipeline in the saved cache would answer a later run's full build) or in `st_pipelines`. - It returns nothing where the full build is already made without the optimizer (`RX_VK_NOOPT`, AMD's task and mesh rule, the AMD Windows cloth pipeline), and for stage libraries (`part >= 0`). - A quick build doesn't wait at the driver-fault gate (issue 996). When the gate is busy it sets `gate_busy`, and the entry is tried again later. - A full build that fails, or that the driver faults on, leaves the quick pipeline in use for the session. That is deliberate: before this PR such a draw was left out. It is logged once per program and documented. A quick pipeline the full build replaced is kept for the session: nothing says when the recordings that used it are done on the GPU. The log's quick pipeline lines count these pipelines. - jobs: `jobs_hurry(flag)` (`vk_jobs.cpp`) moves a queued job to the front of the queue. The brief wait uses it and then waits (`jobs_wait_for`) within its budget. It doesn't run the build on the recording thread, as Mako did: once started, a build can't be stopped, and AMD's took seconds. - `RX_VK_TEST_SLOW_MS` (Mako's test switch) holds each worker's full build *inside* the driver-fault gate, so a quick build meets the busy gate as it would on AMD. Off AMD, add `RX_VK_DRIVER_FAULTS=2` to make the gate serial. - All three switches are read strictly (`quick_pipe::read_ms`) and logged once when on, and are documented in `docs/development.md`. - tests: - New `tests/test_quick_pipelines`. With fake handles, it checks `quick_pipe.h`: - the budget, including a busy ask and a busy build that cost time, followed by an entry whose expected build is above the budget, which still gets the frame's one build; - the caches asked first, and one try per entry; - a lost quick pipeline destroyed, and the full build replacing a quick one; - a busy gate that leaves the entry to be tried again; - a throw that leaves the entry `kTried`; - the 5 s poll measured by the clock, with 1 ms and 15.6 ms polls. It also cuts `vk_draw.cpp`'s own `quick_budget_ms`, `pipe_wait_budget_ms` and `quick_pipeline` out and builds them against stand-ins. That covers the caches, the flags, a busy gate at either step, a throw out of `build_graphics` that clears the flags, and `RX_VK_QUICK_MS` off by default on AMD and NVIDIA. Finally it checks source rules for `build_graphics`, the job and `draw()`. - `test_draw_side_effects`: the pipeline's quick path and brief wait, a translation still waited out, and no quick path under `RX_VK_PIPE_SKIP=0`. - `test_jobs_wait_runs_queued`: `jobs_hurry`. - `test_driver_faults`, `test_gpl_library_cache` and `test_fallback_pipeline_state` follow `quick_build` / `gate_busy` / `slow_full_build`. Their checks are kept. ## Flags and settings | Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on | |---|---|---|---|---|---|---| | `RX_VK_QUICK_MS=<ms>` | Quick pipelines: up to this many ms per frame, on each recording thread, of pipelines from the caches or built without the optimizer, for draws that could be left out. A frame's first build always runs. Draws that must wait get one whatever the budget. `0` or unset turns it off; anything that isn't a number is ignored with a warning | **Off on every driver** | `8` to try it (half a 60 fps frame, as `RX_VK_TRANSLATE_WAIT_MS`). On AMD, where a quick build takes tens of ms, 8 in practice means **about one quick build a frame on each recording thread**, each a hitch of tens of ms. Turn it on by default for AMD's proprietary driver in a follow-up with numbers | AMD's proprietary driver, where the optimizer took 5-13 s per pipeline (Mako's report, RX 6000 on Windows) | NVIDIA, RADV, Intel: their full builds take milliseconds. On AMD, draws whose program is already building in the driver (see Risk) | Everything in Longshot: not run in the game | | `RX_VK_PIPE_WAIT_MS=<ms>` | Brief wait: a skippable draw's queued full build moves to the front of the queue and is waited for, up to this many ms per frame on each recording thread, before the draw is left out. Pipelines only | Off | Off by default. Try `100` (Mako's default) where draws flicker in for a frame or two on first sight | Draws whose full build finishes within a few ms of being reached | AMD's slow optimizer (quick pipelines cover that) | Everything: not run in the game | | `RX_VK_TEST_SLOW_MS=<ms>` | Test: every worker's full build is held this long (at most 60000) inside the driver-fault gate. Off AMD, add `RX_VK_DRIVER_FAULTS=2` so the gate is serial as on AMD | Off | Testers only | Testing | Play | Not run in the game | ## Measurements None in the game: there is no game dump on this PC, and no AMD card with the proprietary driver. The motivating report is Mako's (sharkiix, 8292d363): AMD's optimizer took 5-13 s per pipeline in Volvagia's intro, which drew as a black silhouette. With quick pipelines the boss cutscenes were no longer black, and draws that had to wait no longer froze loading. main: 2e1e2e8c (this PR's base; main is now fbd42024, see Testing) this PR: feat/quick-pipelines @ d49cd033 Hardware: RTX 4070 Ti, NVIDIA 595.99.02, Linux (game-free checks only) Scene: none (no game) Method: `python -I -B tests/run_all.py --jobs 4` at each commit, and the touched tests one by one How to reproduce (in the game, for a tester): see Testing. | | main @ 2e1e2e8c | this PR @ d49cd033 | |---|---|---| | `tests/test_quick_pipelines.py` | fails (no `quick_pipe.h`) | passes (7 driver/value runs) | | `tests/run_all.py --jobs 4` | 498 passed, 1 failed, 30 skipped | 499 passed, 1 failed, 30 skipped (the same failure) | ``` PASS (amd, RX_VK_QUICK_MS unset, budget 0 ms): quick pipelines' decision PASS (nvidia, RX_VK_QUICK_MS unset, budget 0 ms): quick pipelines' decision PASS (amd, RX_VK_QUICK_MS 0, budget 0 ms): quick pipelines' decision PASS (nvidia, RX_VK_QUICK_MS 250, budget 250 ms): quick pipelines' decision PASS (amd, RX_VK_QUICK_MS 8, budget 8 ms): quick pipelines' decision PASS (amd, RX_VK_QUICK_MS 8ms, budget 0 ms): quick pipelines' decision PASS (nvidia, RX_VK_QUICK_MS fast, budget 0 ms): quick pipelines' decision PASS: quick pipelines: budget a frame, caches first, never into the pipeline cache, replaced by the full build ``` ## Testing **Tested:** - `python -I -B tests/run_all.py --jobs 4` at d49cd033 and at main @ 2e1e2e8c (Linux, RTX 4070 Ti, 595.99.02): 499 / 498 passed, 30 skipped each. The one failure, `test_lift_cache` ("the lift's scripts import"), fails the same way on main. The skips need Windows, a Vulkan 1.3 device for readback, numpy or the SPIR-V headers, none of which is set up here. - Main has moved on by one commit (fbd42024, #198, which touches `vk_draw.cpp`). It merges cleanly, and on the merge `test_quick_pipelines`, `test_draw_side_effects`, `test_driver_faults`, `test_gpl_library_cache`, `test_jobs_wait_runs_queued` and `test_fallback_pipeline_state` pass. - The new cases were mutation-checked against d49cd033. With 4a629554's counting (a build that met the busy gate counted as started), the busy-build budget case fails. Without the `e.quick` guard, both throw cases fail. With `ask_start_first` set by hand, the cut-code throw case fails. A 16x longer poll bound fails the clock case. Before 4a629554, a quick build sent into `pcache` or `quick_build` set during the cache ask each made the test fail. **Not tested:** - Not run in the game: there is no game dump on this PC and no AMD card with the proprietary driver. The PR stays WIP until a tester has run it. - Not built for Windows (llvm-mingw isn't installed here). The change is portable C++. - Draws using a quick pipeline could look slightly different from the optimized build for a few frames (floating-point differences). Nobody has looked yet. - How much memory the kept quick pipelines take hasn't been measured. The log counts them. **Needs other testers** (open, but they can't be claimed until a maintainer adds the `testing wanted` label): - **T-H86M** — AMD RX 6000 or newer, Adrenalin, Windows, cold pipeline cache. Volvagia's intro and a new area, with `RX_VK_QUICK_MS=8` and without it, against main. Expect `vk: quick pipeline (no optimizer) for ... in X ms` lines (report the times and the worst frame time), fewer draws left out, and no longer freezes than before. Watch for the known limit (see Risk): a boss whose programs are already building still draws as a silhouette. - **T-HE7V** — NVIDIA or RADV, cold pipeline cache: `RX_VK_QUICK_MS=8 RX_VK_TEST_SLOW_MS=8000 RX_VK_DRIVER_FAULTS=2` into a new area, and the same without `RX_VK_QUICK_MS`. Expect quick pipeline lines, nothing missing for long while full builds are held, and no 8 s freezes, except for a draw whose own program is already building (that one waits, as on main). - **T-4982** — any GPU, `RX_VK_PIPE_WAIT_MS=100` against unset and main, in a new area. Expect fewer draws flickering in late, and frame-time spikes of at most about 100 ms. - **Check for all three:** geometry with a depth pre-pass must draw whole, with no z-fighting, holes or flicker where a surface switches between the quick pipeline and the full one. A depth-EQUAL colour pass drawn with one, over a pre-pass drawn with the other, needs bit-identical positions. On AMD, `gl_Position` is decorated `Invariant` (mma_layout.h), which should cover this across optimisation levels. On NVIDIA it isn't decorated, so T-HE7V is where this would show. ## Screenshots This change should show on screen (the AMD boss intro), but there is no game here to take them. A tester's before and after from the same spot is wanted. ## Risk and rollback - **How far it reaches on AMD.** The driver-fault gate (issue 996) is per program, for any pipeline state. While any build of a program is in the driver, every quick attempt for that program finds the gate busy. A worker waiting at the gate also gets it back before a draw's next non-blocking try. So quick pipelines mostly come for entries whose program no worker has reached yet: a backlog, as in a new area. Draws whose program is already building stay out (skippable) or wait (must) for the whole build, as on main. That may be most of Volvagia's intro. The gate can't be skipped: a second build hangs behind a faulted one. T-H86M watches for this. - **Hitches.** With `RX_VK_QUICK_MS=8` on AMD, a recording thread can make about one quick build a frame (tens of ms). Draws that must wait get one whatever the budget, as before they waited for the whole full build. - **Quick pipelines drawn longer than intended.** A full build that fails or faults leaves the quick pipeline in use for the session, where the draw was left out before. A quick pipeline could misbehave where the optimizer is what makes a shader work. The known cases (AMD mesh, the cloth pipeline) are already built without the optimizer, so no quick build is made for them. - **Memory.** Each entry that got a quick pipeline keeps one more, unoptimised pipeline for the session. - **Rollback.** Nothing changes unless `RX_VK_QUICK_MS` or `RX_VK_PIPE_WAIT_MS` is set (`RX_VK_PIPE_SKIP=0` is as on main). Unsetting them, or reverting the three commits, is enough. ## Reviewer notes Start with `quick_pipe.h` (`FrameBudget::fits` / `started`, `take`, `Raise`), then `quick_pipeline` and the `quick_build` / `gate_busy` lines in `build_graphics`, then the pipeline block in `draw()` and the job in `graphics_pipeline`. Deliberate differences from Mako: - the brief wait hurries the build instead of running it on the recording thread; - translations are untouched; - no quick builds go into the pipeline cache; - no quick build where the full one is already unoptimized; - no wait at the driver-fault gate (a busy gate is retried by a later draw); - a lost quick pipeline is destroyed; - a real per-frame budget, and off by default everywhere. ## Issues None open; motivated by Mako's report (sharkiix, 8292d363). Builds on https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/233 (merged). ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [ ] Rebased on current `main`. (Based on 2e1e2e8c. Main is one commit ahead, fbd42024 (#198); it merges cleanly and the touched tests pass on the merge.) - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [ ] Tested in the game, or what wasn't is named under Testing. (Not run in the game: no dump on this PC, no AMD card. Named under Testing, with test requests open.) - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [ ] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (No game to measure in; only game-free results above.) - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Flags noted, and the switch reference updated (docs/development.md's switch table; the README sends test switches there). - [ ] Screenshots for a change that shows on screen. (No game here; asked of testers.) - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [x] Docs updated.
fix(vk): RX_VK_JOB_STEAL=true no longer turns off a draw's pipeline build where it waits
Some checks failed
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Failing after 16s
ab7b495070
Mako's 0d93fb19 (an RX 6600: one draw waited 118 s behind 26 queued jobs) adds a claim so that a draw
that must wait for its pipeline builds it itself when no worker has taken it. Longshot has had the same
since aafbaf5d: jobs_wait takes a job pushed with the flag it waits for out of the queue and runs it on
the waiting thread, and graphics_pipeline (mesh programs included) and both translation jobs are pushed
with their flags. A claim beside it would be a second mechanism for the same wait, so this keeps the
steal and closes what was left:

- RX_VK_JOB_STEAL was read with atoi: =true, =on or =yes turned the steal off. It now follows
  longshot_env.h's rule (off for 0, false, off, no), and is documented in docs/development.md.
- jobs_wait says whether it ran the job here; draw() logs the first 10 times a pipeline build or a
  translation ran where it waited, with the jobs queued or running, so a tester can see it happen.
- tests/test_jobs_wait_runs_queued: the stolen job runs once and leaves nothing queued; 2000 rounds of
  free workers racing two waiters for the same job never run it twice; RX_VK_JOB_STEAL=true and " 1 "
  keep it on (main: =true hangs, the steal off), 0/off/No turn it off.

Compute pipelines (dispatch, the mesh fallback's) are not waited for behind the queue: they are pushed
urgent and the dispatch waits 50 ms then is left out, so they are left as they are.

Ported from Mako, by sharkiix, 0d93fb1972

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Assisted-by: Claude Opus 5.5
Co-authored-by: Cursor <[email protected]>
feat(vk): quick pipelines for draws whose full build is not done
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 16s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
471cb33929
A draw whose pipeline a worker is still building is left out (on an earlier frame's targets) or waits
for the build. On AMD's proprietary driver the optimizer took 5-13 s a pipeline for Volvagia's intro
(Mako's report): left out, the boss was a black silhouette; waited for, the intro froze.

Such a draw now first asks the caches for the full pipeline without compiling (as pipeline_from_caches
does, where the driver has pipelineCreationCacheControl), else gets one made at once without the
driver's optimizer, and is drawn with it until the worker's full build replaces it (quick_pipeline,
quick_pipe.h). At most RX_VK_QUICK_MS of those a frame on each recording thread for draws that could be
left out; draws that must wait get one whatever the budget, and wait for one another thread is making.

What Longshot has that Mako lacks:
- a quick build goes into no pipeline cache and does not count towards a save (build_graphics's
  quick_build): an unoptimized pipeline saved would answer a later run's full build;
- none is made where the full build is unoptimized already (RX_VK_NOOPT, AMD's task and mesh programs,
  the cloth pipeline on AMD Windows), nor while the driver-fault gate has another build of the program
  in the driver: the quick build does not wait there, and the draw goes on as before;
- shaders still translating are waited for exactly as before (#1573, RX_VK_TRANSLATE_WAIT_MS): only
  pipelines take the quick path, for draws Longshot may already leave out or that must wait;
- RX_VK_PIPE_WAIT_MS, Mako's brief wait, moves a skippable draw's queued full build to the front of the
  workers' queue (jobs_hurry) and waits for it within a budget a frame, instead of running it on the
  recording thread, where one started cannot be stopped and took seconds on AMD. Off unless set.

RX_VK_QUICK_MS is on (1000) by default only on AMD's proprietary driver, where the slow optimizer was
measured; off elsewhere, and any number turns it on anywhere. RX_VK_TEST_SLOW_MS holds each full build,
to watch it on any GPU. All three switches are read strictly (quick_pipe.h, as translate_wait.h) and
documented in docs/development.md.

tests/test_quick_pipelines: the budget a frame, the caches first, an entry given one once, a quick one
that loses to the full build destroyed, vk_draw.cpp's own quick_pipeline cut out and built against
stand-ins, and build_graphics's rules (no cache, not counted, not where the full build is unoptimized,
no wait at the gate). test_draw_side_effects: the quick path and brief wait in draw(), translations
unchanged. test_jobs_wait_runs_queued: jobs_hurry. test_driver_faults, test_gpl_library_cache and
test_fallback_pipeline_state follow build_graphics's quick_build.

Ported from Mako, by sharkiix, 8292d36346

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Assisted-by: Claude Opus 5.5
Co-authored-by: Cursor <[email protected]>
HumanBean left a comment

Reviewed 471cb339 only (ab7b4950, PR 233, taken as the base). Read-only; no build or tests run.

What a quick pipeline is here: the same build_graphics call with the same state, shaders, specialisation and dynamic state, plus VK_PIPELINE_CREATE_DISABLE_OPTIMIZATION_BIT, built into no cache. Before that it asks the caches for the full pipeline without compiling. It is not a GPL fast-link and not a fallback shader, so no specialisation constant or dynamic state is skipped. Good parts:

  • Translations are left as they were, and memory writers (https://github.com/NotRexed/Hookshot/pull/1573) still never get left out. They draw with the quick pipeline instead of waiting.
  • The swap needs no locks. A quick pipeline that loses the CAS was never published, so destroying it is safe. One that wins is never destroyed, so in-flight command buffers and bound-pipeline tracking can't see a reused handle.
  • Keeping quick builds out of pcache and out of pcache_built_since_save is right.
  • jobs_hurry with a bounded wait is better than Mako's synchronous build.
  • The default is unchanged off AMD's proprietary driver.

Must change

  1. On AMD, the target hardware, a busy driver-fault gate uses up the entry's one try. set_serial is on for every AMD card (vendorID 0x1002). The cache ask and the quick build both use begin(p.key, false), so either gets kBusy while any build of the same program is in the driver. That includes the entry's own full build, which graphics_pipeline queued a moment earlier and which an idle worker may already have taken. take then stores kTried, so that entry never gets a quick pipeline again. Its skippable draws stay out for the full 5-13 s, and its must draws stall as before. This is the scenario the PR is for. Leave the entry at kNone (retry next frame) when the build answered kBusy, and keep kTried for "made" and for "won't ever be made" (NOOPT, part, faulted). Also, RX_VK_TEST_SLOW_MS sleeps before build_graphics and so outside the gate, and the gate is serial only on AMD. The NVIDIA/RADV test plan therefore never exercises this path. Hold the sleep inside the gate, or ask testers for RX_VK_DRIVER_FAULTS=2 as well.
  2. A new default with no measurement. It is on by default (1000 ms) on AMD's proprietary driver, but no run on any hardware has been reported, and that driver is the only place it's on. AGENTS.md asks a new default to name the hardware it was measured on. Either get the AMD test run before pr ready, or ship it opt-in (RX_VK_QUICK_MS unset = 0 everywhere) and flip it in a follow-up with the numbers. Also, "Rebased on current main" is ticked, but the base is fdc2840b and main is now 1ef93e79. Nothing conflicts (those two commits touch none of these files), so either rebase or untick the box.

Should change

  • The budget is 1000 ms a frame, on each recording thread. That's effectively no budget for skippable draws: the policy accepts leaving these draws out, and this trades that for frame hitches of up to a second. Also, take checks the budget before a build, so the last build always runs past it. Say why 1000 (what Mako ships) and measure the worst hitch on AMD, or start lower.
  • RX_VK_PIPE_SKIP=0 no longer means "wait for every pipeline". always_wait draws are must, and they now draw with a quick pipeline when RX_VK_QUICK_MS is on. Either skip the quick path under always_wait, or update the switch's row.
  • Each entry can now keep two pipelines for the session. The replaced quick pipeline (or the cache-asked one) is never destroyed, and pipelines is never cleared. On a cold AMD cache that's most new pipelines, as unoptimised code. Log a running count, or retire them once the frames that might hold them are done.
  • When the full build fails, the quick pipeline stays and is drawn with for the session. Before, such a draw was left out (SKIP_HERE). That's probably what you want, but it also covers a program the driver later faults on (the "left out" log line is then wrong). State it in the description, and make it a deliberate choice.
  • Depth-EQUAL passes. A pre-pass drawn with the optimised pipeline and its colour pass drawn with a quick one, or the other way round, need bit-identical positions. On AMD gl_Position is decorated Invariant (mma_layout.h), which should cover this across optimisation levels, but it's off on NVIDIA, where testers are asked to run with RX_VK_QUICK_MS=1000. Add "geometry with a depth pre-pass draws whole, no z-fighting or holes" to the testers' checks.
  • Nits: st_pipelines and st_pipeline_us now count quick builds too, so the stats line mixes the two. kOtherQuickPolls is 5000 × sleep_for(1ms), which is about 15 ms a sleep on Windows without a raised timer resolution; waiting on done_cv or quick.wait would be tighter.

Not checked: game behaviour on any GPU, the Windows build, and the PR's test results (not run, by the brief).

Reviewed 471cb339 only (ab7b4950, PR 233, taken as the base). Read-only; no build or tests run. **What a quick pipeline is here:** the same `build_graphics` call with the same state, shaders, specialisation and dynamic state, plus `VK_PIPELINE_CREATE_DISABLE_OPTIMIZATION_BIT`, built into no cache. Before that it asks the caches for the full pipeline without compiling. It is not a GPL fast-link and not a fallback shader, so no specialisation constant or dynamic state is skipped. Good parts: - Translations are left as they were, and memory writers (https://github.com/NotRexed/Hookshot/pull/1573) still never get left out. They draw with the quick pipeline instead of waiting. - The swap needs no locks. A quick pipeline that loses the CAS was never published, so destroying it is safe. One that wins is never destroyed, so in-flight command buffers and bound-pipeline tracking can't see a reused handle. - Keeping quick builds out of `pcache` and out of `pcache_built_since_save` is right. - `jobs_hurry` with a bounded wait is better than Mako's synchronous build. - The default is unchanged off AMD's proprietary driver. ## Must change 1. **On AMD, the target hardware, a busy driver-fault gate uses up the entry's one try.** `set_serial` is on for every AMD card (vendorID 0x1002). The cache ask and the quick build both use `begin(p.key, false)`, so either gets `kBusy` while *any* build of the same program is in the driver. That includes the entry's own full build, which `graphics_pipeline` queued a moment earlier and which an idle worker may already have taken. `take` then stores `kTried`, so that entry never gets a quick pipeline again. Its skippable draws stay out for the full 5-13 s, and its must draws stall as before. This is the scenario the PR is for. Leave the entry at `kNone` (retry next frame) when the build answered `kBusy`, and keep `kTried` for "made" and for "won't ever be made" (NOOPT, part, faulted). Also, `RX_VK_TEST_SLOW_MS` sleeps before `build_graphics` and so outside the gate, and the gate is serial only on AMD. The NVIDIA/RADV test plan therefore never exercises this path. Hold the sleep inside the gate, or ask testers for `RX_VK_DRIVER_FAULTS=2` as well. 2. **A new default with no measurement.** It is on by default (1000 ms) on AMD's proprietary driver, but no run on any hardware has been reported, and that driver is the only place it's on. AGENTS.md asks a new default to name the hardware it was measured on. Either get the AMD test run before `pr ready`, or ship it opt-in (`RX_VK_QUICK_MS` unset = 0 everywhere) and flip it in a follow-up with the numbers. Also, "Rebased on current `main`" is ticked, but the base is fdc2840b and main is now 1ef93e79. Nothing conflicts (those two commits touch none of these files), so either rebase or untick the box. ## Should change - **The budget is 1000 ms a frame, on each recording thread.** That's effectively no budget for skippable draws: the policy accepts leaving these draws out, and this trades that for frame hitches of up to a second. Also, `take` checks the budget before a build, so the last build always runs past it. Say why 1000 (what Mako ships) and measure the worst hitch on AMD, or start lower. - **`RX_VK_PIPE_SKIP=0` no longer means "wait for every pipeline".** `always_wait` draws are `must`, and they now draw with a quick pipeline when `RX_VK_QUICK_MS` is on. Either skip the quick path under `always_wait`, or update the switch's row. - **Each entry can now keep two pipelines for the session.** The replaced quick pipeline (or the cache-asked one) is never destroyed, and `pipelines` is never cleared. On a cold AMD cache that's most new pipelines, as unoptimised code. Log a running count, or retire them once the frames that might hold them are done. - **When the full build fails, the quick pipeline stays and is drawn with for the session.** Before, such a draw was left out (`SKIP_HERE`). That's probably what you want, but it also covers a program the driver later faults on (the "left out" log line is then wrong). State it in the description, and make it a deliberate choice. - **Depth-EQUAL passes.** A pre-pass drawn with the optimised pipeline and its colour pass drawn with a quick one, or the other way round, need bit-identical positions. On AMD `gl_Position` is decorated `Invariant` (mma_layout.h), which should cover this across optimisation levels, but it's off on NVIDIA, where testers are asked to run with `RX_VK_QUICK_MS=1000`. Add "geometry with a depth pre-pass draws whole, no z-fighting or holes" to the testers' checks. - Nits: `st_pipelines` and `st_pipeline_us` now count quick builds too, so the stats line mixes the two. `kOtherQuickPolls` is 5000 × `sleep_for(1ms)`, which is about 15 ms a sleep on Windows without a raised timer resolution; waiting on `done_cv` or `quick.wait` would be tighter. Not checked: game behaviour on any GPU, the Windows build, and the PR's test results (not run, by the brief).
@ -0,0 +11,4 @@
namespace quick_pipe {
/// RX_VK_QUICK_MS unset: this budget on AMD's proprietary driver, where the slow optimizer was measured; none elsewhere
constexpr double kQuickDefaultMs = 1000;
Owner

1000 ms a frame on each recording thread is effectively no budget for draws the policy already accepts leaving out: it trades a missing draw for a hitch of up to a second (more, since the budget is checked before a build, not after). This is also the new AMD default, and nothing has been measured. Say where 1000 comes from (Mako's value?), and measure the worst frame on AMD, or start lower / opt-in.

1000 ms a frame on each recording thread is effectively no budget for draws the policy already accepts leaving out: it trades a missing draw for a hitch of up to a second (more, since the budget is checked before a build, not after). This is also the new AMD default, and nothing has been measured. Say where 1000 comes from (Mako's value?), and measure the worst frame on AMD, or start lower / opt-in.
@ -0,0 +73,4 @@
for (int i = 0; must && i < kOtherQuickPolls && e.quick.load(std::memory_order_acquire) == kMaking && !there() &&
!e.ready.load(std::memory_order_acquire);
i++)
poll();
Owner

Nit: sleep_for(1ms) is about 15.6 ms on Windows unless the timer resolution has been raised, so a must draw polls more coarsely than the comment says. Waiting on e.quick (C++20 atomic::wait) or done_cv would wake on the change.

Nit: `sleep_for(1ms)` is about 15.6 ms on Windows unless the timer resolution has been raised, so a must draw polls more coarsely than the comment says. Waiting on `e.quick` (C++20 `atomic::wait`) or `done_cv` would wake on the change.
@ -0,0 +88,4 @@
} else {
got = there() ? Got::kOther : Got::kNothing;
}
e.quick.store(kTried, std::memory_order_release);
Owner

This also stores kTried when the build returned nothing only because the driver-fault gate was busy (kBusy). On AMD the gate is serial on every card, and the entry's own full build, queued a moment earlier, is often already in the driver. That uses up the entry's only try for good: its skippable draws stay out for the full 5-13 s, and its must draws stall. Keep kNone (retry next frame) for a busy gate, and keep kTried for made or never-makeable.

This also stores `kTried` when the build returned nothing only because the driver-fault gate was busy (`kBusy`). On AMD the gate is serial on every card, and the entry's own full build, queued a moment earlier, is often already in the driver. That uses up the entry's only try for good: its skippable draws stay out for the full 5-13 s, and its must draws stall. Keep `kNone` (retry next frame) for a busy gate, and keep `kTried` for made or never-makeable.
@ -2252,2 +2264,4 @@
}();
if (slow_ms > 0) std::this_thread::sleep_for(std::chrono::milliseconds(slow_ms));
use_counters.made_how = 0;
if (const VkPipeline full = build_graphics(*job->p, job->s, job->at, job->topo)) job->e->pipeline = full;
Owner

When this build fails (null), the quick pipeline stays as e->pipeline and the entry is marked ready, so the draw uses the unoptimised pipeline for the session where before it was SKIP_HERE. That includes a program the driver faults on. Probably fine, but state it, and the "left out" log line is then wrong. The replaced quick pipeline is also never destroyed, so it's one extra pipeline per entry for the session: worth a counter in the log at least.

When this build fails (null), the quick pipeline stays as `e->pipeline` and the entry is marked ready, so the draw uses the unoptimised pipeline for the session where before it was `SKIP_HERE`. That includes a program the driver faults on. Probably fine, but state it, and the "left out" log line is then wrong. The replaced quick pipeline is also never destroyed, so it's one extra pipeline per entry for the session: worth a counter in the log at least.
@ -2249,3 +2252,3 @@
job->predicted = predicted;
jobs_push([job] {
// (over a linked one, which is left alone: draws recorded with it may still be on their way)
// (over a linked one or a quick one, which are left alone: draws recorded with them may still be on their way)
Owner

RX_VK_TEST_SLOW_MS sleeps here, outside the driver-fault gate, and the gate is serial only on AMD. So the suggested NVIDIA/RADV test never has a quick build meet a full build in progress, which is the AMD case. Hold the sleep inside build_graphics after begin (full builds only), or have testers add RX_VK_DRIVER_FAULTS=2.

`RX_VK_TEST_SLOW_MS` sleeps here, outside the driver-fault gate, and the gate is serial only on AMD. So the suggested NVIDIA/RADV test never has a quick build meet a full build in progress, which is the AMD case. Hold the sleep inside `build_graphics` after `begin` (full builds only), or have testers add `RX_VK_DRIVER_FAULTS=2`.
@ -2488,3 +2507,2 @@
if (gated) {
const auto gate = fault_quarantine().begin(p.key, !cold);
if (gate == driver_faults::Quarantine::kBusy) return *cold = true, VK_NULL_HANDLE;
const auto gate = fault_quarantine().begin(p.key, !cold && !quick_build);
Owner

With wait=false this answers kBusy while any pipeline of p.key is in the driver, not only this entry's full build. In a new area on AMD (serial gate, 5-13 s builds), many quick attempts will get nothing here, and quick_pipe.h:91 then marks the entry tried. The cache ask a few lines up is gated the same way.

With `wait=false` this answers `kBusy` while *any* pipeline of `p.key` is in the driver, not only this entry's full build. In a new area on AMD (serial gate, 5-13 s builds), many quick attempts will get nothing here, and quick_pipe.h:91 then marks the entry tried. The cache ask a few lines up is gated the same way.
@ -6924,0 +7024,4 @@
// (longshot, from Mako) one to draw with until the full build is done: from the caches, or quick (quick_pipeline). Else a
// draw that may be left out first waits briefly for its build, moved to the queue's front (RX_VK_PIPE_WAIT_MS, off unless
// set), and one that may not waits for it, running it here if no worker has (jobs_wait)
if (!pe->pipeline.load(std::memory_order_acquire) && !quick_pipeline(*pe, p, s, at, topo, !may_skip)) {
Owner

!may_skip includes always_wait (RX_VK_PIPE_SKIP=0). With quick pipelines on, those draws now draw with a quick pipeline instead of waiting for every pipeline, as the switch's row in docs/development.md still says they do. Skip the quick path under always_wait, or update the row.

`!may_skip` includes `always_wait` (`RX_VK_PIPE_SKIP=0`). With quick pipelines on, those draws now draw with a quick pipeline instead of waiting for every pipeline, as the switch's row in docs/development.md still says they do. Skip the quick path under `always_wait`, or update the row.
HumanBean added this to the v3.0.0 milestone 2026-11-13 16:59:40 +00:00
HumanBean force-pushed feat/quick-pipelines from 471cb33929
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 16s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to 4698784874
All checks were successful
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Third-party parts / The same as upstream (pull_request) Has been skipped
Website / Website on Cloudflare (pull_request) Has been skipped
CI / Conventional Commit title (pull_request) Successful in 10s
PR description / Description follows the template (pull_request_target) Successful in 10s
CI / Workflow lint (actionlint) (pull_request) Successful in 9s
Third-party parts / As their notes say (pull_request) Successful in 14s
2026-11-13 17:08:32 +00:00
Compare
HumanBean left a comment

Re-review at 46987848 (9cf8b511, the author's, plus 46987848, the maintainer's fix). The base is main@origin 2e1e2e8c, which is current main. #233 is merged as d5fc892f. I only read the code: no build, no tests, nothing run in the game.

Earlier review (1124, at 471cb339)

Point Now
A busy driver-fault gate used up the entry's one try Resolved. build_graphics sets gate_busy on kBusy, and take leaves the entry at kNone. kTried stays for "made", for "lost the CAS", and for "won't ever be made" (NOOPT, part >= 0, kFaulted, a failed create). I went through every branch and they all hold.
RX_VK_TEST_SLOW_MS slept outside the gate Resolved. The sleep now runs after begin, for the worker's build only, and the row says to add RX_VK_DRIVER_FAULTS=2 off AMD.
A new default with no measurement Resolved. It is off on every driver now. quick_ms no longer takes the driver.
"Rebased on current main" ticked but false Resolved. It is true at this head.
A budget of 1000 ms a frame Resolved, but the new fits has a bug (Must change 1).
RX_VK_PIPE_SKIP=0 drew with quick pipelines Resolved. always_wait goes straight to wait_for(pe->ready), as on main, and test_draw_side_effects checks that no quick pipeline is asked for.
A replaced quick pipeline is kept for the session Resolved as a choice: it is documented, and the log lines say so.
A failed full build leaves the quick pipeline Resolved. The choice is deliberate, logged once and documented.
Depth-EQUAL / invariance check for testers Open. The description was not updated.
Nit: st_pipelines mixed quick and full builds Resolved.
Nit: the sleep_for(1ms) poll Open. On Windows it is about 15 ms a poll, so 5000 polls is not the "5 s" it reads as.

What's good

  • The retry costs little: one non-blocking begin per draw, after the setup in build_graphics. It can't spin. A draw that must wait and polls stops as soon as the other attempt goes back to kNone, and then waits for the full build as on main.
  • gate_busy can't carry a stale value into a quick decision. Both lambdas clear it right before their build_graphics call and read it right after, and build_graphics doesn't call itself.
  • The swap is still sound. A quick pipeline that loses the CAS was never published, so destroying it is safe. One that wins is never destroyed. The job's plain store of the full pipeline replaces it.
  • The tests that cut source still match it at this head, as far as I can tell by reading. None was weakened, and most were tightened: the gate placement, the kBusy regex, st_pipelines, always_wait. The stubs in test_fallback_pipeline_state (gate_busy, slow_full_build, test_slow_ms, <thread>) cover every new name build_graphics uses. By reading, they should build under -Werror, but nobody has run them.

Must change

  1. FrameBudget::fits treats any time spent in the frame as its first build, so a busy retry starves the quick builds after it. fits is l > 0 && (used <= 0 || build_ms <= l). A busy retry, or a cache answer, spends a few microseconds, so used > 0. From then on, a build runs only if the thread's last build time (build_ms) fits in what is left of the budget.

    • On the target hardware, at the suggested 8, build_ms is above 8: the description itself puts AMD's quick builds at tens of ms.
    • Draw order is the same every frame. So an entry whose program is in the driver retries every frame at the same point, often its own full build, for 5-13 s per build and serially across the program's variants.
    • Every skippable draw after that entry on the same thread then gets no quick pipeline for as long as the program is in the driver.
    • The unit test misses this because its busy lambdas don't advance the clock.
    • Fix: base "the frame's first" on a build actually started this frame (a flag set when build is called and reset in left), not on used <= 0. Add a test where a busy ask costs some time and a later entry with build_ms above the budget still gets the frame's one build.
  2. The description is the PR as it was at 471cb339, not as it is now. These parts are wrong at this head:

    • "Stacked on #233": that PR is merged.
    • "Only 471cb339 belongs to this PR".
    • The flags table: "On (1000) on AMD's proprietary driver".
    • The risk section: "up to 1000 ms per frame".
    • The tester recipe: RX_VK_QUICK_MS=1000 RX_VK_TEST_SLOW_MS=8000, which now needs RX_VK_DRIVER_FAULTS=2 and the suggested value.
    • The test results: they are for 471cb339.

    The "run_all.py run and passing" box is ticked, but nothing has been run at 46987848, and this commit changed test_fallback_pipeline_state's stubs, test_quick_pipelines, test_draw_side_effects and test_driver_faults. Run python -I -B tests/run_all.py, then pr update the description and the checklist. The description also says "#1573" bare. That is Hookshot's PR and needs its full URL, https://github.com/NotRexed/Hookshot/pull/1573.

Should change

  • Say how far the retry reaches on AMD. The gate is per program (p.key), for any pipeline state. While any build of the program is in the driver, every quick attempt for that program is busy, and a waiting worker (wait=true) takes the gate back before a non-blocking retry can. In practice a quick pipeline comes only for entries whose program hasn't reached a worker yet: a backlog, as in a new area. Draws whose program is already building stay out (skippable) or wait (must) for the whole build, as on main. That may be most of Volvagia's intro. The gate can't be skipped (issue 996: a second build hangs behind a faulted one), so say so in the description and have the AMD testers watch for it.
  • The suggested 8 is in practice "one quick build a frame per recording thread". With quick builds taking tens of ms, the first build always runs and nothing after it fits. Each one is then a hitch of tens of ms, and a frame can run past the budget by one full build. Say that next to the suggestion.
  • Nit, exception paths: quick_build, ask_start_first and slow_full_build are set and cleared by hand, and e.quick is set to kMaking with no guard. run_guarded keeps a thread alive after a job throws. A throw inside build_graphics would leave slow_full_build set on that worker, and e.quick stuck at kMaking, so every must draw of that entry would poll all 5000 times. A small scope guard would close both.

Not checked: the game on any GPU, the Windows build, and whether the tests pass at this head (not run, by the brief).

Re-review at 46987848 (9cf8b511, the author's, plus 46987848, the maintainer's fix). The base is main@origin 2e1e2e8c, which is current main. https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/233 is merged as d5fc892f. I only read the code: no build, no tests, nothing run in the game. ## Earlier review (1124, at 471cb339) | Point | Now | |---|---| | A busy driver-fault gate used up the entry's one try | **Resolved.** `build_graphics` sets `gate_busy` on `kBusy`, and `take` leaves the entry at `kNone`. `kTried` stays for "made", for "lost the CAS", and for "won't ever be made" (NOOPT, `part >= 0`, `kFaulted`, a failed create). I went through every branch and they all hold. | | `RX_VK_TEST_SLOW_MS` slept outside the gate | **Resolved.** The sleep now runs after `begin`, for the worker's build only, and the row says to add `RX_VK_DRIVER_FAULTS=2` off AMD. | | A new default with no measurement | **Resolved.** It is off on every driver now. `quick_ms` no longer takes the driver. | | "Rebased on current `main`" ticked but false | **Resolved.** It is true at this head. | | A budget of 1000 ms a frame | **Resolved, but the new `fits` has a bug** (Must change 1). | | `RX_VK_PIPE_SKIP=0` drew with quick pipelines | **Resolved.** `always_wait` goes straight to `wait_for(pe->ready)`, as on main, and test_draw_side_effects checks that no quick pipeline is asked for. | | A replaced quick pipeline is kept for the session | **Resolved** as a choice: it is documented, and the log lines say so. | | A failed full build leaves the quick pipeline | **Resolved.** The choice is deliberate, logged once and documented. | | Depth-EQUAL / invariance check for testers | **Open.** The description was not updated. | | Nit: `st_pipelines` mixed quick and full builds | **Resolved.** | | Nit: the `sleep_for(1ms)` poll | **Open.** On Windows it is about 15 ms a poll, so 5000 polls is not the "5 s" it reads as. | ## What's good - The retry costs little: one non-blocking `begin` per draw, after the setup in `build_graphics`. It can't spin. A draw that must wait and polls stops as soon as the other attempt goes back to `kNone`, and then waits for the full build as on main. - `gate_busy` can't carry a stale value into a quick decision. Both lambdas clear it right before their `build_graphics` call and read it right after, and `build_graphics` doesn't call itself. - The swap is still sound. A quick pipeline that loses the CAS was never published, so destroying it is safe. One that wins is never destroyed. The job's plain store of the full pipeline replaces it. - The tests that cut source still match it at this head, as far as I can tell by reading. None was weakened, and most were tightened: the gate placement, the `kBusy` regex, `st_pipelines`, `always_wait`. The stubs in test_fallback_pipeline_state (`gate_busy`, `slow_full_build`, `test_slow_ms`, `<thread>`) cover every new name `build_graphics` uses. By reading, they should build under `-Werror`, but nobody has run them. ## Must change 1. **`FrameBudget::fits` treats any time spent in the frame as its first build, so a busy retry starves the quick builds after it.** `fits` is `l > 0 && (used <= 0 || build_ms <= l)`. A busy retry, or a cache answer, spends a few microseconds, so `used > 0`. From then on, a build runs only if the thread's last build time (`build_ms`) fits in what is left of the budget. - On the target hardware, at the suggested `8`, `build_ms` is above 8: the description itself puts AMD's quick builds at tens of ms. - Draw order is the same every frame. So an entry whose program is in the driver retries every frame at the same point, often its own full build, for 5-13 s per build and serially across the program's variants. - Every skippable draw after that entry on the same thread then gets no quick pipeline for as long as the program is in the driver. - The unit test misses this because its busy lambdas don't advance the clock. - Fix: base "the frame's first" on a build actually started this frame (a flag set when `build` is called and reset in `left`), not on `used <= 0`. Add a test where a busy ask costs some time and a later entry with `build_ms` above the budget still gets the frame's one build. 2. **The description is the PR as it was at 471cb339, not as it is now.** These parts are wrong at this head: - "Stacked on #233": that PR is merged. - "Only 471cb339 belongs to this PR". - The flags table: "On (1000) on AMD's proprietary driver". - The risk section: "up to 1000 ms per frame". - The tester recipe: `RX_VK_QUICK_MS=1000 RX_VK_TEST_SLOW_MS=8000`, which now needs `RX_VK_DRIVER_FAULTS=2` and the suggested value. - The test results: they are for 471cb339. The "`run_all.py` run and passing" box is ticked, but nothing has been run at 46987848, and this commit changed test_fallback_pipeline_state's stubs, test_quick_pipelines, test_draw_side_effects and test_driver_faults. Run `python -I -B tests/run_all.py`, then `pr update` the description and the checklist. The description also says "#1573" bare. That is Hookshot's PR and needs its full URL, https://github.com/NotRexed/Hookshot/pull/1573. ## Should change - **Say how far the retry reaches on AMD.** The gate is per program (`p.key`), for any pipeline state. While any build of the program is in the driver, every quick attempt for that program is busy, and a waiting worker (`wait=true`) takes the gate back before a non-blocking retry can. In practice a quick pipeline comes only for entries whose program hasn't reached a worker yet: a backlog, as in a new area. Draws whose program is already building stay out (skippable) or wait (must) for the whole build, as on main. That may be most of Volvagia's intro. The gate can't be skipped (issue 996: a second build hangs behind a faulted one), so say so in the description and have the AMD testers watch for it. - **The suggested `8` is in practice "one quick build a frame per recording thread".** With quick builds taking tens of ms, the first build always runs and nothing after it fits. Each one is then a hitch of tens of ms, and a frame can run past the budget by one full build. Say that next to the suggestion. - **Nit, exception paths:** `quick_build`, `ask_start_first` and `slow_full_build` are set and cleared by hand, and `e.quick` is set to `kMaking` with no guard. `run_guarded` keeps a thread alive after a job throws. A throw inside `build_graphics` would leave `slow_full_build` set on that worker, and `e.quick` stuck at `kMaking`, so every must draw of that entry would poll all 5000 times. A small scope guard would close both. Not checked: the game on any GPU, the Windows build, and whether the tests pass at this head (not run, by the brief).
@ -222,6 +222,9 @@ Pipeline-cache file ownership, temporary storage and failure behavior are descri
| `RX_VK_PIPE_SKIP=0`, `1` or `2` | Default/`1`: wait for shader translation (with `RX_VK_TRANSLATE_WAIT_MS` set, a draw whose targets hold an earlier frame waits at most that long a frame in all, then is left out). A pending pipeline can be skipped only for attachment-only draws whose targets hold an earlier frame. `0`: wait for every translation and pipeline. `2`: restore the old skip for untranslated draws with persistent targets at once; this can omit writes before the shader is known. Translated memory-writing draws still wait |
| `RX_VK_TRANSLATE_WAIT_MS=<ms>` | Off by default: each shader still translating is waited out, as #1573 does. Set (`8` is the tested value): how long, in all, each recording thread waits in a frame for shaders still translating before leaving their draws on earlier frames' targets out. For first-visit pauses of 0.2-3 s on a cold or changed program bank; a translation slower than the bound can lose a memory write, since its writes are not known before it is translated. A negative value is off; anything but a number is ignored with a warning; the value in use is logged. Compute dispatches still wait their translation out |
| `RX_VK_JOB_STEAL=0` | A draw that must wait for its shader translation or pipeline build waits for a worker to reach it, behind every job queued before it, as before. By default a job it waits for that no worker has started yet is taken out of the queue and run on the waiting thread (it runs once either way; one a worker has started is waited for). The log's `vk: a draw's ... ran where it waited` lines (the first 10) say when it happened. Off follows the on/off rule above (`0`, `false`, `off`, `no`) |
| `RX_VK_QUICK_MS=<ms>` | Quick pipelines (ported from Mako, by sharkiix): a draw whose pipeline is still being built by a worker first asks the caches for the full one without compiling (where the driver has `pipelineCreationCacheControl`), else gets one made at once without the driver's optimizer, into no pipeline cache, and is drawn with it until the worker's full build replaces it. Up to this many ms of those a frame on each recording thread for draws that could be left out: a build is not started where the thread's last one would not fit in what is left, but a frame's first always is, so a frame runs past the budget by at most one build. Draws that must wait get one whatever the budget. Off by default on every driver until measured in the game (it is for AMD's proprietary driver, where the optimizer took 5-13 s a pipeline: Mako, Volvagia's intro a black silhouette); `8` is the suggested value (half a 60 fps frame, as `RX_VK_TRANSLATE_WAIT_MS`; Mako's 1000 trades a missing draw for a hitch of up to a second); `0` or unset: off. Not made where the full build is made without the optimizer already (`RX_VK_NOOPT`, AMD's task and mesh programs); while the driver-fault gate has another build of the program in the driver (AMD, often the entry's own full build) none is made then, and a later draw tries again. Not with `RX_VK_PIPE_SKIP=0`, which waits for every pipeline. Deliberately, a full build that fails (or that the driver faults on) leaves the quick pipeline drawn with for the session, where the draw was left out before. Memory: a quick pipeline the full build replaced is not destroyed (nothing tells when the recordings that used it are done on the GPU), so each entry that got one keeps one more pipeline, unoptimised, for the session; the log's quick pipeline lines count them. Quick builds are not counted in the stats' pipelines. Anything but a number is ignored with a warning; the value in use is logged. Untested in the game in Longshot |
Owner

With quick builds taking tens of ms on AMD, 8 in practice means one quick build a frame on each recording thread: the frame's first always runs and nothing after it fits. That one is still a hitch of tens of ms. Worth saying next to the suggested value.

With quick builds taking tens of ms on AMD, `8` in practice means one quick build a frame on each recording thread: the frame's first always runs and nothing after it fits. That one is still a hitch of tens of ms. Worth saying next to the suggested value.
@ -0,0 +56,4 @@
/// frame): a frame runs past the budget by at most one build, not by every build started before it was spent
bool fits(uint64_t now, double total) {
const double l = left(now, total);
return l > 0 && (used <= 0 || build_ms <= l);
Owner

used <= 0 stands in for "the frame's first build", but any time spent sets it: a busy retry (a few µs), a cache answer, a must draw's build. Once used > 0, a build runs only if the thread's last build time (build_ms) fits in what is left. On AMD at the suggested 8 ms, quick builds take tens of ms (the description's own figure), so they never fit. Draw order is the same every frame, so an entry whose program is in the driver (5-13 s a build, serial across the program's variants) retries at the same point each frame and takes the frame's one slot. Every skippable draw after it on this thread gets nothing for that whole time. Track a build actually started this frame (a flag set when build is called and reset in left) instead of used <= 0.

`used <= 0` stands in for "the frame's first build", but any time spent sets it: a busy retry (a few µs), a cache answer, a must draw's build. Once `used > 0`, a build runs only if the thread's last build time (`build_ms`) fits in what is left. On AMD at the suggested 8 ms, quick builds take tens of ms (the description's own figure), so they never fit. Draw order is the same every frame, so an entry whose program is in the driver (5-13 s a build, serial across the program's variants) retries at the same point each frame and takes the frame's one slot. Every skippable draw after it on this thread gets nothing for that whole time. Track a build actually started this frame (a flag set when `build` is called and reset in `left`) instead of `used <= 0`.
@ -2252,2 +2271,3 @@
// pipeline stays for the session, one more per entry that got one, counted in quick_pipeline's log line)
use_counters.made_how = 0;
if (const VkPipeline full = build_graphics(*job->p, job->s, job->at, job->topo)) job->e->pipeline = full;
slow_full_build = true; // (RX_VK_TEST_SLOW_MS: held inside the driver-fault gate, build_graphics)
Owner

Nit: run_guarded keeps a worker alive after a job throws. A throw out of build_graphics here leaves slow_full_build set on that thread. Likewise a throw inside take leaves e.quick at kMaking, and every must draw of that entry then polls all 5000 times. A small scope guard for these flags (and quick_build / ask_start_first in quick_pipeline) would close it.

Nit: `run_guarded` keeps a worker alive after a job throws. A throw out of `build_graphics` here leaves `slow_full_build` set on that thread. Likewise a throw inside `take` leaves `e.quick` at `kMaking`, and every must draw of that entry then polls all 5000 times. A small scope guard for these flags (and `quick_build` / `ask_start_first` in quick_pipeline) would close it.
@ -2489,2 +2526,2 @@
const auto gate = fault_quarantine().begin(p.key, !cold);
if (gate == driver_faults::Quarantine::kBusy) return *cold = true, VK_NULL_HANDLE;
const auto gate = fault_quarantine().begin(p.key, !cold && !quick_build);
if (gate == driver_faults::Quarantine::kBusy) return gate_busy = true, cold ? (*cold = true, VK_NULL_HANDLE) : VK_NULL_HANDLE;
Owner

The gate is per program (p.key), for any state. While any build of the program is in the driver, every quick attempt for it is kBusy, and a worker waiting at begin (wait=true) takes the gate back before a non-blocking retry can get it. So the retry mostly helps entries whose program hasn't reached a worker yet (a backlog). Draws whose program is already building stay out, or wait, for the whole build, as on main. That's correct, since the gate can't be skipped (issue 996), but the description and the AMD test plan should say so.

The gate is per program (`p.key`), for any state. While any build of the program is in the driver, every quick attempt for it is `kBusy`, and a worker waiting at `begin` (`wait=true`) takes the gate back before a non-blocking retry can get it. So the retry mostly helps entries whose program hasn't reached a worker yet (a backlog). Draws whose program is already building stay out, or wait, for the whole build, as on main. That's correct, since the gate can't be skipped (issue 996), but the description and the AMD test plan should say so.
@ -0,0 +177,4 @@
{
FakeEntry g;
int asks = 0, builds = 0;
const auto ask_busy = [&](bool& busy) { return asks++, busy = true, 0; };
Owner

The busy lambdas don't advance clock, so a busy retry spends 0 and the starvation in FrameBudget::fits (quick_pipe.h:59) can't show here. Worth a case where the busy ask costs some time, then a second entry with build_ms above the budget still gets the frame's one build.

The busy lambdas don't advance `clock`, so a busy retry spends 0 and the starvation in `FrameBudget::fits` (quick_pipe.h:59) can't show here. Worth a case where the busy ask costs some time, then a second entry with `build_ms` above the budget still gets the frame's one build.
HumanBean force-pushed feat/quick-pipelines from 4698784874
All checks were successful
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Third-party parts / The same as upstream (pull_request) Has been skipped
Website / Website on Cloudflare (pull_request) Has been skipped
CI / Conventional Commit title (pull_request) Successful in 10s
PR description / Description follows the template (pull_request_target) Successful in 10s
CI / Workflow lint (actionlint) (pull_request) Successful in 9s
Third-party parts / As their notes say (pull_request) Successful in 14s
to 4a62955427
All checks were successful
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Third-party parts / The same as upstream (pull_request) Has been skipped
PR description / Description follows the template (pull_request_target) Successful in 25s
CI / Conventional Commit title (pull_request) Successful in 14s
CI / Workflow lint (actionlint) (pull_request) Successful in 24s
Third-party parts / As their notes say (pull_request) Successful in 52s
2026-11-13 17:16:59 +00:00
Compare
Owner

Since #233 merged, I rebased your quick-pipelines commit onto main @ 2e1e2e8c (dropping 233's) and addressed my first review in one commit of mine on top. It's pushed as 4a629554, and your commit is otherwise unchanged.

My commit, "quick pipelines retry a busy driver-fault gate, opt-in with a real budget":

  • A busy gate is retried. build_graphics reports kBusy through a gate_busy flag, and take then leaves the entry at kNone, so a later draw asks again. kTried stays for made, won't be made, faulted, and failed. RX_VK_TEST_SLOW_MS now holds the full build inside the gate, so a test can meet a busy gate. Off AMD, add RX_VK_DRIVER_FAULTS=2 for that.
  • Off by default on every driver, until there's an AMD run. RX_VK_QUICK_MS=<ms> turns it on anywhere. Please flip the AMD default back with numbers.
  • A real budget: 8 ms suggested, half a 60 fps frame, as RX_VK_TRANSLATE_WAIT_MS uses. A frame's first quick build always runs. After that, a build starts only if the thread's last one would fit in what is left. The re-review caught that my first version took "first" to mean "no time used yet", so a busy retry early in the frame used up the frame's one build. It now counts builds actually started, with a test for that case.
  • RX_VK_PIPE_SKIP=0 waits for every pipeline and makes no quick one, as its docs say.
  • A failed full build leaves the quick pipeline in use for the session. That's deliberate, and the log and docs now say so.
  • Replaced quick pipelines are kept for the session, documented. I found no point where destroying them is provably safe.
  • Counting: quick builds no longer count in st_pipelines.
  • Links: the Hookshot PR 1573 refs in the code and test are now full URLs.

tests/test_quick_pipelines.py and tests/test_draw_side_effects.py pass on my side. I didn't run the full suite, so the run_all box isn't settled at this head.

Still yours:

  • Description and checklist. They still describe 471cb339:
    • drop "stacked on #233" and "on (1000) by default on AMD";
    • the tester recipe becomes RX_VK_QUICK_MS=8, plus RX_VK_DRIVER_FAULTS=2 with RX_VK_TEST_SLOW_MS off AMD;
    • the Hookshot #1573 ref becomes a full URL;
    • untick run_all until it has run at this head.
  • How far the retry reaches. The gate is per program, and a worker waiting at it takes it back first. So quick pipelines mostly come for programs that haven't reached a worker yet. Please say that in the description and the AMD test plan, and say next to the 8 ms that it means about one quick build (a hitch of tens of ms) a frame per recording thread on AMD.
  • Testers' check: add the depth pre-pass / depth-EQUAL check.
  • Should do: a small scope guard for slow_full_build, quick_build, ask_start_first and e.quick = kMaking, so a job that throws can't leave them set.
Since https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/233 merged, I rebased your quick-pipelines commit onto main @ 2e1e2e8c (dropping 233's) and addressed my first review in one commit of mine on top. It's pushed as 4a629554, and your commit is otherwise unchanged. **My commit**, "quick pipelines retry a busy driver-fault gate, opt-in with a real budget": - **A busy gate is retried.** `build_graphics` reports `kBusy` through a `gate_busy` flag, and `take` then leaves the entry at `kNone`, so a later draw asks again. `kTried` stays for made, won't be made, faulted, and failed. `RX_VK_TEST_SLOW_MS` now holds the full build *inside* the gate, so a test can meet a busy gate. Off AMD, add `RX_VK_DRIVER_FAULTS=2` for that. - **Off by default on every driver**, until there's an AMD run. `RX_VK_QUICK_MS=<ms>` turns it on anywhere. Please flip the AMD default back with numbers. - **A real budget: 8 ms suggested,** half a 60 fps frame, as `RX_VK_TRANSLATE_WAIT_MS` uses. A frame's first quick build always runs. After that, a build starts only if the thread's last one would fit in what is left. The re-review caught that my first version took "first" to mean "no time used yet", so a busy retry early in the frame used up the frame's one build. It now counts builds actually started, with a test for that case. - **`RX_VK_PIPE_SKIP=0`** waits for every pipeline and makes no quick one, as its docs say. - **A failed full build** leaves the quick pipeline in use for the session. That's deliberate, and the log and docs now say so. - **Replaced quick pipelines** are kept for the session, documented. I found no point where destroying them is provably safe. - **Counting:** quick builds no longer count in `st_pipelines`. - **Links:** the Hookshot PR 1573 refs in the code and test are now full URLs. `tests/test_quick_pipelines.py` and `tests/test_draw_side_effects.py` pass on my side. I didn't run the full suite, so the run_all box isn't settled at this head. **Still yours:** - **Description and checklist.** They still describe 471cb339: - drop "stacked on #233" and "on (1000) by default on AMD"; - the tester recipe becomes `RX_VK_QUICK_MS=8`, plus `RX_VK_DRIVER_FAULTS=2` with `RX_VK_TEST_SLOW_MS` off AMD; - the Hookshot #1573 ref becomes a full URL; - untick run_all until it has run at this head. - **How far the retry reaches.** The gate is per program, and a worker waiting at it takes it back first. So quick pipelines mostly come for programs that haven't reached a worker yet. Please say that in the description and the AMD test plan, and say next to the 8 ms that it means about one quick build (a hitch of tens of ms) a frame per recording thread on AMD. - **Testers' check:** add the depth pre-pass / depth-EQUAL check. - **Should do:** a small scope guard for `slow_full_build`, `quick_build`, `ask_start_first` and `e.quick = kMaking`, so a job that throws can't leave them set.
fix(vk): a quick build that met a busy gate does not use up the frame's one build
Some checks failed
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Failing after 3s
d49cd0339d
Review 1136 of #234:
- FrameBudget counts a quick build as started only once the driver-fault
  gate let it in (or it will never be made). A build that met the gate
  busy (no pipelineCreationCacheControl, so no cache ask first, or the
  gate taken between the ask and the build) built nothing but was
  counted, so on AMD an entry whose program is in the driver, retried at
  the same point each frame, kept every skippable draw after it from a
  quick pipeline for the whole build. Tested with a busy build that costs
  time and a later entry whose expected build is above the budget.
- quick_pipe::Raise sets quick_build, ask_start_first and
  slow_full_build for a scope and clears them on a throw too (run_guarded
  keeps a worker running after its job throws), and take leaves the entry
  kTried, not kMaking, when ask or build throws.
- A draw that must wait polls another thread's quick attempt for
  kOtherQuickWaitMs (5000) by the clock, not 5000 polls of a 1 ms sleep
  that lasts about 15 ms on Windows.
- docs: what the suggested 8 means where a quick build takes tens of ms,
  and how far the busy-gate retry reaches on AMD.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Assisted-by: Claude Opus 5.5
Co-authored-by: Cursor <[email protected]>
Author
Contributor

Thank you, @HumanBean, for the rebase and for 4a629554. It cleared most of review 1124 in one go and kept my commit intact. I checked it against review 1136's first Must change before building on it. Fixes are in d49cd033 (head now d49cd033, base still 2e1e2e8c), and the description has been rewritten for this head.

Review 1136

Must change 1, FrameBudget::fits counting builds actually started. 4a629554 fixes the case the review describes. fits now asks builds == 0, not used <= 0. A busy cache ask returns before started is reached. Your new test (a busy ask that advances the clock, then an entry whose build is above the budget) covers that, and I confirmed it passes.

One path was still open: started was called before build. A quick build that met the busy gate itself was therefore counted as the frame's one build. That happens wherever there's no cache ask (no pipelineCreationCacheControl), or when the gate is taken between the ask and the build. An entry whose program is in the driver was then retried at the same point every frame, and starved the draws after it as before. d49cd033 counts a build only once the gate let it in (if (!busy) budget.started(...) after build). A build that will never be made (NOOPT) still counts once, since it is kTried and not retried. The new test: a busy build that costs 1 ms, at a build_ms of 400 against a budget of 8, followed by an entry that still gets the frame's one build. With 4a629554's ordering it fails: "a frame's first build still comes after a build that met the gate busy took some of its time".

Must change 2, the description. Rewritten for d49cd033:

  • "stacked on #233" and "only 471cb339" are gone, and the three commits are listed;
  • off by default everywhere;
  • RX_VK_QUICK_MS=8 suggested;
  • tester recipe RX_VK_QUICK_MS=8 RX_VK_TEST_SLOW_MS=8000 RX_VK_DRIVER_FAULTS=2 off AMD;
  • a new risk section;
  • Hookshot 1573 as https://github.com/NotRexed/Hookshot/pull/1573;
  • fresh run_all results: at d49cd033, 499 passed, 1 failed, 30 skipped; at main @ 2e1e2e8c, 498 passed, 1 failed, 30 skipped. The one failure, test_lift_cache, fails the same way on main.

"Rebased on current main" is unticked. Main moved one commit, to fbd42024 (#198, touches vk_draw.cpp). It merges cleanly, and the touched tests pass on the merge. I didn't rebase, so as not to rewrite your commit without a conflict to resolve. Say if you'd like it rebased.

Should change, how far the retry reaches on AMD. Done. The description's risk section, the AMD request (T-H86M) and the RX_VK_QUICK_MS row in docs/development.md now say it. The gate is per program, and a waiting worker takes it back first. So quick pipelines mostly come for programs no worker has reached yet. Draws whose program is already building get nothing (they are left out, or wait), as on main. That may be most of Volvagia's intro, and the gate can't be skipped (issue 996).

Should change, the suggested 8. Done. On AMD it is in practice about one quick build a frame on each recording thread, each a hitch of tens of ms. This is stated next to the suggestion in the flags table, in docs/development.md and in kQuickSuggestedMs's comment.

Nit, exception paths. Done, in d49cd033. A small quick_pipe::Raise guard sets quick_build, ask_start_first and slow_full_build for a scope and clears them on a throw. In take, a guard stores the entry's final state, kTried unless a busy gate leaves it kNone, so a throw out of the ask or the build can't leave it kMaking. Tests: a throw from either lambda leaves kTried, and a throw out of the cut build_graphics clears both flags. Each fails without its guard. fault_quarantine().begin/end around graphics_with has no guard. That is main's code, so I left it alone.

Open from the table, the depth-EQUAL / invariance check for testers. Done. It is in Testing for all three requests, with the reason: Invariant is on AMD but not on NVIDIA, so T-HE7V is where it would show.

Open from the table, the sleep_for(1ms) poll. Done by making the "5 s" true rather than by waiting on an atomic. kOtherQuickPolls (5000 polls) is now kOtherQuickWaitMs (5000 ms), measured with now_ms. A test polls in 1 ms and in 15.6 ms steps and checks that both stop at 5 s by the clock. I didn't use e.quick.wait because the loop also ends on ready (the full build), which doesn't change quick, and a wait with no timeout could hang a must draw. The coarser wake-up on Windows (a must draw may resume up to about 15 ms late) is stated in the constant's comment.

The rest of 1136's table was already resolved by 4a629554. I re-checked each point at this head and they still hold: busy gate retried, the slow-build test inside the gate, opt-in everywhere, always_wait, the kept and failed-build quick pipelines, and st_pipelines.

Review 1124

  • Must 1, busy gate uses up the try / test sleep outside the gate: fixed by 4a629554, and the starvation that followed from it by d49cd033 (above).
  • Must 2, a new default with no measurement / the rebase box: off everywhere (4a629554). The box is unticked with the reason above.
  • Should, 1000 ms budget: 8 ms suggested with a real budget (4a629554); what 8 means on AMD is documented (d49cd033).
  • Should, RX_VK_PIPE_SKIP=0: no quick path under always_wait (4a629554).
  • Should, two pipelines per entry: kept for the session by choice, documented, and counted in the log lines (4a629554). Memory isn't measured; it is listed under Not tested.
  • Should, failed full build keeps the quick pipeline: deliberate, logged and documented (4a629554), and in the description's risk section.
  • Should, depth-EQUAL: in the testers' checks (this update).
  • Nit, st_pipelines: quick builds are no longer counted (4a629554).
  • Nit, the sleep_for(1ms) poll: bounded by the clock (d49cd033).

Tests and requests

  • Touched tests pass at d49cd033, and on a merge with current main: test_quick_pipelines, test_draw_side_effects, test_driver_faults, test_fallback_pipeline_state, test_gpl_library_cache, test_jobs_wait_runs_queued.
  • Full suite at d49cd033 and main as above.
  • Not run in the game: no dump on this PC, and no AMD card. The PR stays WIP.
  • Test requests are open: T-H86M (AMD, Adrenalin), T-HE7V (NVIDIA or RADV with the slow-build recipe) and T-4982 (the brief wait, any GPU). The PR isn't from a collaborator, so nobody can claim them until a maintainer adds the testing wanted label.
Thank you, @HumanBean, for the rebase and for 4a629554. It cleared most of review 1124 in one go and kept my commit intact. I checked it against review 1136's first Must change before building on it. Fixes are in **d49cd033** (head now `d49cd033`, base still 2e1e2e8c), and the description has been rewritten for this head. ## Review 1136 **Must change 1, `FrameBudget::fits` counting builds actually started.** 4a629554 fixes the case the review describes. `fits` now asks `builds == 0`, not `used <= 0`. A busy *cache ask* returns before `started` is reached. Your new test (a busy ask that advances the clock, then an entry whose build is above the budget) covers that, and I confirmed it passes. One path was still open: `started` was called *before* `build`. A quick build that met the busy gate itself was therefore counted as the frame's one build. That happens wherever there's no cache ask (no `pipelineCreationCacheControl`), or when the gate is taken between the ask and the build. An entry whose program is in the driver was then retried at the same point every frame, and starved the draws after it as before. d49cd033 counts a build only once the gate let it in (`if (!busy) budget.started(...)` after `build`). A build that will never be made (NOOPT) still counts once, since it is `kTried` and not retried. The new test: a busy build that costs 1 ms, at a `build_ms` of 400 against a budget of 8, followed by an entry that still gets the frame's one build. With 4a629554's ordering it fails: "a frame's first build still comes after a build that met the gate busy took some of its time". **Must change 2, the description.** Rewritten for d49cd033: - "stacked on #233" and "only 471cb339" are gone, and the three commits are listed; - off by default everywhere; - `RX_VK_QUICK_MS=8` suggested; - tester recipe `RX_VK_QUICK_MS=8 RX_VK_TEST_SLOW_MS=8000 RX_VK_DRIVER_FAULTS=2` off AMD; - a new risk section; - Hookshot 1573 as https://github.com/NotRexed/Hookshot/pull/1573; - fresh `run_all` results: at d49cd033, 499 passed, 1 failed, 30 skipped; at main @ 2e1e2e8c, 498 passed, 1 failed, 30 skipped. The one failure, `test_lift_cache`, fails the same way on main. "Rebased on current `main`" is unticked. Main moved one commit, to fbd42024 (#198, touches `vk_draw.cpp`). It merges cleanly, and the touched tests pass on the merge. I didn't rebase, so as not to rewrite your commit without a conflict to resolve. Say if you'd like it rebased. **Should change, how far the retry reaches on AMD.** Done. The description's risk section, the AMD request (T-H86M) and the `RX_VK_QUICK_MS` row in docs/development.md now say it. The gate is per program, and a waiting worker takes it back first. So quick pipelines mostly come for programs no worker has reached yet. Draws whose program is already building get nothing (they are left out, or wait), as on main. That may be most of Volvagia's intro, and the gate can't be skipped (issue 996). **Should change, the suggested 8.** Done. On AMD it is in practice about one quick build a frame on each recording thread, each a hitch of tens of ms. This is stated next to the suggestion in the flags table, in docs/development.md and in `kQuickSuggestedMs`'s comment. **Nit, exception paths.** Done, in d49cd033. A small `quick_pipe::Raise` guard sets `quick_build`, `ask_start_first` and `slow_full_build` for a scope and clears them on a throw. In `take`, a guard stores the entry's final state, `kTried` unless a busy gate leaves it `kNone`, so a throw out of the ask or the build can't leave it `kMaking`. Tests: a throw from either lambda leaves `kTried`, and a throw out of the cut `build_graphics` clears both flags. Each fails without its guard. `fault_quarantine().begin/end` around `graphics_with` has no guard. That is main's code, so I left it alone. **Open from the table, the depth-EQUAL / invariance check for testers.** Done. It is in Testing for all three requests, with the reason: `Invariant` is on AMD but not on NVIDIA, so T-HE7V is where it would show. **Open from the table, the `sleep_for(1ms)` poll.** Done by making the "5 s" true rather than by waiting on an atomic. `kOtherQuickPolls` (5000 polls) is now `kOtherQuickWaitMs` (5000 ms), measured with `now_ms`. A test polls in 1 ms and in 15.6 ms steps and checks that both stop at 5 s by the clock. I didn't use `e.quick.wait` because the loop also ends on `ready` (the full build), which doesn't change `quick`, and a wait with no timeout could hang a must draw. The coarser wake-up on Windows (a must draw may resume up to about 15 ms late) is stated in the constant's comment. The rest of 1136's table was already resolved by 4a629554. I re-checked each point at this head and they still hold: busy gate retried, the slow-build test inside the gate, opt-in everywhere, `always_wait`, the kept and failed-build quick pipelines, and `st_pipelines`. ## Review 1124 - **Must 1, busy gate uses up the try / test sleep outside the gate:** fixed by 4a629554, and the starvation that followed from it by d49cd033 (above). - **Must 2, a new default with no measurement / the rebase box:** off everywhere (4a629554). The box is unticked with the reason above. - **Should, 1000 ms budget:** 8 ms suggested with a real budget (4a629554); what 8 means on AMD is documented (d49cd033). - **Should, `RX_VK_PIPE_SKIP=0`:** no quick path under `always_wait` (4a629554). - **Should, two pipelines per entry:** kept for the session by choice, documented, and counted in the log lines (4a629554). Memory isn't measured; it is listed under Not tested. - **Should, failed full build keeps the quick pipeline:** deliberate, logged and documented (4a629554), and in the description's risk section. - **Should, depth-EQUAL:** in the testers' checks (this update). - **Nit, `st_pipelines`:** quick builds are no longer counted (4a629554). - **Nit, the `sleep_for(1ms)` poll:** bounded by the clock (d49cd033). ## Tests and requests - Touched tests pass at d49cd033, and on a merge with current main: `test_quick_pipelines`, `test_draw_side_effects`, `test_driver_faults`, `test_fallback_pipeline_state`, `test_gpl_library_cache`, `test_jobs_wait_runs_queued`. - Full suite at d49cd033 and main as above. - Not run in the game: no dump on this PC, and no AMD card. The PR stays WIP. - Test requests are open: **T-H86M** (AMD, Adrenalin), **T-HE7V** (NVIDIA or RADV with the slow-build recipe) and **T-4982** (the brief wait, any GPU). The PR isn't from a collaborator, so nobody can claim them until a maintainer adds the `testing wanted` label.
HumanBean changed title from WIP: feat(vk): quick pipelines for draws whose full build is not done to feat(vk): quick pipelines for draws whose full build is not done 2026-11-13 18:10:13 +00:00
HumanBean deleted branch feat/quick-pipelines 2026-11-13 18:10:19 +00:00
Some checks failed
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Failing after 3s
Some workflows are waiting to be reviewed.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!234
No description provided.