fix(vk): prepare again when the shader seed changes #1387

Merged

hckrman101 merged 2 commits from prepare-seed-stamp into main

2026-11-10 04:25:46 +00:00

hckrman101 commented

2026-11-10 00:08:50 +00:00

(Migrated from github.com)

What was wrong

prepare_run (the "Preparing shaders" start-up step and hookshot --prepare-shaders) decided whether to prepare again from the translator's stamp alone: hookshot_prepared.bin held program_cache_stamp() and nothing else. The seed, hookshot_shaders.bin, is the list of programs to prepare, and it can change while the translator stays the same. When an update grew the seed, a folder that had already been prepared never prepared the new programs. Each one was then translated in play the first time it was used, at 200-400 ms apiece.

Evidence

4221050d ("nx2: the shader census reads agl's archives, networks, the movie upscaler and the particle sets") added compute programs to the seed and left the translator as it was. Folders prepared before that commit don't have those programs in the program cache, and nothing prepares them.

The fix

hookshot_prepared.bin now holds five words: the translator stamp, a hash of the seed's contents, the seed's size and write time, and when the record was written.
The seed is hashed only when its size or write time differs from the recorded values. While they match, the check is a stat (a few µs) and nothing is read. On Windows the write time comes from the file's FILETIME (100 ns), because libstdc++'s std::filesystem there gives whole seconds. As git does for its index, the shortcut is only trusted for a seed written more than 2 s before the record. A seed written closer to it is hashed, so a same-size rewrite within the time's resolution is still caught. A record without that time (four words) gets one hash. Hashing the 514 MB seed takes about 0.1 s from the page cache and 0.3-0.6 s cold; a byte-at-a-time FNV took 0.45 s from memory.
If a build rewrites the seed with the same bytes, the hash still matches. Nothing is prepared, and the record takes the new write time, so the next start doesn't hash the seed again.
When the seed changed and the translator didn't, one line is logged: vk: the game's shader list (hookshot_shaders.bin) is not the one the shaders were prepared from: preparing again (only the programs not kept yet).
The prepare was already incremental. seed_translate skips any key the program cache keeps, and seed_compute skips any variant it keeps. So "prepare again" only makes the programs that are new, and the driver warm-up runs only for those.
A record written before this change (the stamp alone) triggers one more prepare. Where nothing is missing it makes nothing new.
If there is no seed, only the translator stamp is compared, as before. prepare_is_update (RX_VK_PREPARE_BACKGROUND) still reads only the first word, so it works the same.

How it was tested

New tests/test_prepare_seed_stamp.py (standard library; it compiles the record helpers taken from vk_draw.cpp and runs them on files in a temporary folder):
- same seed and same translator: no prepare and no new hash;
- grown seed: prepares again;
- one byte changed (in the middle and in the last partial block): prepares again;
- changed translator: prepares again;
- seed rewritten with the same bytes: hashed once, no prepare, and not hashed on the next start;
- old 8-byte record: prepares once;
- four-word record: hashed once, then the shortcut;
- seed written in the same tick as the record, then rewritten with the same size and the very same time: still prepares again;
- no seed: the translator alone decides.
- The test sets every write time itself, so it doesn't depend on timing or on the file system's time resolution.
- It also checks that prepare_run uses all of this. Replacing the seed comparison with "never changed" makes it fail.
tests/test_startup_progress.py now looks for the new write (prepared_write) after the stop branch.
python3 -I -B tests/run_all.py: everything passes except tests/test_program_cache_cleanup.py, which fails the same way on main without this change (FAIL:115 disk_in).
Linux build. The Windows hookshot_vk.dll cross-builds with llvm-mingw.
hookshot --prepare-shaders on a new folder (RTX 3090, 24 threads). The "older" seed was the same seed with its last 40 programs removed:
1. Older seed, empty caches: full prepare, 38769 programs in 252 s.
2. Same seed again: no prepare and no hash, just the precompile.
3. Full seed in place (40 more programs): the seed-change line is logged, then prepared 38809 shader programs (40 new, ...) in 10.0 s.
4. Same full seed again: no prepare.
5. Seed touched (same bytes): hashed in 234 ms, no prepare.
6. Record cut back to the old 8-byte form: one prepare, 0 new in 10.0 s, and the record is rewritten in the new form.

## What was wrong `prepare_run` (the "Preparing shaders" start-up step and `hookshot --prepare-shaders`) decided whether to prepare again from the translator's stamp alone: `hookshot_prepared.bin` held `program_cache_stamp()` and nothing else. The seed, `hookshot_shaders.bin`, is the list of programs to prepare, and it can change while the translator stays the same. When an update grew the seed, a folder that had already been prepared never prepared the new programs. Each one was then translated in play the first time it was used, at 200-400 ms apiece. ## Evidence 4221050d ("nx2: the shader census reads agl's archives, networks, the movie upscaler and the particle sets") added compute programs to the seed and left the translator as it was. Folders prepared before that commit don't have those programs in the program cache, and nothing prepares them. ## The fix - `hookshot_prepared.bin` now holds five words: the translator stamp, a hash of the seed's contents, the seed's size and write time, and when the record was written. - The seed is hashed only when its size or write time differs from the recorded values. While they match, the check is a stat (a few µs) and nothing is read. On Windows the write time comes from the file's FILETIME (100 ns), because libstdc++'s `std::filesystem` there gives whole seconds. As git does for its index, the shortcut is only trusted for a seed written more than 2 s before the record. A seed written closer to it is hashed, so a same-size rewrite within the time's resolution is still caught. A record without that time (four words) gets one hash. Hashing the 514 MB seed takes about 0.1 s from the page cache and 0.3-0.6 s cold; a byte-at-a-time FNV took 0.45 s from memory. - If a build rewrites the seed with the same bytes, the hash still matches. Nothing is prepared, and the record takes the new write time, so the next start doesn't hash the seed again. - When the seed changed and the translator didn't, one line is logged: `vk: the game's shader list (hookshot_shaders.bin) is not the one the shaders were prepared from: preparing again (only the programs not kept yet)`. - The prepare was already incremental. `seed_translate` skips any key the program cache keeps, and `seed_compute` skips any variant it keeps. So "prepare again" only makes the programs that are new, and the driver warm-up runs only for those. - A record written before this change (the stamp alone) triggers one more prepare. Where nothing is missing it makes nothing new. - If there is no seed, only the translator stamp is compared, as before. `prepare_is_update` (`RX_VK_PREPARE_BACKGROUND`) still reads only the first word, so it works the same. ## How it was tested - New `tests/test_prepare_seed_stamp.py` (standard library; it compiles the record helpers taken from `vk_draw.cpp` and runs them on files in a temporary folder): - same seed and same translator: no prepare and no new hash; - grown seed: prepares again; - one byte changed (in the middle and in the last partial block): prepares again; - changed translator: prepares again; - seed rewritten with the same bytes: hashed once, no prepare, and not hashed on the next start; - old 8-byte record: prepares once; - four-word record: hashed once, then the shortcut; - seed written in the same tick as the record, then rewritten with the same size and the very same time: still prepares again; - no seed: the translator alone decides. - The test sets every write time itself, so it doesn't depend on timing or on the file system's time resolution. - It also checks that `prepare_run` uses all of this. Replacing the seed comparison with "never changed" makes it fail. - `tests/test_startup_progress.py` now looks for the new write (`prepared_write`) after the stop branch. - `python3 -I -B tests/run_all.py`: everything passes except `tests/test_program_cache_cleanup.py`, which fails the same way on main without this change (`FAIL:115 disk_in`). - Linux build. The Windows `hookshot_vk.dll` cross-builds with llvm-mingw. - `hookshot --prepare-shaders` on a new folder (RTX 3090, 24 threads). The "older" seed was the same seed with its last 40 programs removed: 1. Older seed, empty caches: full prepare, 38769 programs in 252 s. 2. Same seed again: no prepare and no hash, just the precompile. 3. Full seed in place (40 more programs): the seed-change line is logged, then `prepared 38809 shader programs (40 new, ...) in 10.0 s`. 4. Same full seed again: no prepare. 5. Seed touched (same bytes): hashed in 234 ms, no prepare. 6. Record cut back to the old 8-byte form: one prepare, `0 new` in 10.0 s, and the record is rewritten in the new form.

NotRexed commented

2026-11-10 02:24:42 +00:00

(Migrated from github.com)

This landed in the v2.0.12 branch (merged there for testing; it reaches main with that release), so closing. Thanks!

This landed in the v2.0.12 branch (merged there for testing; it reaches `main` with that release), so closing. Thanks!

NotRexed commented

2026-11-10 03:54:42 +00:00

(Migrated from github.com)

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.

klimovich008 commented

2026-11-10 04:00:38 +00:00

(Migrated from github.com)

Windows FILETIME follow-up on the inspected PR code: both seed_file_id and file_clock_now overflow signed 64-bit arithmetic when converting today's FILETIME values to nanoseconds. The ticks fit; multiplying by 100 does not.

Clang 23.1.5 with signed-overflow UBSan reports both conversions. The proposed change below keeps native 100 ns ticks on Windows and scales the two-second slack accordingly; other platforms keep nanoseconds. Actual helper/record tests then have no sanitizer diagnostics. A synthetic old five-word record hashes once, rewrites its times and skips hashing on the next check.

The optimized old code happened to preserve the comparison in my CPU test. I have not reproduced a wrong preparation decision or a game stutter caused by this overflow.

Minimal follow-up patch

--- a/gpu/vk/vk_draw.cpp
+++ b/gpu/vk/vk_draw.cpp
@@ -1,18 +1,22 @@
 struct PreparedFor {
     uint64_t stamp = 0, seed_hash = 0, seed_size = 0;
-    int64_t seed_time = 0, recorded_at = 0;   // (nanoseconds, of the file system's clock: libstdc++'s counts from 2174, below 0)
+    int64_t seed_time = 0, recorded_at = 0;   // (Windows: FILETIME100ns ticks; elsewhere: filesystem-clock nanoseconds)
     bool timed = false;   // (recorded_at read: records of four words had none)
 };
+#ifdef _WIN32
+constexpr int64_t kSeedTimeSlack = 20'000'000;   // two seconds in FILETIME ticks
+#else
 constexpr int64_t kSeedTimeSlack = 2'000'000'000;
+#endif
 enum class PrepareWhy { None, First, Translator, Seed };
-/// The seed's size and write time, in nanoseconds (false: there is none). On Windows from the file's FILETIME (100 ns): what
-/// std::filesystem gives there can be whole seconds
+/// The seed's size and write time (false: there is none). Windows uses native100ns FILETIME ticks; other platforms use
+/// filesystem-clock nanoseconds. std::filesystem on Windows can give only whole seconds
 bool seed_file_id(const std::string& path, uint64_t* size, int64_t* time) {
 #ifdef _WIN32
     WIN32_FILE_ATTRIBUTE_DATA a;
     if (!GetFileAttributesExA(path.c_str(), GetFileExInfoStandard, &a) || (a.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) return false;
     *size = (uint64_t)a.nFileSizeHigh << 32 | a.nFileSizeLow;
-    *time = (int64_t)((uint64_t)a.ftLastWriteTime.dwHighDateTime << 32 | a.ftLastWriteTime.dwLowDateTime) * 100;
+    *time = (int64_t)((uint64_t)a.ftLastWriteTime.dwHighDateTime << 32 | a.ftLastWriteTime.dwLowDateTime);
 #else
     std::error_code ec;
     const auto fsize = std::filesystem::file_size(path, ec);
@@ -29,7 +33,7 @@
 #ifdef _WIN32
     FILETIME t;
     GetSystemTimeAsFileTime(&t);
-    return (int64_t)((uint64_t)t.dwHighDateTime << 32 | t.dwLowDateTime) * 100;
+    return (int64_t)((uint64_t)t.dwHighDateTime << 32 | t.dwLowDateTime);
 #else
     return (int64_t)std::chrono::duration_cast<std::chrono::nanoseconds>(
                std::filesystem::file_time_type::clock::now().time_since_epoch()).count();

This applies to the inspected PR head; the release branch mentioned above is not published in the repository's branch list. Both Windows helpers and the slack must change together.

Windows FILETIME follow-up on the inspected PR code: both `seed_file_id` and `file_clock_now` overflow signed 64-bit arithmetic when converting today's FILETIME values to nanoseconds. The ticks fit; multiplying by 100 does not. Clang 23.1.5 with signed-overflow UBSan reports both conversions. The proposed change below keeps native 100 ns ticks on Windows and scales the two-second slack accordingly; other platforms keep nanoseconds. Actual helper/record tests then have no sanitizer diagnostics. A synthetic old five-word record hashes once, rewrites its times and skips hashing on the next check. The optimized old code happened to preserve the comparison in my CPU test. I have not reproduced a wrong preparation decision or a game stutter caused by this overflow. <details> <summary>Minimal follow-up patch</summary> ```diff --- a/gpu/vk/vk_draw.cpp +++ b/gpu/vk/vk_draw.cpp @@ -1,18 +1,22 @@ struct PreparedFor { uint64_t stamp = 0, seed_hash = 0, seed_size = 0; - int64_t seed_time = 0, recorded_at = 0; // (nanoseconds, of the file system's clock: libstdc++'s counts from 2174, below 0) + int64_t seed_time = 0, recorded_at = 0; // (Windows: FILETIME100ns ticks; elsewhere: filesystem-clock nanoseconds) bool timed = false; // (recorded_at read: records of four words had none) }; +#ifdef _WIN32 +constexpr int64_t kSeedTimeSlack = 20'000'000; // two seconds in FILETIME ticks +#else constexpr int64_t kSeedTimeSlack = 2'000'000'000; +#endif enum class PrepareWhy { None, First, Translator, Seed }; -/// The seed's size and write time, in nanoseconds (false: there is none). On Windows from the file's FILETIME (100 ns): what -/// std::filesystem gives there can be whole seconds +/// The seed's size and write time (false: there is none). Windows uses native100ns FILETIME ticks; other platforms use +/// filesystem-clock nanoseconds. std::filesystem on Windows can give only whole seconds bool seed_file_id(const std::string& path, uint64_t* size, int64_t* time) { #ifdef _WIN32 WIN32_FILE_ATTRIBUTE_DATA a; if (!GetFileAttributesExA(path.c_str(), GetFileExInfoStandard, &a) || (a.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) return false; *size = (uint64_t)a.nFileSizeHigh << 32 | a.nFileSizeLow; - *time = (int64_t)((uint64_t)a.ftLastWriteTime.dwHighDateTime << 32 | a.ftLastWriteTime.dwLowDateTime) * 100; + *time = (int64_t)((uint64_t)a.ftLastWriteTime.dwHighDateTime << 32 | a.ftLastWriteTime.dwLowDateTime); #else std::error_code ec; const auto fsize = std::filesystem::file_size(path, ec); @@ -29,7 +33,7 @@ #ifdef _WIN32 FILETIME t; GetSystemTimeAsFileTime(&t); - return (int64_t)((uint64_t)t.dwHighDateTime << 32 | t.dwLowDateTime) * 100; + return (int64_t)((uint64_t)t.dwHighDateTime << 32 | t.dwLowDateTime); #else return (int64_t)std::chrono::duration_cast<std::chrono::nanoseconds>( std::filesystem::file_time_type::clock::now().time_since_epoch()).count(); ``` </details> This applies to the inspected PR head; the release branch mentioned above is not published in the repository's branch list. Both Windows helpers and the slack must change together.

Commenting is not possible because the repository is archived.

No reviewers