Repository navigation
perf(build): a renderer header edit compiles the files that read it, not the whole back end - #2706
jordanella wants to merge 2 commits into
Conversation
…not the whole back end The back end's own files (gpu/vk/*.cpp) depended on every header under gpu/vk, gpu/render, gpu/compat and suyu's texture decoders: on Windows a comment in vk_present_rect.h, which two files include, compiled 35 files again (417 s of compiler time, a 44 s build against 26 s for a .cpp edit). This is the dev-loop proposal's A6 (NotRexed#2636). They are compiled with -MD now, and clang's depfile beside each object (vk_x.d) says which headers it read. The next build keys the object by those of them under the same folders, their names and contents (renderer_deps); a file with no depfile yet, or one that names nothing there, keeps the folders' blob. flag_text leaves -MD out, so no key changes and nothing compiles again for the change itself: depfiles come as files are compiled. An object compiled now is cached and stamped under the key its new depfile gives as well (rekey_depfiles), so the next build does not compile it once more. Eden's and sirit's files keep their own headers' blob; the host, its blob. tests/test_renderer_depfiles.py is new (the depfile as clang writes it, the keys, the re-keying, the wiring); test_dlss_sdk_cache.py and test_shader_stamp_cache.py run the job loop with its new helpers, and still take an older build.py. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Thanks: that's a real hole, and your steps reproduce it exactly. Fixed in f0f345b:
On running the game: with this branch's
|
…depfile's key only hckrman101 on NotRexed#2706: compile_one cached every object under its planned key, and for a back-end file that key came from the previous compile's depfile, the new source hashed with the old include list. Add an include, take it out, edit the header, put the include back, and that key named an object built with the old header: a cache hit, and a depfile that was never rewritten. compile_one now stamps such an object DEPFILE_PENDING and caches nothing; rekey_depfiles stamps it and caches it by the key its own new depfile gives, also when that is the planned key. Only sound keys reach the cache: a blob key covers every header, a depfile key the headers that compile read. A build that stops before the re-keying leaves the object pending, and the next build compiles it again. tests/test_renderer_depfiles.py walks the four steps with a stand-in compiler (it fails on the previous commit: step 4 got step 3's object); test_compile_retry_jobs.py takes the constants. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Joey-Magnus
left a comment
There was a problem hiding this comment.
hckrman101's stale-object hole is closed in f0f345b. compile_one stamps an object it compiled under a depfile key depfile-pending and doesn't cache it (nx2/build.py:952). rekey_depfiles then caches and stamps it under the key from its own new depfile, so the cache only holds blob keys or a compile's own depfile key. I traced the cache-hit path (compile_one restoring from OBJ_CACHE). A hit needs the planned key to equal a key computed from that object's own depfile, so the source and the set of hashed headers match, and leaving the old .d on disk is sound. A build that fails before rekey_depfiles leaves pending stamps, which never match, so those files compile again. I merged this into current main and ran the depfile, compile, mem_gate, dlss_sdk, shader_stamp and build checks: 31 passed. test_precompile_compute_guard and test_portable_local_builder also fail on main on this machine, so they're not from this PR. The in-game run after the header edit is what was asked for. The red appimage job timed out in the PPA apt step, which is infrastructure, so a re-run should clear it.
|
Linux result on Setup: Arch Linux, kernel 7.2.15, i9-13900K, RTX 4090, NVIDIA 615.78.08, Clang 23.1.3 release builds with the same PGO profile and compatible translated inputs. Separate checkouts and game folders. After the initial builds, I added the same harmless comment to
The five were The stale-cache review fix also passes with real compilers. A separate check drives the actual cache/key/compile/rekey functions, adds an include, removes it, edits that header, and adds it back. It links and runs each resulting object. GCC 16.2 and Clang 23.1.3 both return the new value 50 at step 4 on Game check after the header edit: windowless, fresh copied save, 2048×1152 game output / 1920×1080 offscreen presentation, 60 cap, DLSS/frame generation off. Base: 37.9 seconds, 1,505 game-present rows, exit 0. PR: 34.3 seconds, 1,454 rows, exit 0. Both show Link, the forest and HUD; neither logs a crash, lost device or kernel driver fault. Load averages were 36.37→34.31 and 13.85→13.41, respectively: these are not frame-rate comparisons. The experimental comments were removed afterward; the retained tested binaries are explicitly the post-comment builds. All six changed checks pass in both compiler environments. Against base source, the new depfile check reports one flag-key failure and ten missing-helper errors; the compile-failure/memory-gate adaptations reject missing constants. The existing compile-retry, DLSS-cache and shader-stamp checks pass on base too, as compatibility coverage. Full suite on the head: 473 passed, 0 failed, 20 skipped. The hosted AppImage job separately timed out in System packages, exit 124, before compiling this change: https://github.com/NotRexed/Hookshot/actions/runs/43211349705/job/131159467435. Not tested: Windows runtime, all possible include-path shadowing changes, other game scenes, or quiet repeated build timings. No new defect found in the exercised head. |

Editing any renderer header recompiles the whole back end: on Windows a comment in
vk_present_rect.h, which two files include, compiled 35 files again (417 s of compiler time; a 44 s build against 26 s for a.cppedit, and minutes on a 4–8 thread PC). This is the dev-loop proposal's A6 (#2636), validated by hckrman101's Linux numbers there (16 objects for one header).Change
gpu/vk/*.cpp) are compiled with-MD, and clang writes a depfile beside each object (vk_x.d).gpu/vk,gpu/render,gpu/compat, suyu's texture decoders), by name and content (renderer_deps). A header the file doesn't read no longer changes its key; one it reads, or one that's gone, does.flag_textleaves-MDout, so no key changes and nothing compiles again for this change itself. Depfiles appear as files are compiled.rekey_depfiles), so the next build doesn't compile it once more. Only an object of this build, so made from this build's source and headers, is re-keyed.compile_onestamps itdepfile-pending, andrekey_depfilesstamps and caches it by its own depfile's key, also when that's the planned key. A build that stops before re-keying leaves it pending, and the next build compiles it again. (hckrman101's review: an include added, removed, the header edited, the include back, used to get the first object back from the cache.)Measured
On Windows (Ryzen 9 3900X, NVMe):
vk_present_rect.h, beforeThe four are the files that read it:
vk_framegen.cpp,vk_menu.cpp,vk_replay.cpp, andhost/hos_main.c, through the host's own blob. Getting depfiles took one compile of each back-end file, done here by commenting them all. The re-keying then kept the reverted files from compiling a third time.In the game
Windows 11, RTX 3080 (NVIDIA 621.14), from this branch's
build.py(with the review fix) onmainee29dd5: a build with depfiles, then one with a comment added tovk_present_rect.h(5 files compiled: the four above, andvk_hangdump.cpp, which reads no repo header, so its depfile names nothing under those folders and it keeps the blob; no object left pending; with the comment taken out again, 0 files). That game, launched headless through the control bridge from a save: in play in Kokiri Forest, 27–34 fps over five samples, nothing in the log about a lost device or a crash.Testing
tests/test_renderer_depfiles.py(new, 11 cases):/and\in paths with spaces, CRLF continuations), plus$$and\#;-MDis in no key;b.h.It fails against
main'sbuild.py.test_compiler_failure.pyandtest_mem_gate_leases.pytakeDEPFILE_DEPSandDEPFILE_PENDINGfrombuild.pywithcompile_one.test_dlss_sdk_cache.pyandtest_shader_stamp_cache.pyrun the job loop with its new helpers. Both still take an olderbuild.py(--build-script), andtest_shader_stamp_cachepasses againstmain's.Every check that reads
build.py(39), on Windows, passes apart fromtest_driver_cache_folder,test_fpx_stickyandtest_linux_update, which fail the same way onmainhere.Not tested: Linux. gcc and clang write the same make-style depfile.
🤖 Generated with Claude Code