Repository navigation
perf(agc): release unused registered shader resource plans - #6178
adriadam10 merged 4 commits into
Conversation
7224df1 to
9f00bfa
Compare
|
@MarcZX100 I had a local fix for the same holder from #6139 before I saw this PR, so I won't open a second one. It takes a different route, and the numbers may help you decide. Mine never loads the plan at registration when the variant comes from the disk cache. The first capture loads or builds it ( One thing I noticed reading this PR: |
|
Confirmed a retained-full-IR consequence of the fallback @fma-rc identified, on head Using a small synthetic buffer shader and unmodified production recompiler, I ran
The retained fixture has two blocks/seven instructions alongside its eight-node plan, surviving every phase above. All sixteen paired materializations preserve output and prepared artifact identity. Cache-off, disk-plans-off, active-capture and failed-capture controls pass; the existing The fallback needs to release the rebuilt full IR when an existing artifact serves the request. These are CPU-only lifetime measurements; no whole-title memory or FPS effect is claimed. AI-assisted: yes (Codex) |
|
Prepared a small follow-up for this head: f1b42dae46, available to cherry-pick. It avoids keeping newly rebuilt full IR when an existing artifact serves the request, including compatible relocated push layouts. It preserves IR waiting for a new layout and validates the prepared source handle before suppressing retention. All eight lifetime scenarios now pass, including missing/corrupt/unwritable plan storage; their materialized outputs and source/artifact identities match the base. The new CPU regression fails on this PR's original head and passes with the fix; the existing resource-plan codec suite passes. Selective controls also catch dropping pending new-layout IR, omitting the relocated-layout hint, and skipping its validation. Test host: NVIDIA GeForce RTX 3060 12 GB, driver 591.86 (32.0.15.9186), Vulkan 1.4.325, Windows Server 2025 10.0.26100. This validation is CPU-only, with no GPU/title test or whole-title RAM/FPS claim. No duplicate PR opened. AI-assisted: yes Co-Authored-By: Codex noreply@openai.com |
bb7e524 to
bb4a178
Compare
|
Thanks for identifying and validating the cache-miss retention case. I rebased #6178 onto current I added a red/green regression to |
bb4a178 to
c00b0df
Compare
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
@MarcZX100 Thanks,
Results on
Minimal red test, placed after your regression in Test diff@@ -1893,6 +1893,31 @@ void verifyPlanRoundTrip() {
static_cast<void>(PrepareShader(secondLayout, &preparation));
require(preparation.ResourcePrograms() == 1u, "a plan reload retained full IR after an artifact for that layout was already available");
+ auto relocatedFixture = fixtures.front();
+ relocatedFixture.waveSize = 32u;
+ relocatedFixture.code.insert(relocatedFixture.code.begin(), 0xbf800005u);
+ const auto relocatedRequest = relocatedFixture.Request(true);
+ const auto relocatedHandle = PrepareShader(relocatedRequest);
+ static_cast<void>(GetResourcePlan(relocatedRequest));
+ ShaderDiskCache::Flush();
+ std::vector<std::byte> relocatedKey;
+ ShaderDiskCache::BuildPlanKey(relocatedRequest, 32u, relocatedKey);
+ const auto relocatedPath = ShaderDiskCache::EntryDirectory() / ShaderDiskCache::EntryName(relocatedKey);
+ std::filesystem::rename(relocatedPath, relocatedPath.string() + ".reload-preserved");
+ ReleaseUndrawnResourcePlan(*relocatedHandle);
+ auto drawn = relocatedRequest;
+ drawn.layout.pushConstantOffsetBytes = 16u;
+ drawn.layout.pushConstantSizeBytes = 112u;
+ const auto relocatedInvocation = PreparedShaderInvocation::TryCreate(drawn, relocatedHandle);
+ require(relocatedInvocation.has_value(), "the prepared artifact refused a relocated push layout");
+ RequestMemoryView drawnMemory(drawn.context.memory);
+ static_cast<void>(relocatedInvocation->Materialize(*relocatedInvocation->Capture(drawnMemory.MakeRuntime(drawn.context.userData, drawn.shader.codeAddress))));
+ auto relocatedProbe = relocatedRequest;
+ relocatedProbe.layout.pushConstantSizeBytes = 0u;
+ ShaderPreparationContext relocatedPreparation;
+ static_cast<void>(PrepareShader(relocatedProbe, &relocatedPreparation));
+ require(relocatedPreparation.ResourcePrograms() == 1u, "a plan reload retained full IR after the prepared artifact served a relocated push layout");
+
RequestMemoryView registeredMemory(registeredRequest.context.memory);
const auto runtimeCapture = CaptureResources(registeredRequest, registeredMemory.MakeRuntime(registeredRequest.context.userData, registeredRequest.shader.codeAddress), *registeredHandle);
const auto beforeKeep = ShaderDiskCache::Totals();A fix: pass the prepared handle from both capture paths through There was no GPU or title run. The cost is memory held by the retained IR; output doesn't change. AI-assisted: yes |
|
A title measurement, since the description has none: Crimson Desert (PPSA25286, 01.000.568). It compiles about 12,000 to 13,000 shaders in its first five minutes and was one of the titles growing host memory in #6139. Setup: Linux, RTX 4070 Ti, NVIDIA 595.99. Both builds are
AI-assisted: yes |
A prepared artifact also serves draws whose push constants start at another offset. When such a capture reloads a released plan that is missing from the disk cache, the rebuilt resource program must not stay on the source entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Prepared captures pass their source handle to the plan reload. When the handle matches the request, its artifact serves the draw, so the rebuilt resource program is not kept even if the push layout differs from the artifact's. A handle that does not match the source is rejected before the entry changes. Same change as f1b42da on top of c00b0df. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
I reviewed the proposed relocated-layout regression and fix, then carried the two commits onto this PR while preserving fdfawef324 as the author. Locally, |
What
Fixes the resource-plan retention reported in #6139. After registered shader preparation completes, release its cached plan when it has not been captured for runtime use and disk-plan caching is enabled. The plan is stored under the existing request cache key, so the first runtime use can reload it. Runtime capture marks the plan as used, and each capture retains its own shared plan so it remains valid across a reload.
When a released plan misses the disk cache, the recompiler must rebuild the resource plan. If the source already has a compiled variant for the exact binding layout, this rebuild no longer leaves a full IR program resident. If no matching variant exists, the IR remains available for compiling that layout.
Tested
mainata08de6827f217115078660b9f47201edae59668b.BUILD_TESTING=ON.agc_shader_disk_cache_testsand ranctest --test-dir build --output-on-failure -R '^agc_shader_disk_cache$': passed (1/1).a plan reload retained full IR; with the guard it passes.python3 tools/check_conventions.py --base upstream/main --head HEADandgit diff --check upstream/main...HEADpass.templatedisambiguator in an unrelated existing source file, then removed it; it is not part of this pull request. No GPU, Windows, or console test was run.Checklist
main; no other open PR implements the same functions