Repository navigation
Conversation
b114d01 to
0508e81
Compare
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the work. I'm requesting changes because, as the description says, the runtime benefit hasn't been shown yet, and the PR is too big to merge on trust.
- Measure the hit rate with a static pack. The 10/10 hits come from replaying requests captured from the emulator, which is just a copy of an existing cache. What this tool claims is that variants built from the archive hit live requests, and that isn't measured. Keys include target, binding layout and stage configuration, so guessed variants can miss every time. Please post
[shader-prebuilt]hits/misses from a real run with a pack built bybuild --paksonly, along with load time with and without the pack. Without that number we can't merge 4k lines. - Say what it saves. Since #639 the RDNA → SPIR-V translation already runs at registration. First-draw cost is specialization plus the driver pipeline compile, and this PR doesn't touch either (
pipeline-cache.binisn't loaded). Please state the expected saving at load time, measured. - Split it. The two recompiler commits (
CompileShaderUncached, the encoded entry-key helpers) go in their own PR, with tests. The tool goes in a follow-up that depends on it. Mesh shader-object binaries,pipeline-cache.binand theAPS5_PIPELINE_SALTbenchmarking path aren't used by the emulator, so leave them out until something consumes them. - Oodle decoder provenance. The description says groups were verified byte-identical "to the licensed-SDK reference", while TechnicalDebt says "against the previous decoder". Which one was it? Please clarify the provenance in the PR. A decoder for a proprietary codec that was checked against the licensed SDK needs a clear statement that no SDK code or headers were used.
- Docs reference things that aren't in the repo.
docs/dev/ShaderArchive.mdmentions "the Rust extractor", itsshaders.jsonl/groups/*.binand "the Rust tool's static-target profile". None of these are in the tree. Please drop them or point to what users are supposed to run. - Linux. Only tested on Windows. CI doesn't run the tool on a real archive, so please run
shader_archiveand one extract/build on Linux, or say it's untested there in TechnicalDebt. - Conflicts.
ShaderDiskCache.hppoverlaps with #5509 and #6243 (see the overlap comment). Rebase once those settle.
CompileShaderUncached compiles one request without the process-lifetime source cache or a resource snapshot, so batch tools can compile shader archives with bounded worker lifetimes. Optional frontend and backend timings report where each compilation spent its time.
EntryKey returns the validated key stored in an encoded cache entry and IsCurrentEntryHeader checks that a file header belongs to this compiler source and format, so offline tooling can group and screen existing cache files without decoding them. Both validate against the aligned payload layout.
…lt packs agc_shader_archive extracts native shader programs from IoStore containers, builds static AGC requests, compiles them through the production recompiler with bounded workers, validates the SPIR-V, can compile pipelines on a Vulkan device, and publishes the results as prebuilt packs for the runtime loader. Captured requests can be replayed offline with their actual targets, and probe reports exact pack hit coverage and load timings. Oodle Mermaid streams are decoded by a new decoder in Oodle.cpp whose bitstream layout follows the MIT-licensed oozextract 0.5.5 decoder; unsupported codecs and entropy forms throw and are recorded in technical debt. The target is excluded from the default build. Extraction of one measured title reproduces all 4,803 shader groups byte-for-byte. Replaying eleven captured requests publishes an eleven-entry pack, and probe then loads eleven of eleven from the pack with an empty writable cache. Depends on the uncached compile entry point and the prebuilt pack loader commits.
New Markdown outside docs/ is rejected by the conventions check, so the tool guide lives with the other developer documents.
0508e81 to
99dcb19
Compare
|
Thanks for the thorough review. This push covers the mechanical points; the measurement and the split are the next round and I am not claiming they are done. Done in this push:
Still open, in your order:
|
|
Split as requested: the two recompiler commits (uncached compile entry point, encoded entry-key helpers) now have their own PR with tests (linked below once opened). This tool PR will return as the follow-up on top of it, with the real-run hit-rate measurement. Closing this one because of the one-open-PR limit; the branch |
|
The split recompiler PR is #6296. |
|
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.
|
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the update. Checking this head against the points of the previous review:
- Hit rate not measured (previous point 1). The description still says the real-game hit-rate measurement with a statically built pack is open. The 10/10 hits are replayed captured requests, which don't show that keys built from the archive match live ones. Please post
[shader-prebuilt]hits/misses from a run with a pack built bybuild --paksonly, with load time with and without the pack. - Saving not stated (point 2). Depends on the above. Without a measured gain, 4k lines of a proprietary-codec decoder plus extractor can't be merged on trust.
- Split only half done (point 3). The branch still carries
b280f00899andc5af57b355(the uncached compile entry point and the entry-key helpers), which are what #6296 adds. Once #6296 lands, drop them here and rebase so this PR contains only the tool; until then it duplicates and will conflict with it. - Linux untested (point 6). CI only builds the relinker for this PR, so
shader_archivehas not run on Linux; the Windows run is the author's. Please runshader_archiveon Linux or keep the "untested on Linux" note in TechnicalDebt accurate.
Happy to review again once #6296 is merged and the hit-rate number is posted.
What
An offline tool that extracts shader programs from a game's archives, recompiles them ahead of time, and publishes the results as a prebuilt shader pack for the loader in #5792 (merged).
tools/shader-archive(new): reads Unreal IoStore v8.utoc/.ucaspairs, decodes the Oodle Mermaid quanta those archives use with its own clean-room decoder (no vendored Oodle), scans shader libraries, and builds shader variants fully offline through the existing recompiler. Output is published atomically as a*.packfile (the format feat(recompiler): load prebuilt shader packs before the writable cache #5792 loads) plus a manifest.docs/dev/TechnicalDebt.md; the tool fails on archives it cannot decode instead of guessing.Scope, honestly: extraction covers Unreal IoStore v8 only today; other engines' archives (Insomniac, RAGE) need their own readers. Keys are exact, so statically built variants only hit when the runtime request matches exactly — the verified end-to-end numbers are for replayed captured requests, and a real-game hit-rate measurement with a statically built pack is still open (also in TechnicalDebt). The tool also writes
pipeline-cache.bin, but nothing loads it at runtime yet; whether offline pipeline compiles remove driver compile cost is unmeasured, so this PR claims no pipeline-compile saving.Tested
Windows 11: rebuilt on current main;
shader_archive,agc_shader_disk_cacheandshader_cache_file_readpass. Full real-archive run: 4,803/4,803 IoStore groups decoded byte-identical to the licensed-SDK reference, 80,794 shader hashes matching. Offline build + publish + probe against captured requests: 10/10 exact-key hits. Not tested on Linux locally (no Linux box here); CI covers it.Checklist
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 (attach them to this PR)