Skip to content

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

Open
Jiro131 wants to merge 1 commit into
NotRexed:mainfrom
Jiro131:perf/damaged-check-threads
Open

Jiro131 wants to merge 1 commit into
NotRexed:mainfrom
Jiro131:perf/damaged-check-threads

Conversation

@Jiro131

@Jiro131 Jiro131 commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

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 Builder: the first build after the next release spends ~8 minutes opening every game file (damaged-texture check, one file at a time) #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 (ba005ca), 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 51da046 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 ba005ca). 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

@Joey-Magnus Joey-Magnus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

… a time

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 NotRexed#2754

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.

The whole check after a fresh unpack, with the stamp removed as NotRexed#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/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 NotRexed#2754 and the tested-configurations row of
the game run on this build.

Assisted-by: Claude Code
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Jiro131
Jiro131 force-pushed the perf/damaged-check-threads branch from 5b97842 to 3207ded Compare November 12, 2026 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants