Repository navigation
perf(shaders): reuse each thread's translation pools - #2388
Conversation
Every translated stage built three new ObjectPools, and the default block chunk of 8192 IR::Blocks committed 68 MB a stage before faulting in fresh pages. Each thread now keeps its finished translations' pools, emptied, for its next ones, with small block chunks. The SPIR-V is unchanged. tests/test_translation_pools.py checks the reuse and that no pool commits a large chunk. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hckrman101
left a comment
There was a problem hiding this comment.
Thanks, this is a clean change and the write-up is very good. I went through it with object_pool.h open next to it.
On state leaking between translations: I can't find a way for it to happen. Create() always runs construct_at, so every Inst and Block starts out freshly constructed even when it sits in reused storage. ReleaseContents() runs the same Chunk::Release() that ~Chunk ran before, and the destructor keeps the old teardown order (program, then flow_blocks, blocks, insts). Nothing else in the pool carries over. After a release used_objects is 0 and node points back at the root chunk. The one thing that does change is where the IR objects land in memory, and those addresses were never stable anyway (malloc, ASLR). If some pass depended on them, the output would already differ from run to run. So I agree the SPIR-V should be byte-identical. The 24 synthetic programs show that for real Maxwell code, but they aren't game shaders. If you have a dump, a local before/after of the SPIR-V hashes over a real prepare would settle it: run with an empty cache, then post just the program count and the number of mismatches. Not blocking.
Lifetime: every Translation is a local of translate_into / translate_mesh_into and never leaves its thread. With the destructor declared, Translation has no implicit move, so a null pools can't sneak in. guarded_graphics's siglongjmp only jumps within its own frame, so no Translation is ever skipped. The workers that #2195's stall guard detaches still free their spare list when they exit. A worker stuck in the driver for good was already leaking its stack, so this adds nothing there. The prepare's joined workers free theirs as you describe.
Memory: see the inline note. Each spare set stays at the high-water mark of the largest stage its thread has translated. That's fine, but it's worth knowing for the replay thread and the vk_jobs workers.
Cache stamp: yes, one retranslation, as you say. Since the output is unchanged that only costs time. #2309 also edits the lines just below translate(), so whichever lands second will need a small rebase.
CI: the failing Python checks are test_vcore_fs_open.py / test_vcore_video_acquire.py (env_on undeclared), which this PR doesn't touch. The appimage job failed on DNS while fetching packages. The new test_translation_pools.py passes on Linux and Windows.
Approving. The inline notes are nits.
| pools->flow_blocks.ReleaseContents(); | ||
| pools->blocks.ReleaseContents(); | ||
| pools->insts.ReleaseContents(); | ||
| spare_pools().push_back(std::move(pools)); |
There was a problem hiding this comment.
Nit, on memory: when a translation fills the root chunk, ReleaseContents() squashes all of its chunks into one root of the combined size, and later translations never shrink it. So each spare set stays at the high-water mark of the largest stage its thread has seen, for the life of the thread (the replay thread and the detached vk_jobs workers live as long as the game). It's probably small for real shaders, but one huge stage would pin that memory for good. A cheap guard would be to keep only sets whose pools stay under a few MB, and simply drop the rest instead of pushing them back. Up to you.
There was a problem hiding this comment.
Done in 75412d5: the destructor estimates the stage's IR from the program (blocks x (IR::Block + Flow::Block) + instructions x IR::Inst) and, over 8 MB, frees the pools instead of keeping them. It is an estimate, since the pool can hold more than the live IR after dead code is removed, but it catches the one huge stage case. test_translation_pools.py checks that a translation over the limit does not keep its pools, and fails without the check.
| /// (each thread's pools of its finished translations, emptied, for its next ones: fresh pages were 20-30% of a stage's | ||
| /// translation time) | ||
| std::vector<std::unique_ptr<Pools>>& spare_pools() { | ||
| static thread_local std::vector<std::unique_ptr<Pools>> spare; |
There was a problem hiding this comment.
Nit: push_back in ~Translation could throw bad_alloc from a noexcept destructor. That only matters in theory, but spare.reserve(3) the first time through (task + mesh + fs is the most that are alive at once) would make it impossible.
There was a problem hiding this comment.
Done in 75412d5: the thread's spare list reserves 3 when it is made, so the push in the destructor cannot allocate. The test checks the capacity.
A released pool keeps the size of the largest stage it held, so one very large stage pinned that memory for the life of the replay thread and the background workers. A translation whose IR was over 8 MB now frees its pools instead of keeping them. The spare list reserves room for the three translations alive at once, so the destructor does not allocate. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every translated shader stage builds three new IR pools, and the default block chunk (8192 IR::Blocks of about 8 KB) commits 68 MB a stage and then faults in fresh pages. Reusing each thread's emptied pools, with small block chunks, cuts translation CPU by 20 to 30% and peak commit by about 6x, with the same SPIR-V.
What changed
gpu/vk/vk_shader.cpp:struct Pools:insts{8192},blocks{32},flow_blocks{32}.spare_pools()is a function-localthread_locallist of emptied pools.Translationholds aunique_ptr<Pools>, taken from the thread's list or made new. Its destructor clears the program, then callsReleaseContentson flow_blocks, blocks and insts (the order the members were destroyed before), and puts the pools back.translate()readst->pools->...and is otherwise unchanged.Translations alive at the same time (vs + fs, task + mesh + fs) each get their own set, so a thread keeps at most 3. Nothing is shared between threads.
Translationobjects are locals of the translating functions, so they are always destroyed on their own thread.Measurements
Synthetic Maxwell fragment programs (24 programs, 60 to 3200 instructions, varied branching and textures), built with llvm-mingw clang 23 from the v2.0.14 builder at
-O2 -march=x86-64-v3 -ffp-contract=offagainst the repo's Eden and sirit sources. Each iteration keeps two translations alive at once, then emits SPIR-V for both. 9 interleaved repetitions, medians [min-max]:Reuse with the default chunk sizes is as fast but does not lower peak commit, and leaves about 140 MB committed per idle thread. Smaller chunks without reuse lower commit but are not faster. Both parts are needed.
This speeds up the translation step only. In a prepare the driver's pipeline compiles dominate, and the whole-prepare effect was not measured on game data.
How it was tested
tests/test_translation_pools.py(stdlib Python and a C++ fixture): it cutsPools,spare_poolsandTranslationout ofvk_shader.cppand compiles them with Eden's realobject_pool.hand stand-in IR types of the real sizes. It checks that no allocation is over 4 MB, that the next translation on a thread reuses the pools without a new large allocation, that two live translations get different pools, that another thread gets its own, and that every IR object is destroyed. It fails onmain, fails with the default block chunk, and fails without the push back to the list. It is skipped without a C++ compiler.vk_shader.cppcompiles with llvm-mingw clang 23 at the build's flags.python -I -B tests/run_all.pyon Windows, with and without llvm-mingw onPATH: the same failing set as untouchedmainat 32693f0, none new.Notes
vk_shader.cppis part of the shader stamp, so like any edit to it this makes caches retranslate once.vk_shader.cppbut notTranslationortranslate(): its diff applies cleanly on top of this change.