perf(build): a source whose size, time and id are unchanged is not read again to name its object #2702

Closed
github-sync wants to merge 1 commit from github/pr/2702 into main
Owner

@jordanella opened this pull request on GitHub · 2026-11-12 07:25 UTC · original
Merged on GitHub in ba005ca4fa

Every build reads and hashes each source again to name its object, and the 1.4 GB of lifted C up to three times: migrate_stamps, the log's fingerprint and plan_jobs. On Windows that planning was 6–7.5 s of a one-file renderer edit's ~31 s build, as #2693's table shows. This is the dev-loop proposal's A5 (#2636).

Change

  • The record: build.py keeps <build>/hash_cache.json. For each source it holds the size, modification time and file id, the content's sha1, and the object key worked out from it for each set of flags and headers (kept_part).
  • key_of() returns that key without reading the file while all three match, and otherwise reads and hashes the file as before.
  • No key changes: object_key, the key's definition, is unchanged, so every object's key, every object cache and every player's kept translation stays as it is.
  • The racy rule: a file written less than 2 s before the record was is read again, as with git's "racily clean" entries. An untrusted record whose file turns out to have other content loses its keys.
  • Callers: plan_jobs and the kept list use key_of. The log's fingerprint uses the recorded sha1, so its value changes once, and it still says which part changed. migrate_stamps reads nothing stamped with today's kind of key, since that key's object is in the cache.
  • When it's saved: after the plan, which make_game.py's builds run (HOOKSHOT_TIMES). A bare build.py run without it hashes in compile_one as before.

Measured

On Windows (Ryzen 9 3900X, NVMe), three one-file gpu/vk/vk_draw.cpp edit builds in a row, with #2693's table:

Build Planning Compiling Total
first (the record made) 6 s 18 s 31 s
second 2 s 17 s 26 s
third 2 s 19 s 29 s

Each build still compiled the edited file.

Testing

  • tests/test_build_hash_cache.py (new, 9 cases):

    • the key equals object_key, and the next build doesn't read the source;
    • another size or time reads it again;
    • other flags or headers are another key, and both are kept;
    • a file written near the record isn't trusted;
    • an untrusted record of other content loses its keys;
    • a record that's broken or of another scheme is ignored;
    • a gone file leaves the record;
    • migrate_stamps reads nothing already stamped with today's kind of key;
    • and main wires it in that order.

    All fail against main's build.py.

  • The checks that cut functions out of build.py (test_compile_retry_jobs, test_dlss_sdk_cache, test_shader_stamp_cache) pass unchanged, since object_key and compile_one are as they were.

  • Every check that reads build.py (39), on Windows, passes apart from test_driver_cache_folder, test_fpx_sticky and test_linux_update, which fail the same way with main's build.py here.

  • Not tested: Linux. The file id there is the inode, as os.stat gives it.

🤖 Generated with Claude Code

> [@jordanella](https://github.com/jordanella) opened this pull request on GitHub · 2026-11-12 07:25 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2702) > **Merged on GitHub** in https://git.magicbean.dev/MagicBeanDev/Hookshot/commit/ba005ca4fa179ccd2140f06f9308d0ff63894ff6 Every build reads and hashes each source again to name its object, and the 1.4 GB of lifted C up to three times: `migrate_stamps`, the log's fingerprint and `plan_jobs`. On Windows that planning was 6–7.5 s of a one-file renderer edit's ~31 s build, as #2693's table shows. This is the dev-loop proposal's A5 (#2636). ## Change - **The record:** `build.py` keeps `<build>/hash_cache.json`. For each source it holds the size, modification time and file id, the content's sha1, and the object key worked out from it for each set of flags and headers (`kept_part`). - **`key_of()`** returns that key without reading the file while all three match, and otherwise reads and hashes the file as before. - **No key changes:** `object_key`, the key's definition, is unchanged, so every object's key, every object cache and every player's kept translation stays as it is. - **The racy rule:** a file written less than 2 s before the record was is read again, as with git's "racily clean" entries. An untrusted record whose file turns out to have other content loses its keys. - **Callers:** `plan_jobs` and the kept list use `key_of`. The log's fingerprint uses the recorded sha1, so its value changes once, and it still says which part changed. `migrate_stamps` reads nothing stamped with today's kind of key, since that key's object is in the cache. - **When it's saved:** after the plan, which `make_game.py`'s builds run (`HOOKSHOT_TIMES`). A bare `build.py` run without it hashes in `compile_one` as before. ## Measured On Windows (Ryzen 9 3900X, NVMe), three one-file `gpu/vk/vk_draw.cpp` edit builds in a row, with #2693's table: | Build | Planning | Compiling | Total | |---|---|---|---| | first (the record made) | 6 s | 18 s | 31 s | | second | **2 s** | 17 s | 26 s | | third | **2 s** | 19 s | 29 s | Each build still compiled the edited file. ## Testing - **`tests/test_build_hash_cache.py` (new, 9 cases):** - the key equals `object_key`, and the next build doesn't read the source; - another size or time reads it again; - other flags or headers are another key, and both are kept; - a file written near the record isn't trusted; - an untrusted record of other content loses its keys; - a record that's broken or of another scheme is ignored; - a gone file leaves the record; - `migrate_stamps` reads nothing already stamped with today's kind of key; - and `main` wires it in that order. All fail against `main`'s `build.py`. - **The checks that cut functions out of `build.py`** (`test_compile_retry_jobs`, `test_dlss_sdk_cache`, `test_shader_stamp_cache`) pass unchanged, since `object_key` and `compile_one` are as they were. - **Every check that reads `build.py` (39), on Windows,** passes apart from `test_driver_cache_folder`, `test_fpx_sticky` and `test_linux_update`, which fail the same way with `main`'s `build.py` here. - **Not tested:** Linux. The file id there is the inode, as `os.stat` gives it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- github-sync:item:2702 -->
Every build read and hashed each source again to name its object, and the 1.4 GB of lifted C up to three times
(migrate_stamps, the fingerprint in the log, plan_jobs): planning was 6-7.5 s of a one-file renderer edit's ~31 s
build on Windows (A1's table, #2693). This is the dev-loop proposal's A5 (#2636).

build.py keeps a record in <build>/hash_cache.json: for each source its size, modification time and file id, its
content's sha1, and the object key worked out from it for each set of flags and headers (kept_part). key_of() returns
that key without reading the file while all three match; anything else reads and hashes it as before, and object_key
itself, the key's definition, is unchanged, so every object's key and every cache stays as it is. A file written less
than 2 s before the record was is read again (git's "racily clean" entries), and a record not trusted for its time
loses its keys once the file is read with other content. plan_jobs and the kept list use key_of, the log's
fingerprint uses the recorded sha1, and migrate_stamps reads nothing stamped with today's kind of key (its object is
in the cache by it). The record is saved after the plan, which make_game.py's builds run; a bare build.py run without
HOOKSHOT_TIMES keeps hashing in compile_one as before.

Measured on Windows with A1's table: planning 6 s on the first edit build, 2 s on the next two.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
github-sync closed this pull request 2026-11-12 09:32:55 +00:00
Author
Owner

@hckrman101 commented on nx2/build.py line 729 on GitHub · 2026-11-12 09:24 UTC · original

This is the time of the save, after plan_jobs stat'd and read every source (seconds earlier on a first build). For the 2 s rule to mean "the file was read at least one timestamp tick after it was written", this should be the time before the first stat of this build, e.g. time.time_ns() taken in load_hashes and kept for the save. Non-blocking: with ns/100 ns mtimes it's theoretical.

> [@hckrman101](https://github.com/hckrman101) commented on `nx2/build.py` line 729 on GitHub · 2026-11-12 09:24 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2702#discussion_r4548558110) This is the time of the save, after `plan_jobs` stat'd and read every source (seconds earlier on a first build). For the 2 s rule to mean "the file was read at least one timestamp tick after it was written", this should be the time before the first stat of this build, e.g. `time.time_ns()` taken in `load_hashes` and kept for the save. Non-blocking: with ns/100 ns mtimes it's theoretical. <!-- github-sync:rc:4548558110 -->
Author
Owner

@hckrman101 approved on GitHub · 2026-11-12 09:24 UTC · original

Thanks, this is a clean A5. The key's definition is untouched (key_of computes exactly what object_key does, just from a remembered sha1 when size, mtime and file id all match), so no cached object or kept translation gets renamed, and a miss falls straight back to reading the file. The migrate_stamps shortcut is sound too: an old-kind stamp can't name an object in the cache, so skipping stamps whose object is already cached only skips work that would have done nothing. The 6 s to 2 s planning numbers on the same one-file edit are what I wanted to see, and CI is green.

Two non-blocking points:

  • The racy rule's clock (inline): written_ns is taken when the record is saved, after the whole plan, while each file was stat'd and read earlier, up to the ~6 s planning time before. Git's rule works because the index's time is close to the stat. Taking the time once before the first stat (e.g. in load_hashes, or at the start of main) and saving that would keep the 2 s rule exact even on a filesystem with coarse timestamps (exFAT, some network shares). It only matters for an edit landing in the same timestamp tick right after the read, so on NTFS/ext4 it's theoretical, but it's a one-line change.
  • Nit, the record only grows: save_hashes keeps every entry still on disk and every kept_part each entry has seen. kept_part includes the headers, so each edit to a header under gpu/vk adds one key to every renderer source that isn't itself touched. It's small per edit, but saving only the entries and keys this build used would keep the file (and the JSON parse every build) bounded.

A Linux dev-loop run would be nice to have in the description when you get a chance (it should behave the same, as st_ino is the inode there), but I don't see anything platform-specific that would change. #2706 also changes build.py (and how the renderer's headers feed the key), so whichever of the two lands second may need a small merge.

> [@hckrman101](https://github.com/hckrman101) approved on GitHub · 2026-11-12 09:24 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2702#pullrequestreview-5872024996) Thanks, this is a clean A5. The key's definition is untouched (`key_of` computes exactly what `object_key` does, just from a remembered sha1 when size, mtime and file id all match), so no cached object or kept translation gets renamed, and a miss falls straight back to reading the file. The `migrate_stamps` shortcut is sound too: an old-kind stamp can't name an object in the cache, so skipping stamps whose object is already cached only skips work that would have done nothing. The 6 s to 2 s planning numbers on the same one-file edit are what I wanted to see, and CI is green. Two non-blocking points: - **The racy rule's clock** (inline): `written_ns` is taken when the record is saved, after the whole plan, while each file was stat'd and read earlier, up to the ~6 s planning time before. Git's rule works because the index's time is close to the stat. Taking the time once before the first stat (e.g. in `load_hashes`, or at the start of `main`) and saving that would keep the 2 s rule exact even on a filesystem with coarse timestamps (exFAT, some network shares). It only matters for an edit landing in the same timestamp tick right after the read, so on NTFS/ext4 it's theoretical, but it's a one-line change. - **Nit, the record only grows:** `save_hashes` keeps every entry still on disk and every `kept_part` each entry has seen. `kept_part` includes the headers, so each edit to a header under `gpu/vk` adds one key to every renderer source that isn't itself touched. It's small per edit, but saving only the entries and keys this build used would keep the file (and the JSON parse every build) bounded. A Linux dev-loop run would be nice to have in the description when you get a chance (it should behave the same, as `st_ino` is the inode there), but I don't see anything platform-specific that would change. #2706 also changes `build.py` (and how the renderer's headers feed the key), so whichever of the two lands second may need a small merge. <!-- github-sync:r:5872024996 -->
Commenting is not possible because the repository is archived.
No description provided.