Repository navigation
revert(agc): keep driver pipeline caches and costly compute pipeline binaries across updates (#13931) - #14839
Conversation
…binaries across updates (boykopovar#13931) This reverts commit d8b60b4, reversing changes made to d94fad1.
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — approval withheld pending validation evidence
The revert is internally consistent and removes the pipeline-binary path, but I cannot verify the required matched Vulkan validation results from the PR evidence.
What I ran
Reviewed head d6f1eccfe2a31dc4c0244808f3afd8fd73ed0faa against base e0caf1360035286bd22b9f2a3030e97b880ffb28; the exact-head merge preview is clean. Environment: Debian 12 x86_64, Python 3.11.2, GCC 12.2.0; I did not compile or build.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 14839
git diff --check e0caf1360035286bd22b9f2a3030e97b880ffb28...d6f1eccfe2a31dc4c0244808f3afd8fd73ed0faa
git grep -n -i -E 'PipelineBinaryStore|VK_KHR_PIPELINE_BINARY|APS5_NO_PIPELINE_BINARIES|pipeline_binary_store_tests|PipelineBinaryRun' d6f1eccfe2a31dc4c0244808f3afd8fd73ed0faa -- core docs
python3 tools/pr_abi_audit.py --base e0caf1360035286bd22b9f2a3030e97b880ffb28 --head d6f1eccfe2a31dc4c0244808f3afd8fd73ed0faa --json
gh pr checks 14839 --repo boykopovar/AnyPS5Results
| Check | Result | Note |
|---|---|---|
| Diff and conventions | passed | Whitespace check and local/hosted conventions pass. |
| Merge preview | passed | Clean against the exact base. |
| Removed-feature reference scan | passed | No remaining pipeline-binary implementation, flag or test references in core/ or docs/. |
| Export audit | not applicable | No exported functions changed. |
| Precheck automation | failed | Its isolated snapshot lacks workspace-only ABI and test-quality helpers; local fallback found no export changes. |
| Hosted checks | not run | Windows is pending; conventions, intake, labels, report, eligibility and library-portability pass; Linux and specialized jobs are skipped. |
| NVIDIA and C++ tests | not run | The PR reports hardware results; I did not run a build, CTest or GPU test. |
Precheck table and captured command output
| Check | Status | Command / reason |
|---|---|---|
| build_and_compile | not_applicable | `The precheck deliberately does not configure, build, or compile AnyPS5.` |
| diff_check | passed | `git diff --check e0caf1360035286bd22b9f2a3030e97b880ffb28...d6f1eccfe2a31dc4c0244808f3afd8fd73ed0faa` |
| conventions_and_pr_template | passed | `/usr/bin/python3 tools/check_conventions.py --base e0caf1360035286bd22b9f2a3030e97b880ffb28 --head d6f1eccfe2a31dc4c0244808f3afd8fd73ed0faa --title revert(agc): keep driver pipeline caches and costly compute pipeline binaries across updates (#13931) --body /tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-14839-d6f1eccfe2a3/pr-body.md` |
| changed_export_abi_audit | failed | `/usr/bin/python3 tools/pr_abi_audit.py --base e0caf1360035286bd22b9f2a3030e97b880ffb28 --head d6f1eccfe2a31dc4c0244808f3afd8fd73ed0faa --json` |
| python_syntax | not_applicable | `No changed Python implementation files.` |
| related_python_unit_tests | not_applicable | `No changed tools/ module has an existing sibling tools/tests/test_<module>.py suite.` |
| python_test_registration | not_applicable | `No changed test_*.py files.` |
| live_dependency_and_mergeability_check | passed | `/usr/bin/python3 /marc/AnyPS5/tools/pr_stack_check.py 14839 --repo boykopovar/AnyPS5 --output-dir /tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-14839-d6f1eccfe2a3/stack` |
| build_dependent_cpp_tests | not_applicable | `Not run: C++ tests need exact-head binaries, which require a build; the precheck does not build.` |
| test_quality_heuristics | failed | `/usr/bin/python3 /tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-14839-d6f1eccfe2a3/head-d6f1eccfe2a3-966d71d1/tools/pr_test_quality.py --root /tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-14839-d6f1eccfe2a3/head-d6f1eccfe2a3-966d71d1 --json core/libs/prx/libSceAgcDriver/CMakeLists.txt core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp core/libs/prx/libSceAgcDriver/Graphics/include/PipelineBinaryStore.hpp core/libs/prx/libSceAgcDriver/Graphics/include/PipelineCache.hpp core/libs/prx/libSceAgcDriver/Graphics/src/PipelineBinaryStore.cpp core/libs/prx/libSceAgcDriver/Graphics/src/PipelineCache.cpp core/libs/prx/libSceAgcDriver/tests/PipelineBinaryRun.cmake core/libs/prx/libSceAgcDriver/tests/PipelineBinaryStore.cpp core/libs/prx/libSceAgcDriver/tests/PipelineCacheFailures.cpp core/libs/prx/libSceAgcDriver/tests/Recorder.cpp core/libs/prx/libSceAgcDriver/tests/cmake/agc_driver_recorder_tests.cmake core/libs/prx/libSceAgcDriver/tests/cmake/agc_pipeline_binary_store_tests.cmake core/libs/prx/libSceAgcDriver/tests/cmake/agc_pipeline_cache_allocation_tests.cmake docs/dev/ARCHITECTURE.md docs/dev/ENVIRONMENT.md docs/dev/debt/libSceAgcDriver.md docs/user/USAGE.md` |
| negative_controls | not_applicable | `No changed Python test module in tools/ can be run as a no-build base-source control.` |
The isolated-checkout helper errors were:
/usr/bin/python3: can't open file '/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-14839-d6f1eccfe2a3/head-d6f1eccfe2a3-966d71d1/tools/pr_abi_audit.py': [Errno 2] No such file or directory
/usr/bin/python3: can't open file '/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-14839-d6f1eccfe2a3/head-d6f1eccfe2a3-966d71d1/tools/pr_test_quality.py': [Errno 2] No such file or directory
The workspace ABI audit fallback printed:
status: not_applicable
symbols: 0
findings: 0
The full source-reference scan returned no matches (reference_scan_exit=1); git diff --check returned diff_check_exit=0.
gh pr checks 14839 --repo boykopovar/AnyPS5:
windows pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/41341594027/job/124952622943
linux skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41341594027/job/124952624585
register-context skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41341594027/job/124952623916
relinker skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41341594027/job/124952624429
relinker-asan skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41341594027/job/124952623975
write-watch skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41341594027/job/124952624213
conventions pass 16s https://github.com/boykopovar/AnyPS5/actions/runs/41341593972/job/124952585995
eligibility pass 7s https://github.com/boykopovar/AnyPS5/actions/runs/41341594027/job/124952585537
intake pass 4s https://github.com/boykopovar/AnyPS5/actions/runs/41341593954/job/124952586639
labels pass 9s https://github.com/boykopovar/AnyPS5/actions/runs/41341593995/job/124952586816
library-portability pass 1m42s https://github.com/boykopovar/AnyPS5/actions/runs/41341593908/job/124952584939
report pass 1m25s https://github.com/boykopovar/AnyPS5/actions/runs/41341594061/job/124952585584
Negative control
| Assertion | Change to code under test | Did the new test fail? |
|---|---|---|
| Device creation no longer crashes on the affected NVIDIA path | No local mutation or GPU execution; the PR reports the revert passes the affected tests. | Could not be exercised locally. |
Findings
Blocking
None found in the revert diff. It removes the vkGetPipelineKeyKHR pipeline-binary path, its device setup, tests, registrations and documentation together. The revert also restores the source-version-scoped driver cache, so that cache is no longer retained across AnyPS5 updates; this is consistent with the stated full rollback.
Non-blocking
The repository requires AGC changes to include comparable validation-layer runs on main and the change, with no missing-layer message or new VUIDs. The PR reports a 4090 run and says affected tests crash on main, but I could not verify the required exact commands and logs from the PR description. The Windows hosted job is also pending, so approval is withheld.
Sources
- Removed pipeline-binary initialization and normal pipeline creation at the reviewed head
- Reverted global pipeline-key call at the merged #13931 commit
- AGC validation-layer test requirement at the PR base
- Maintainer’s NVIDIA crash report on #13931
Not verified
No local C++ build, CTest run, NVIDIA or other GPU run, console behavior, or validation-layer log comparison was performed. The PR’s hardware test results are author-reported. The hosted Windows job was pending; Linux and specialized jobs were skipped.
To get this to a good state
Please add the focused ctest -V results and validation logs for the affected tests on main and this head, including the requested-layer and VUID checks, or explain why a matched baseline cannot complete because it crashes. Once the Windows job finishes and that evidence is available, I will recheck the exact head and the same requirements.
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
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.
|
What
Reverts #13931 (merge d8b60b4).
With the default settings (shader cache enabled), every Vulkan device creation on our NVIDIA test machine segfaults in the
PipelineBinaryStoreconstructor, right aftervkGetPipelineKeyKHRfor the global key (PipelineBinaryStore.cpp:127) returnsVK_SUCCESS. Tenagc_drivertests that set their own environment withoutANYPS5_NO_SHADER_CACHEreach that path and crash on main, for exampleagc_driver_small_copy_syncand the twoagc_driver_address_passestests. CI did not catch it because it runs on lavapipe and most AGC tests disable the shader cache.APS5_NO_PIPELINE_BINARIES=1avoids the crash.Whether the cause is the driver or how the call is made is not established yet. This is a plain revert so main is green on NVIDIA again; @liskeee, the work is welcome back in a new PR, and we're happy to run a candidate on the same machine. Details follow on #13931.
Tested
CachyOS (Linux 7.2.3), NVIDIA GeForce RTX 4090, driver 610.57.04, default shader cache, no
APS5_NO_*variables. Tree identical to this branch (main f2d874c plus the #13879 revert from #14823 and this revert):agc_drivertests: 10/10 passagc_driver_dcc_keys_first_write: 2/2 passagc_driversuite: 841/841 passChecklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository