Skip to content

perf(agc): create compute pipelines before taking the GPU lock - #4897

Closed
karanvora2599 wants to merge 1 commit into
boykopovar:mainfrom
karanvora2599:perf/agc-compute-pipeline-unlocked
Closed

karanvora2599 wants to merge 1 commit into
boykopovar:mainfrom
karanvora2599:perf/agc-compute-pipeline-unlocked

Conversation

@karanvora2599

@karanvora2599 karanvora2599 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What

A compute pipeline is now created in PrepareDispatch, before the dispatch takes GuestMemory::GpuMutex, instead of in VulkanDevice::dispatch under it.

Background.

How.

  • The creation code moves unchanged into VulkanDevice::computePipeline: module, layout, specialization, subgroup size, feat(agc): capture and replay compute pipeline compilation #3717's capture and the statistics.
  • computePipeline returns the pipeline from computePipelines, or creates and inserts it.
    • A pipeline that another thread is creating is waited for, through a shared_future per key in computePipelinesBuilding. So two queues that need the same new variant create it once.
    • If creation fails, every waiter gets the error.
  • PrepareDispatch calls it once the descriptor set layout is known: from stage A, or from the cached ShaderResources. The result travels in PreparedDispatch::objects, and dispatch() uses it.
  • dispatch() still creates the pipeline under the lock, through the same function, in three cases:
    • when the layout is only built under the lock (address-table shaders build it in Complete());
    • with APS5_LOCKED_BUILD=1;
    • with the new APS5_LOCKED_PIPELINE=1, which keeps main's behaviour for comparisons.
  • With APS5_PROFILE_DRAW=1, a pipeline over 100K words now also prints [dispatch] created a pipeline for N SPIR-V words in M ms (program 0x...).

Why creating it without the lock is safe.

  • The creation reads only:
    • the compiled shader;
    • the set layout of a ShaderResources that the dispatch owns, or that PreparedDispatch::cached keeps alive;
    • workgroupMemoryLayout, made at device creation;
    • the pipeline cache.
  • A ShaderResources sets its layout in its build and clears it only in release(), which runs from the destructor or a failed build. A cached object has finished its build.
  • The pipeline cache is created without VK_PIPELINE_CACHE_CREATE_EXTERNALLY_SYNCHRONIZED_BIT, so creating pipelines with it from several threads is valid (pipeline cache). Graphics library prebuilds (PrebuildStageLibrary) and the library job thread in PipelineLibrary.cpp already create pipelines with it outside the lock.
  • The pipeline layout comes from this dispatch's set layout. On main, the map entry made by the first dispatch of a variant already serves its later dispatches the same way.

Before / after on an RTX 4060 Laptop (Ada, driver 591.86).

  • Setup:
    • A measurement test, attached and not part of this PR, submits through sceAgcDriverSubmitAcb.
    • Its program is 8,192 independent f32 ops, which give 771,233 SPIR-V words with APS5_SPIRV_OPT=none.
    • Three runs each, alternating APS5_LOCKED_PIPELINE=1 (main's behaviour) and this change.
APS5_LOCKED_PIPELINE=1 This change
Two new programs submitted at once to compute queues 0x20 and 0x28: both done after 20.4, 20.9, 20.8 s 14.3, 14.5, 14.4 s
A cached dispatch submitted to 0x28 every 200 ms while 0x20 creates a new pipeline: worst wait 7.8, 7.5, 7.5 s 4, 3, 3 ms
  • One creation takes 7.6 to 7.9 s alone. Two at once take 9.4 s each. The rest of the first row is the recompile to SPIR-V (about 4.6 s), which already ran without the lock.
  • In the second row, the creation itself took 8.5 to 8.8 s in the unlocked runs, against 7.6 to 7.9 s locked. It ran right after the first row's two parallel creations, with 66 probe dispatches beside it instead of 26.

Limits.

  • A queue whose own next dispatch needs the new pipeline still waits for it, and so does a guest thread waiting on that queue's work. This change frees the other queues and presentation during a compile, and lets compiles from different queues overlap. Starting compiles ahead of the dispatch (the resolve-ahead path) would be a separate change.
  • A compile still takes as long as before. The compile time itself depends on the SPIR-V size, which is separate work.
  • perf(agc): pre-check indirect draw argument and count buffer coverage #4868 first also looked up the pipeline in PrepareDispatch. It has since been narrowed to the indirect draw checks and leaves that lookup to this PR.
  • fix(agc): serialize pipeline cache snapshots #4795 puts pipeline creation under a shared pipeline-cache lock. If it lands first, the moved CreateComputePipeline call takes that lock in computePipeline.

Tested

Windows 11, MinGW-w64 GCC 15.2.0, Release. Devices:

  • RTX 4060 Laptop (Ada, driver 591.86)
  • Intel UHD Graphics (Raptor Lake, driver 101.6790)
  • lavapipe from Mesa 25.2.8 (llvmpipe, LLVM 21.1.7): the version CI installs; Windows build with GALLIUM_OVERRIDE_CPU_CAPS=avx.

The full runs were on main at 43e3a70. The branch is now rebased onto d1fd937, where I reran the tests under Dispatch path on the three devices, plus agc_driver_graphics, and repeated the measurement once each way (21.1 s → 14.1 s; worst wait 7.8 s → 4 ms). They pass.

  • Dispatch path: these tests go through PrepareDispatch and the pipeline map. They pass on the three devices, with APS5_VERIFY_RECIPE=1 on the RTX 4060, and with APS5_LOCKED_PIPELINE=1 (the old path) on the RTX 4060:
    • agc_driver_prepared_*, agc_driver_compute_user_data_calls*, agc_driver_gated_dispatch_capture*;
    • agc_driver_oversized_*, agc_driver_flip_compute, agc_driver_wave32_subgroup, agc_driver_resource_cache_oversized_storage_admission;
    • agc_compute_pipeline_capture, agc_compute_pipeline_replay and agc_compute_pipeline_capture_run (feat(agc): capture and replay compute pipeline compilation #3717's capture, which moved into computePipeline).
  • Shader and driver tests (agc_driver_*, agc_shader_*, agc_spirv_*, vector_*, agc_masked_select, agc_validate_program, agc_denormal*, agc_float16_*, agc_graphics; 352 tests):
    • RTX 4060: all pass. I left out the BDA tests that lose the device on driver 591.86 (Recompiler: NVIDIA hangs (device lost) when read_bda_span lanes split between in-place loads and the fallback; branch and commit ready #3291): agc_driver_flat_loads*, agc_driver_global_loads*, agc_driver_flat_apertures*, agc_driver_pixel_interlock.
    • Intel UHD: everything passes or is skipped for missing features, except agc_driver_ds_swizzle, which fails the same way on main.
    • lavapipe 25.2.8: everything passes or is skipped for missing features, except agc_driver_depth_fast_clear_submit. It segfaults within 0.4 s in about half its runs, at the same address in vulkan_lvp.dll (+0x10fe588) as main's build of the test.

Checklist

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

PrepareDispatch now creates a missing compute pipeline once the
descriptor set layout is known, so the driver's compile no longer runs
under GuestMemory::GpuMutex. Other queues, presentation and label
writes go on during it, and compiles from different queues overlap.
Two queues that need the same new variant create it once.

The pipeline is still created in dispatch() when the layout is only
built under the lock, with APS5_LOCKED_BUILD=1, or with the new
APS5_LOCKED_PIPELINE=1.
@karanvora2599

Copy link
Copy Markdown
Contributor Author

The measurement test behind the Before / after table. It isn't part of this PR.

To run it:

  • Put it in core/libs/prx/libSceAgcDriver/tests/execution/. The test glob picks it up as agc_driver_nvidia_queue_compile_measure.
  • Run APS5_SPIRV_OPT=none ANYPS5_REQUIRE_VULKAN=1 APS5_PROFILE_DRAW=1 ctest -R '^agc_driver_nvidia_queue_compile_measure$' -V, once with APS5_LOCKED_PIPELINE=1 and once without.
  • APS5_EXP_LENGTH sets the program length; the default 8192 gives 771,233 SPIR-V words.

Each run builds new programs from a time-based seed, so the pipeline cache never serves them.

NvidiaQueueCompileMeasure.cpp
#include "VulkanTestDevice.hpp"
#include "prx/libSceAgcDriver/Execution/include/Driver.hpp"
#include "prx/libSceAgcDriver/Submit/include/Acb.hpp"
#include <algorithm>
#include <array>
#include <atomic>
#include <chrono>
#include <cstdint>
#include <cstdio>
#include <cstdlib>
#include <deque>
#include <initializer_list>
#include <iostream>
#include <stdexcept>
#include <string>
#include <thread>
#include <vector>

namespace {

void Require(bool condition, const std::string& message) {
    if (!condition) throw std::runtime_error(message);
}

constexpr std::uint32_t Threads = 32;
constexpr std::size_t CodeWords = 12288;

struct Program {
    alignas(256) std::array<std::uint32_t, CodeWords> code{};
};

struct alignas(256) Flag {
    volatile std::uint32_t value = 0;
};

std::array<Program, 6> Programs;
std::array<Flag, 8> Flags;
alignas(256) std::array<std::uint32_t, Threads * 4> Input{};
alignas(256) std::array<std::uint32_t, Threads * 16> Output{};
alignas(256) std::array<std::uint32_t, 8> UserData{};

std::uint32_t Low(const volatile void* pointer) {
    return static_cast<std::uint32_t>(reinterpret_cast<std::uintptr_t>(pointer));
}

std::uint32_t High(const volatile void* pointer) {
    return static_cast<std::uint32_t>(static_cast<std::uint64_t>(reinterpret_cast<std::uintptr_t>(pointer)) >> 32u);
}

std::uint32_t Vop2(std::uint32_t opcode, std::uint32_t dst, std::uint32_t src0, std::uint32_t vsrc1) {
    return (opcode << 25) | (dst << 17) | (vsrc1 << 9) | (256u + src0);
}

void BuildIlp(Program& program, std::uint32_t length, std::uint32_t literal) {
    std::vector<std::uint32_t> code{0x34020084u, 0x34060086u, 0xe0381000u, 0x80000401u, 0xbf8c3f70u};
    code.push_back(0x7e000000u | (9u << 17) | (1u << 9) | 255u);
    code.push_back(literal);
    for (std::uint32_t chain = 0; chain < 8u; ++chain) code.push_back(Vop2(chain % 2u == 0u ? 0x08u : 0x03u, 10u + chain, 4u + chain % 4u, 9u));
    for (std::uint32_t i = 0; i < length; ++i) {
        const std::uint32_t dst = 10u + i % 8u;
        const std::uint32_t a = 10u + (i + 3u) % 8u;
        const std::uint32_t b = 10u + (i + 5u) % 8u;
        switch (i % 4u) {
        case 0u: code.push_back(Vop2(0x08u, dst, dst, a)); break;
        case 1u: code.push_back(Vop2(0x03u, dst, a, dst)); break;
        case 2u:
            code.push_back(0xd54b0000u | dst);
            code.push_back(((256u + b) << 18) | ((256u + a) << 9) | (256u + dst));
            break;
        default: code.push_back(Vop2(0x04u, dst, dst, b)); break;
        }
    }
    for (std::uint32_t chain = 0; chain < 4u; ++chain) code.push_back(Vop2(0x03u, 10u + chain, 14u + chain, 10u + chain));
    code.insert(code.end(), {0xe0781000u, 0x80010a03u, 0xbf810000u});
    Require(code.size() <= CodeWords, "program does not fit");
    std::copy(code.begin(), code.end(), program.code.begin());
}

void SetShaderRegister(std::vector<std::uint32_t>& words, std::uint32_t offset, std::uint32_t value) {
    words.insert(words.end(), {0xc0017600u, offset, value});
}

void Dispatch(std::vector<std::uint32_t>& words, const Program& program) {
    const auto address = static_cast<std::uint64_t>(reinterpret_cast<std::uintptr_t>(program.code.data()));
    SetShaderRegister(words, 0x20c, static_cast<std::uint32_t>(address >> 8u));
    SetShaderRegister(words, 0x20d, static_cast<std::uint32_t>(address >> 40u) & 0xffu);
    SetShaderRegister(words, 0x207, Threads);
    SetShaderRegister(words, 0x208, 1);
    SetShaderRegister(words, 0x209, 1);
    SetShaderRegister(words, 0x212, 0);
    SetShaderRegister(words, 0x213, static_cast<std::uint32_t>(UserData.size()) << 1u);
    std::uint32_t offset = 0x240;
    for (const auto value : UserData) SetShaderRegister(words, offset++, value);
    words.insert(words.end(), {0xc0031500u, 1, 1, 1, 0x8041u});
}

void WriteData(std::vector<std::uint32_t>& words, Flag& flag, std::uint32_t value) {
    words.insert(words.end(), {0xc0033700u, 0x00100200u, Low(&flag.value), High(&flag.value), value});
}

void Submit(std::uint32_t queue, std::vector<std::uint32_t>& words) {
    Packet packet{words.data(), static_cast<std::uint32_t>(words.size()), 0, {}};
    Require(sceAgcDriverSubmitAcb(queue, &packet) == 0, "compute submit failed");
}

double Since(std::chrono::steady_clock::time_point start) {
    return std::chrono::duration<double, std::milli>(std::chrono::steady_clock::now() - start).count();
}

double WaitFor(Flag& flag, std::chrono::steady_clock::time_point start) {
    while (flag.value == 0) {
        Require(std::chrono::steady_clock::now() - start < std::chrono::seconds(180), "a submission did not finish");
        std::this_thread::sleep_for(std::chrono::microseconds(200));
    }
    return Since(start);
}

}

int main() {
    try {
        auto device = OpenVulkanTestDevice();
        if (!device) return VulkanTestSkipped;
        device.reset();
        for (std::uint32_t lane = 0; lane < Threads; ++lane) {
            Input[lane * 4u] = 0x3f800000u + lane;
            Input[lane * 4u + 1u] = 0x3f7ff972u;
            Input[lane * 4u + 2u] = 0x358637bdu;
            Input[lane * 4u + 3u] = 0x3f000000u;
        }
        UserData = {Low(Input.data()), High(Input.data()) & 0xffffu, static_cast<std::uint32_t>(Input.size() * 4u), 0x31016facu,
                    Low(Output.data()), High(Output.data()) & 0xffffu, static_cast<std::uint32_t>(Output.size() * 4u), 0x31016facu};
        const char* lengthText = std::getenv("APS5_EXP_LENGTH");
        const std::uint32_t length = lengthText != nullptr && *lengthText != '\0' ? static_cast<std::uint32_t>(std::stoul(lengthText)) : 8192u;
        const auto seed = static_cast<std::uint32_t>(std::chrono::steady_clock::now().time_since_epoch().count());
        for (std::uint32_t index = 0; index < 4u; ++index) BuildIlp(Programs[index], length, 0x3f800000u | ((seed + index * 7919u) & 0x7fffu) | 1u);
        BuildIlp(Programs[4], 16u, 0x3f800000u | ((seed * 3u) & 0x7fffu) | 1u);
        const char* mode = std::getenv("APS5_LOCKED_PIPELINE") != nullptr ? "locked" : "unlocked";

        std::vector<std::uint32_t> warm;
        Dispatch(warm, Programs[4]);
        WriteData(warm, Flags[0], 1);
        const auto warmStart = std::chrono::steady_clock::now();
        Submit(0x28, warm);
        WaitFor(Flags[0], warmStart);

        std::vector<std::uint32_t> first;
        Dispatch(first, Programs[0]);
        WriteData(first, Flags[1], 1);
        std::vector<std::uint32_t> second;
        Dispatch(second, Programs[1]);
        WriteData(second, Flags[2], 1);
        const auto pairStart = std::chrono::steady_clock::now();
        Submit(0x20, first);
        Submit(0x28, second);
        const double firstMs = WaitFor(Flags[1], pairStart);
        const double secondMs = WaitFor(Flags[2], pairStart);

        std::vector<std::uint32_t> big;
        Dispatch(big, Programs[2]);
        WriteData(big, Flags[3], 1);
        std::deque<std::vector<std::uint32_t>> probes;
        std::vector<double> waits;
        double worstAt = 0;
        const auto bigStart = std::chrono::steady_clock::now();
        Submit(0x20, big);
        while (Flags[3].value == 0) {
            Require(std::chrono::steady_clock::now() - bigStart < std::chrono::seconds(180), "the big submission did not finish");
            Flags[4].value = 0;
            auto& small = probes.emplace_back();
            Dispatch(small, Programs[4]);
            WriteData(small, Flags[4], 1);
            const auto smallStart = std::chrono::steady_clock::now();
            Submit(0x28, small);
            waits.push_back(WaitFor(Flags[4], smallStart));
            if (waits.back() >= *std::max_element(waits.begin(), waits.end())) worstAt = Since(bigStart) - waits.back();
            std::this_thread::sleep_for(std::chrono::milliseconds(200));
        }
        const double bigMs = Since(bigStart);
        auto sorted = waits;
        std::sort(sorted.begin(), sorted.end());

        std::fprintf(stderr, "[queues] %s, %u ops: two new programs on two queues done after %.0f and %.0f ms; %zu cached dispatches on another queue while one compiled (done after about %.0f ms): median wait %.0f ms, worst %.0f ms (submitted at %.0f ms)\n",
            mode, length, firstMs, secondMs, waits.size(), bigMs, sorted[sorted.size() / 2u], sorted.back(), worstAt);
        AgcDriverWaitIdle_nid_postfix();
        AgcDriverShutdown_nid_postfix();
        return 0;
    } catch (const std::exception& error) {
        std::cerr << error.what() << '\n';
        try {
            AgcDriverShutdown_nid_postfix();
        } catch (const std::exception& shutdownError) {
            std::cerr << shutdownError.what() << '\n';
        }
        return 1;
    }
}

@karanvora2599

Copy link
Copy Markdown
Contributor Author

@Celegans12, your Ghost of Yōtei set from #4339 is the case this PR is about. On main, each of those 80 to 470 s compiles runs under GpuMutex, so presentation and the other queues stop for the whole compile. Could you compare a Yōtei scene where giant compute pipelines get created, on the base and on the base plus this PR?

  • Cherry-picking it onto your game branch is fine, as long as both captures use the same base. The commit only touches Dispatch.cpp, VulkanDevice.cpp and VulkanDevice.hpp.
  • The pipeline cache has to be cold here: an empty VkPipelineCache and NVIDIA store, as in your perf(shader): choose the wave64 compute layout per program #4339 runs. The form's warm-cache rule would hide the effect, since a cached pipeline is never created under the lock.
  • Capture with APS5_TRACE_SECONDS=<start>:<length> over the stretch where the giants compile, and post the two aps5-summary.jsonl lines plus the traces.
  • APS5_LOCKED_PIPELINE=1 on the PR build gives main's behaviour, so one build covers both arms. A separate run with APS5_PROFILE_DRAW=1 prints [dispatch] created a pipeline for N SPIR-V words in M ms for each large pipeline.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Three things before this can go in:

  • Drop APS5_LOCKED_PIPELINE (VulkanDevice.cpp:3769); compare against a main build instead of adding an env switch.
  • Add the GPUs tested: line to the checklist.
  • The table comes from a synthetic test. Please add the summary lines of main and of this change from one title scene with a cold pipeline cache, with the traces, as the Performance report form describes. The Yōtei run you asked Celegans12 for would do.

@CnRJay CnRJay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I read through this and it looks correct. It is the right shape for moving a compile out of GpuMutex.

  • The pipeline is built from the same ShaderResources layout the dispatch binds (owned by the dispatch, or kept alive by PreparedDispatch::cached). It can't diverge from what dispatch() would have built, so no extra compiles.
  • In-flight dedup through computePipelinesBuilding means two queues that miss on the same variant compile it once. On failure both get the exception through the promise, and the entry is erased under the same mutex.
  • PrepareDispatch has one caller (Driver::dispatch), so no pipeline is built for a dispatch that never runs.
  • Teardown runs while device replacement holds the lock exclusively, and execution holds deviceReplacement shared for the dispatch, so computePipelinesBuilding.clear() can't race a build still in flight.
  • The measurement is the right one for this change: the worst wait of another queue during a compile (7.8 s → 3–4 ms). The limits section is honest that the dispatching queue still waits.

Small points:

  • If finish itself throws (try_emplace allocation), building is destroyed without a value and waiters get broken_promise instead of the original error. That is unlikely, but set_exception before the map update would avoid it.
  • The rule for whether to create here (built.Layout() != VK_NULL_HANDLE && (pushStages == 0 || maxPushConstantsSize >= ComputePushConstantBytes)) is duplicated from the checks in dispatch(). A shared helper would keep the two from drifting apart.
  • A title summary line (for example #2292 or Demon's Souls, where the stalls were reported) would complete the perf( evidence that CONTRIBUTING asks for. The synthetic test already shows the mechanism.

@MarcZX10

Copy link
Copy Markdown
Contributor

I compared this moved compute-pipeline path with #4795. That PR adds PipelineCacheSynchronization to protect vkCreateComputePipelines from a concurrent persistent-cache snapshot, and passes the guard from the existing dispatch path. This PR relocates the call into computePipeline() (around the new call to CreateComputePipeline), but that call currently uses the default null synchronization argument.

If #4795 lands first, this pre-dispatch path would bypass its guard; if this lands first, rebasing #4795 will move the call site it currently patches. Please thread the context's pipeline-cache synchronization through computePipeline() and into CreateComputePipeline, and note the dependency/coordination with #4795 so the snapshot race remains covered after integration.

@MarcZX10

Copy link
Copy Markdown
Contributor

Update: #4795 has now merged, and current main contains the CreateComputePipeline(..., PipelineCacheSynchronization*) guard. This PR's moved call still relies on the default null argument, and GitHub now reports the branch conflicts with main. Please rebase and pass context.pipelineCacheSynchronization through computePipeline() to that call; the lock must protect this pre-dispatch creation too.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
moong15 added a commit to moong15/AnyPS5 that referenced this pull request Oct 14, 2026
…d to current main

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks @karanvora2599. Carried in #5989: your commit rebased onto current main with your authorship kept, context.pipelineCacheSynchronization passed into computePipeline() as MarcZX100 asked, and APS5_LOCKED_PIPELINE dropped. It also creates the address-table shaders' pipelines before the lock, which this branch still left under it (#4977). Closing this one in its favour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* needs rebase Conflicts with main, rebase needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants