MagicBeanDev/Hookshot

Archived

Watch

Fork 0

perf(builder): open the files of the damaged-texture check several at a time #2769

Open

github-sync wants to merge 1 commit from github/pr/2769 into main

github-sync commented

2026-11-12 12:37:03 +00:00

Owner

@Jiro131 opened this pull request on GitHub · 2026-11-12 12:12 UTC · original

The first build of every installed game after the next release spends about 8 minutes on the damaged-texture check, on Windows. #2391 changed the check's stamp, so each game folder runs the full check once. Without the extraction's list (it is deleted after the first check), that means opening all 102,185 romfs .zs files, one at a time. Here each first open of a file took about 4-5 ms, and the build showed only "checked N of 102,185 files" for 486 s and 492 s. This PR opens the files several at a time: 27 s and 28 s.

Fixes #2754. Tracking: #2505.

What changed

nx2/nup.py: damaged_game_files opens the files from a thread pool of min(32, CPU threads + 4), Python's own default for I/O work. The damaged files and the progress still come in the listed order. A file that cannot be read still stops the check. The opens are queued 4096 at a time.
The per-open wait is attributed to the virus scanner (Windows Defender) scanning each file on its first open. This was not confirmed with the scanner off, and the code comment says so.
docs/worklist.md: a row for #2754 (S3 x 0.57 Windows x 1.0 reproduced = 0.57), owner Jiro131.
docs/tested-configurations.md: the game run below.

How it was tested

The check on a fresh unpack. Four new game folders, built from a .nup with nx2/make_game.py --nup (the Builder's steps), interleaved. Each was built once, then built again with .hookshot_romfs_checked removed, as #2391's stamp change does:

	first build	second build	the check in it
`main` (`ba005ca4`), folder 1	247 s	490 s	486 s
this PR, folder 1	248 s	33 s	27 s
this PR, folder 2	251 s	34 s	28 s
`main`, folder 2	255 s	509 s	492 s

None of the four reported a damaged file. On files already opened once (three runs each, alternating), the check took 5.0-5.9 s before and 4.9-5.6 s after.

Game-free tests (tests/test_damaged_check_progress.py):

a new test where the first two opens wait for each other at a barrier, which a check opening one file at a time never gets past. It fails on main (BrokenBarrierError after 5 s) and passes with the change.
a second new test with 4,500 files in three folders, more files than one slice and than the threads: the damaged files and the progress come in the listed order, and an unreadable file still stops the check.

Full suite: python -I -B tests/run_all.py: 451 passed, 3 failed, 41 skipped (Windows). The 3 fail the same way on main 51da0464 on this machine, without the change: nx2/test_nup_paths.py (symbolic links need administrator rights or developer mode here), tests/test_bug_report.py (a UnicodeDecodeError on byte 0x92 in a child process's output) and tests/test_input_background.py (its -Werror compile with llvm-mingw's gcc).

In the game: the second folder of this PR (v2.1.0-rc.3-254-ga3b0d56b, the same change on main ba005ca4). RTX 4090, NVIDIA 621.01, Windows 11, i9-13900K, default options (2048x1152 output, the game's upscaler, frame generation off). Kokiri Forest from a save, moving around: 26 windows of 120 presents, median 59.8 fps (30.5-60.3) at 1,720-3,733 draws. No crash, no device lost.

Not tested

Linux and macOS: the check runs there too, but whether a first open costs as much there is not measured.
HDDs, network drives, machines with few cores. The pool is at most 32 threads, so an HDD may seek more.
The check with the virus scanner off, so the cause of the 4-5 ms per open is not confirmed.
The Builder app itself: built with make_game.py, as the Builder runs it.

docs/tested-configurations.md also gets a row in #2757: whichever merges second needs a rebase of that one line.

🤖 Generated with Claude Code

> [@Jiro131](https://github.com/Jiro131) opened this pull request on GitHub · 2026-11-12 12:12 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2769) The first build of every installed game after the next release spends about 8 minutes on the damaged-texture check, on Windows. #2391 changed the check's stamp, so each game folder runs the full check once. Without the extraction's list (it is deleted after the first check), that means opening all 102,185 romfs `.zs` files, one at a time. Here each first open of a file took about 4-5 ms, and the build showed only "checked N of 102,185 files" for 486 s and 492 s. This PR opens the files several at a time: 27 s and 28 s. Fixes #2754. Tracking: #2505. ## What changed - `nx2/nup.py`: `damaged_game_files` opens the files from a thread pool of `min(32, CPU threads + 4)`, Python's own default for I/O work. The damaged files and the progress still come in the listed order. A file that cannot be read still stops the check. The opens are queued 4096 at a time. - The per-open wait is attributed to the virus scanner (Windows Defender) scanning each file on its first open. This was not confirmed with the scanner off, and the code comment says so. - `docs/worklist.md`: a row for #2754 (S3 x 0.57 Windows x 1.0 reproduced = 0.57), owner Jiro131. - `docs/tested-configurations.md`: the game run below. ## How it was tested **The check on a fresh unpack.** Four new game folders, built from a `.nup` with `nx2/make_game.py --nup` (the Builder's steps), interleaved. Each was built once, then built again with `.hookshot_romfs_checked` removed, as #2391's stamp change does: | | first build | second build | the check in it | |---|---:|---:|---:| | `main` (ba005ca4), folder 1 | 247 s | 490 s | 486 s | | this PR, folder 1 | 248 s | 33 s | 27 s | | this PR, folder 2 | 251 s | 34 s | 28 s | | `main`, folder 2 | 255 s | 509 s | 492 s | None of the four reported a damaged file. On files already opened once (three runs each, alternating), the check took 5.0-5.9 s before and 4.9-5.6 s after. **Game-free tests** (`tests/test_damaged_check_progress.py`): - a new test where the first two opens wait for each other at a barrier, which a check opening one file at a time never gets past. It fails on `main` (`BrokenBarrierError` after 5 s) and passes with the change. - a second new test with 4,500 files in three folders, more files than one slice and than the threads: the damaged files and the progress come in the listed order, and an unreadable file still stops the check. **Full suite:** `python -I -B tests/run_all.py`: 451 passed, 3 failed, 41 skipped (Windows). The 3 fail the same way on `main` 51da0464 on this machine, without the change: `nx2/test_nup_paths.py` (symbolic links need administrator rights or developer mode here), `tests/test_bug_report.py` (a `UnicodeDecodeError` on byte 0x92 in a child process's output) and `tests/test_input_background.py` (its `-Werror` compile with llvm-mingw's gcc). **In the game:** the second folder of this PR (`v2.1.0-rc.3-254-ga3b0d56b`, the same change on `main` ba005ca4). RTX 4090, NVIDIA 621.01, Windows 11, i9-13900K, default options (2048x1152 output, the game's upscaler, frame generation off). Kokiri Forest from a save, moving around: 26 windows of 120 presents, median 59.8 fps (30.5-60.3) at 1,720-3,733 draws. No crash, no device lost. ## Not tested - Linux and macOS: the check runs there too, but whether a first open costs as much there is not measured. - HDDs, network drives, machines with few cores. The pool is at most 32 threads, so an HDD may seek more. - The check with the virus scanner off, so the cause of the 4-5 ms per open is not confirmed. - The Builder app itself: built with `make_game.py`, as the Builder runs it. `docs/tested-configurations.md` also gets a row in #2757: whichever merges second needs a rebase of that one line. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

github-sync added 1 commit

2026-11-12 12:37:03 +00:00

perf(builder): open the files of the damaged-texture check several at a time 5b97842a46

#2391 changed the damaged-texture check's stamp, so every installed game
runs the full check once on its first build with the next release. With
no list from the extraction left (it is deleted after the first check),
that check opens every romfs .zs file, 102,185 of them, one at a time.
On Windows each first open of a file took about 4-5 ms here: the check
took 486 s and 492 s in two builds (i9-13900K, NVMe, Windows Defender
on). We attribute the wait to the virus scanner scanning each file on
its first open; this was not confirmed with the scanner off.

Fixes #2754

## The change

Those waits overlap when several files are open at once, so the files
are opened by a thread pool of min(32, CPU threads + 4), Python's own
default for I/O work. Results and progress stay in the listed order. The
opens are queued 4096 at a time, not all 102,000 at once.

## Measured

The whole check after a fresh unpack, with the stamp removed as #2391's
change does, two new game folders each, interleaved: 486 s and 492 s
before, 27 s and 28 s after, the same files found. On files already
opened once (three runs each, alternating): 5.0-5.9 s before, 4.9-5.6 s
after.

Opening fresh copies of the game's .zs files, 2,044 files per run, ms
per file:

  threads  1    2    4     8     16    32    64
  ms       3.5  2.4  1.16  0.56  0.34  0.25  0.21

Not measured: Linux, HDDs, network drives, machines with few cores, and
the check with the virus scanner off.

## Tests

tests/test_damaged_check_progress.py:
- the first two opens wait for each other at a barrier, which a check
  opening one file at a time never gets past (fails before the change);
- 4,500 files in three folders, more than a slice and than the threads:
  the damaged ones and the progress in the listed order, and an
  unreadable file still stops the check.

docs: the worklist row for #2754 and the tested-configurations row of
the game run on this build.

Assisted-by: Claude Code
Co-Authored-By: Claude Opus 5.5 <[email protected]>

github-sync referenced this pull request

2026-11-12 12:37:04 +00:00

docs: add tested-configurations rows for DLSS Frame Generation on an RTX 4090 (Windows) #2771

github-sync referenced this pull request

2026-11-12 12:37:41 +00:00

Builder: the first build after the next release spends ~8 minutes opening every game file (damaged-texture check, one file at a time) #2754

github-sync referenced this pull request

2026-11-12 14:41:09 +00:00

fix(vk): stop requiring vertex strides for mesh fallback draws #2796

github-sync commented

2026-11-12 14:47:49 +00:00

Author

Owner

@Joey-Magnus approved on GitHub · 2026-11-12 14:35 UTC · original

I merged this into current main and ran tests/test_damaged_check_progress.py, nx2/test_nup_zstd_heads.py and nx2/test_nup_stand_in.py. All three pass. The change in nx2/nup.py:677-711 keeps the results and the progress in the listed order, because pool.map yields in submission order and progress is still called on the caller's thread. An unreadable file still raises, but only after the rest of its 4096-file slice has been opened. That costs a few seconds at most and doesn't change the outcome. Your before/after numbers come from a fresh unpack on the real Builder steps, which is the right measurement. The barrier test is a good way to prove the opens overlap. One heads-up: #2771 and #2778 also add rows at the top of docs/tested-configurations.md, and each pair of the three conflicts on that line, so whichever merges later needs a one-line rebase. HDDs and Linux are still unmeasured, as you say. A 32-thread cap seems safe for those.

> [@Joey-Magnus](https://github.com/Joey-Magnus) approved on GitHub · 2026-11-12 14:35 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2769#pullrequestreview-5876250949) I merged this into current main and ran tests/test_damaged_check_progress.py, nx2/test_nup_zstd_heads.py and nx2/test_nup_stand_in.py. All three pass. The change in nx2/nup.py:677-711 keeps the results and the progress in the listed order, because pool.map yields in submission order and progress is still called on the caller's thread. An unreadable file still raises, but only after the rest of its 4096-file slice has been opened. That costs a few seconds at most and doesn't change the outcome. Your before/after numbers come from a fresh unpack on the real Builder steps, which is the right measurement. The barrier test is a good way to prove the opens overlap. One heads-up: #2771 and #2778 also add rows at the top of docs/tested-configurations.md, and each pair of the three conflicts on that line, so whichever merges later needs a one-line rebase. HDDs and Linux are still unmeasured, as you say. A 32-thread cap seems safe for those.

github-sync referenced this pull request

2026-11-12 14:47:51 +00:00

docs: add tested-configurations rows for DLSS Frame Generation on an RTX 4090 (Windows) #2771

github-sync referenced this pull request

2026-11-12 14:48:09 +00:00

fix(tests): the input-background fixture reads oc_consumed_pads, so clang 23 builds it #2777

github-sync referenced this pull request

2026-11-12 14:48:11 +00:00

docs(tested): record paired Linux RTX 4090 rolling results #2778

github-sync referenced this pull request

2026-11-12 14:48:11 +00:00

docs(tested): record paired Linux RTX 4090 rolling results #2778