WIP: fix(build): the hash record is written under its build's start, and keeps only the keys used #35

Draft
dyst wants to merge 1 commit from dyst/Longshot:fix/hash-cache-clock into main
First-time contributor

Why

The build's hash record was written under the time it was saved, seconds after the sources were stat'd, which loosens the 2 s rule on coarse-timestamp filesystems; and the record only grew, by one key per renderer source for every header edit (hckrman101's two non-blocking points on NotRexed/Hookshot#2702).

What changed

  • build: load_hashes in nx2/build.py takes the time before any source is stat'd (HASHES_STARTED), and save_hashes writes that as written_ns, not the time of the save after the whole plan.
  • build: key_of and content_sha1 note, per source, the keys this build asked for (USED). save_hashes (its kept helper) keeps only those keys for each source this build looked at; a source it didn't look at, such as a renderer-only build's lifted C, keeps its record as it was.
  • build: nothing about the keys themselves changes (HASH_SCHEME stays 1), so no object is renamed and nothing compiles again for this change.
  • tests: two new cases in tests/test_build_hash_cache.py, test_the_record_is_written_under_the_time_its_build_started and test_a_source_this_build_looked_at_keeps_only_the_keys_it_asked_for. Both fail on main.

Flags and settings

None

Measurements

Measured on the GitHub version (NotRexed/Hookshot#2873), not yet on Longshot: this PR stays WIP until it is.

Hardware: Ryzen 9 3900X, Windows 11
Scene: none (a build measurement): a work folder of 874 sources
Method: four builds in a row: a comment added to gpu/vk/vk_present_rect.h, a second comment, both taken out again, then a build with nothing changed. The numbers are the hash record's total keys after each build.

Keys in the hash record GitHub main this PR (GitHub)
at the start 897 897
after the 1st edit 932 897
after the 2nd edit 967 897
after both were reverted 967 897
after a build with no change 967 897
  • "GitHub main" is GitHub's main with NotRexed/Hookshot#2702 in it; neither column's commit is named in the original description.
  • The record stays at one key per source per flag set, at most 3 on one source in both.
  • On this branch the two edits compiled 2 files each, not 35, because main's run had already cached objects for the same header contents.

In the game (a check, not a comparison): the game from this branch's last build, launched headless through the control bridge from a save, on RTX 3080 (NVIDIA 621.14), Ryzen 9 3900X, 64 GB, Windows 11. In play in Kokiri Forest, 27–32 fps over five samples, nothing in the log about a lost device or a crash.

Testing

Tested: the runs under Measurements (on the GitHub version) and the game-free checks this description names. Checked but not measured: the game on Windows (above): the change is build-only, and nothing in play was compared with main.

Not tested: Linux. Nothing here is platform-specific; the record's id is the inode there.

Needs other testers:

  • Linux: an update build after one source changed, expecting only that source read again and the others taken from the record (request to open)

Screenshots

Nothing on screen.

Risk and rollback

Only the build's hash record (nx2/build.py's HASHES, saved by save_hashes) changes. A key dropped from the record costs the next build that asks for it a read and hash of that source, not a wrong object; the earlier written_ns only makes the 2 s rule stricter. There is no switch: reverting the commit is enough, and the record's format is the same either way.

Reviewer notes

Carried over from NotRexed/Hookshot#2873 with the move to Longshot: the same change on Longshot's main, with its rename applied. Its review and discussion are there.

  • The checks and the game run above were done on the GitHub version; this branch is the same change with the rename applied (the same files and line counts). Forgejo runs no checks, and they were not run again here.
  • Checks run (on GitHub, not the whole run_all.py): every check that reads the changed files (35), through run_all.py on Windows with the Builder's Python and clang, rebased on GitHub's main of that day. 31 pass; four fail the same way on main there: test_driver_cache_folder, test_fpx_sticky, test_linux_update and nx2/test_dll.py (it needs a DLL build).
  • See NotRexed/Hookshot#2702 and NotRexed/Hookshot#2636 (A5).

Issues

None.

Checklist

For everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main.
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
  • Tested in the game, or what wasn't is named under Testing.
  • Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Flags table filled in; README / docs/development.md switch reference updated.
  • Screenshots for a change that shows on screen.
  • A row in docs/tested-configurations.md for each measured run.
  • MKWVK_API_VERSION bumped (gpu/vk/vk_api.h's MkwVkHost or MkwVkRenderer changed).
  • The SPIR-V headers regenerated with glslang (a shader changed).
  • A game-free test added for the fix, failing on main and passing here.
  • A check under tests/ that reads changed source updated in this PR, not weakened.
  • Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths.
  • Docs updated.

🤖 Generated with Claude Code

## Why The build's hash record was written under the time it was saved, seconds after the sources were stat'd, which loosens the 2 s rule on coarse-timestamp filesystems; and the record only grew, by one key per renderer source for every header edit (hckrman101's two non-blocking points on [NotRexed/Hookshot#2702](https://github.com/NotRexed/Hookshot/pull/2702)). ## What changed - build: `load_hashes` in `nx2/build.py` takes the time before any source is stat'd (`HASHES_STARTED`), and `save_hashes` writes that as `written_ns`, not the time of the save after the whole plan. - build: `key_of` and `content_sha1` note, per source, the keys this build asked for (`USED`). `save_hashes` (its `kept` helper) keeps only those keys for each source this build looked at; a source it didn't look at, such as a renderer-only build's lifted C, keeps its record as it was. - build: nothing about the keys themselves changes (`HASH_SCHEME` stays 1), so no object is renamed and nothing compiles again for this change. - tests: two new cases in `tests/test_build_hash_cache.py`, `test_the_record_is_written_under_the_time_its_build_started` and `test_a_source_this_build_looked_at_keeps_only_the_keys_it_asked_for`. Both fail on `main`. ## Flags and settings None ## Measurements Measured on the GitHub version (NotRexed/Hookshot#2873), not yet on Longshot: this PR stays WIP until it is. Hardware: Ryzen 9 3900X, Windows 11 Scene: none (a build measurement): a work folder of 874 sources Method: four builds in a row: a comment added to `gpu/vk/vk_present_rect.h`, a second comment, both taken out again, then a build with nothing changed. The numbers are the hash record's total keys after each build. | Keys in the hash record | GitHub main | this PR (GitHub) | |---|---|---| | at the start | 897 | 897 | | after the 1st edit | 932 | 897 | | after the 2nd edit | 967 | 897 | | after both were reverted | 967 | 897 | | after a build with no change | 967 | 897 | - "GitHub main" is GitHub's `main` with [NotRexed/Hookshot#2702](https://github.com/NotRexed/Hookshot/pull/2702) in it; neither column's commit is named in the original description. - The record stays at one key per source per flag set, at most 3 on one source in both. - On this branch the two edits compiled 2 files each, not 35, because `main`'s run had already cached objects for the same header contents. In the game (a check, not a comparison): the game from this branch's last build, launched headless through the control bridge from a save, on RTX 3080 (NVIDIA 621.14), Ryzen 9 3900X, 64 GB, Windows 11. In play in Kokiri Forest, 27–32 fps over five samples, nothing in the log about a lost device or a crash. ## Testing **Tested:** the runs under Measurements (on the GitHub version) and the game-free checks this description names. Checked but not measured: the game on Windows (above): the change is build-only, and nothing in play was compared with `main`. **Not tested:** Linux. Nothing here is platform-specific; the record's `id` is the inode there. **Needs other testers:** - Linux: an update build after one source changed, expecting only that source read again and the others taken from the record (request to open) ## Screenshots Nothing on screen. ## Risk and rollback Only the build's hash record (`nx2/build.py`'s `HASHES`, saved by `save_hashes`) changes. A key dropped from the record costs the next build that asks for it a read and hash of that source, not a wrong object; the earlier `written_ns` only makes the 2 s rule stricter. There is no switch: reverting the commit is enough, and the record's format is the same either way. ## Reviewer notes Carried over from [NotRexed/Hookshot#2873](https://github.com/NotRexed/Hookshot/pull/2873) with the move to Longshot: the same change on Longshot's `main`, with its rename applied. Its review and discussion are there. - The checks and the game run above were done on the GitHub version; this branch is the same change with the rename applied (the same files and line counts). Forgejo runs no checks, and they were not run again here. - Checks run (on GitHub, not the whole `run_all.py`): every check that reads the changed files (35), through `run_all.py` on Windows with the Builder's Python and clang, rebased on GitHub's `main` of that day. 31 pass; four fail the same way on `main` there: `test_driver_cache_folder`, `test_fpx_sticky`, `test_linux_update` and `nx2/test_dll.py` (it needs a DLL build). - See [NotRexed/Hookshot#2702](https://github.com/NotRexed/Hookshot/pull/2702) and [NotRexed/Hookshot#2636](https://github.com/NotRexed/Hookshot/pull/2636) (A5). ## Issues None. ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [ ] Rebased on current `main`. - [ ] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [ ] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [ ] Flags table filled in; README / docs/development.md switch reference updated. - [ ] Screenshots for a change that shows on screen. - [ ] A row in `docs/tested-configurations.md` for each measured run. - [ ] `MKWVK_API_VERSION` bumped (`gpu/vk/vk_api.h`'s `MkwVkHost` or `MkwVkRenderer` changed). - [ ] The SPIR-V headers regenerated with glslang (a shader changed). - [ ] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. - [ ] Docs updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
hckrman101's two points on #2702. The record was written under the time of its save, after the whole plan, seconds after
each source was stat'd; the 2 s "racily clean" rule is about the stat, so it is written under the time load_hashes ran,
before any. And it only grew: each header edit added a key to every renderer source. Each source this build looked at
now keeps only the keys this build asked for (key_of, content_sha1); a source it did not look at, such as a
renderer-only build's lifted C, keeps its record as it was.

tests/test_build_hash_cache.py: the record's time is the load's, and a source looked at keeps only its used keys while
one not looked at keeps all of its (both fail on main).

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
dyst changed title from fix(build): the hash record is written under its build's start, and keeps only the keys used to WIP: fix(build): the hash record is written under its build's start, and keeps only the keys used 2026-11-13 01:13:58 +00:00
Author
First-time contributor

Back to WIP under #48's process: this was carried over before the new template. Its description will be brought to the template and measured on Longshot (main against this PR, at named commits) when it's wanted for review; ask here if you'd like it sooner.

Back to WIP under #48's process: this was carried over before the new template. Its description will be brought to the template and measured on Longshot (main against this PR, at named commits) when it's wanted for review; ask here if you'd like it sooner.
Author
First-time contributor

Description brought to Longshot's PR template (#48). Still WIP: its numbers are from the GitHub version, and it isn't rebased on current main; it gets measured on Longshot (main against this PR, at named commits) before it is marked ready.

Description brought to Longshot's PR template (#48). Still WIP: its numbers are from the GitHub version, and it isn't rebased on current main; it gets measured on Longshot (main against this PR, at named commits) before it is marked ready.
Author
First-time contributor

Description moved to the template's Testing section and whole checklist (#72): what was tested, what wasn't, and what needs other testers (their test requests to open while this is WIP).

Description moved to the template's Testing section and whole checklist (#72): what was tested, what wasn't, and what needs other testers (their test requests to open while this is WIP).
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u 'fix/hash-cache-clock:dyst-fix/hash-cache-clock'
git switch 'dyst-fix/hash-cache-clock'
Sign in to join this conversation.
No description provided.