MagicBeanDev/Hookshot
Archived
Watch
6
0
Fork
You've already forked Hookshot
 0
Code Issues 125 Pull requests 205 Activity

perf(lift): emit the lifted C in several processes on Windows too #2748

Closed
github-sync wants to merge 2 commits from github/pr/2748 into main
pull from: github/pr/2748
merge into: MagicBeanDev:main
Conversation 1 Commits 2 Files changed 3 +425 -30
github-sync commented 2026-11-12 11:35:41 +00:00
Owner

@Jiro131 opened this pull request on GitHub · 2026-11-12 10:45 UTC · original
Merged on GitHub in 8c2a3c3a37

On Windows the lift writes the game's C in one process, because there is no fork: about 65 s of the lift on a 32-thread PC here, and 145–170 s on the Ryzen 9 3900X of #2636's Phase 0. This is that proposal's item A3: the emit now runs in several new processes on Windows and writes the same files, byte for byte.

Who this reaches

Builds that translate the game again. Since #2324 an update with an unchanged lift key skips the lift. nx2/chunks.py is one of the key's inputs, so the first build after this lands translates once more.

Change

nx2/chunks.py:

  • New processes where there is no fork (spawn_emit). Each gets the lifter's small tables once (EMIT_TABLES) and, with each run of 2,000 functions, their bodies; it translates that run's words itself. A3 sketched workers that map image.bin; sending the bodies needed no second discovery.
  • Only when the workers' emit is the lift's: a plain a64_lift.Lifter (no subclass, no emit_function of its own), with nx2/poll.py installed exactly when its counter is passed. Otherwise one process, as before.
  • Each worker checks itself: it compares where its translator and emit passes come from with the lift's, and stops if they differ. The lift then emits the rest itself and says so.
  • The log line says what ran: lift: emit and write (16 processes) or (1 process). #2693's pattern reads both.
  • Linux and macOS are unchanged: forked workers, as before.

One more change, for everyone on Windows: the available-memory cap (one process per 2 GB free, from the Steam Deck reports) now applies on Windows too, where free memory was not read before. It is file_jobs()' cap, so the per-file passes of #2324 are limited by it as well on a PC with little free memory. That cost is not measured, and I can move it to its own PR if you prefer.

Measured

i9-13900K (24 cores, 32 threads), 64 GB, NVMe, Windows 11 Home 10.0.26300. The standalone lift (nx2/lift.py --standalone --write) in its default configuration (polls on, A64_LOCAL_REGS unset, no hot list), main (ba005ca4) and this branch alternated, three runs each:

emit and write whole standalone lift
main, 1 process 66.5 s, 64.9 s, 66.0 s 119 s, 117 s, 120 s
this branch, 16 processes 20.9 s, 21.2 s, 21.2 s 73 s, 73 s, 74 s
  • Same output: the 85 files of the gen folder are SHA-256 identical between the two.
  • Fewer processes (an earlier revision of this branch): 38.1 s with 4, 26.1 s with 8.
  • Memory: about 170 MB per worker. Peak working set summed over the lift's processes: 5.1–5.9 GB with 1, 7.9 GB with 16.

In the game

A full build with the Builder's steps (nx2/make_game.py --nup, French, shaders not prepared) from this branch plus one unrelated commit (the damaged-texture check opening files several at a time, its own PR): the log says lift: emit and write (16 processes) in 23.2s. Build v2.1.0-rc.3-256-gf79aa054, RTX 4090, NVIDIA 621.01, Vulkan 1.4, Windows 11, 5120x1440, upscaler off, frame generation off, 60 fps cap. Kokiri Forest from a save: 52–60 fps, no device lost, no stutter seen, nothing out of place.

Testing

  • tests/test_spawn_emit.py (new, game-free): a real a64_lift.Lifter over a made-up image of 4,500 functions, written by chunks.write with 1 process and with 3 spawned ones: the same files and counters. It covers:

    • the plain configuration;
    • the lift's default (poll.install(), A64_LOCAL_REGS=4, comments on);
    • a translator that differs in the workers (the lift falls back and says so);
    • a Lifter subclass (kept in one process).

    A fallback cannot pass for the spawn: the "worker process stopped" line fails the test.

  • Teeth: each entry of EMIT_TABLES, dropped or sent empty, makes it fail (checked by hand).

  • Where it runs: it builds the translator in a temporary folder, so it needs a C++ compiler, as tests/test_fpx_sticky.py does. On the game-free CI (windows-2022 and ubuntu-24.04, Python 3.12) of a private copy of this branch, it passed on both (15.7 s on Windows; 18.2 s and 8.3 s in two Linux runs). Those two Linux runs each had one other failure, a different one each time and outside this change: tests/test_bake_skeleton.py stopped at 600 s on a runner where every check took twice its usual time, then tests/test_linux_saves_move.py (host/hos_main.c) failed once after passing in the first run.

  • python -I -B tests/run_all.py on Windows: 448 passed, 5 failed. nx2/test_nup_paths.py needs the symlink privilege. tests/test_bug_report.py and tests/test_input_background.py fail the same way here without this change. nx2/test_nup_verify.py and tests/test_prepare_at_build.py failed once with 32 checks at once and pass alone.

Not measured

  • --no-polls, A64_LOCAL_REGS or a hot list on real data (the test covers the first two on the made-up image).
  • A PC with 16 GB or less, where the cap leaves fewer processes.
  • The per-file passes under the new Windows cap.

See #2636
See #2324

Assisted-by: Claude Opus 5.5
🤖 Generated with Claude Code

> [@Jiro131](https://github.com/Jiro131) opened this pull request on GitHub · 2026-11-12 10:45 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2748) > **Merged on GitHub** in https://git.magicbean.dev/MagicBeanDev/Hookshot/commit/8c2a3c3a37ebd4b3b104a26d57cb8aeb7c60cda2 On Windows the lift writes the game's C in one process, because there is no `fork`: about 65 s of the lift on a 32-thread PC here, and 145–170 s on the Ryzen 9 3900X of #2636's Phase 0. This is that proposal's item A3: the emit now runs in several new processes on Windows and writes the same files, byte for byte. ## Who this reaches Builds that translate the game again. Since #2324 an update with an unchanged lift key skips the lift. `nx2/chunks.py` is one of the key's inputs, so the first build after this lands translates once more. ## Change `nx2/chunks.py`: - **New processes where there is no fork** (`spawn_emit`). Each gets the lifter's small tables once (`EMIT_TABLES`) and, with each run of 2,000 functions, their bodies; it translates that run's words itself. A3 sketched workers that map `image.bin`; sending the bodies needed no second discovery. - **Only when the workers' emit is the lift's:** a plain `a64_lift.Lifter` (no subclass, no `emit_function` of its own), with `nx2/poll.py` installed exactly when its counter is passed. Otherwise one process, as before. - **Each worker checks itself:** it compares where its translator and emit passes come from with the lift's, and stops if they differ. The lift then emits the rest itself and says so. - **The log line says what ran:** `lift: emit and write (16 processes)` or `(1 process)`. #2693's pattern reads both. - **Linux and macOS are unchanged:** forked workers, as before. **One more change, for everyone on Windows:** the available-memory cap (one process per 2 GB free, from the Steam Deck reports) now applies on Windows too, where free memory was not read before. It is `file_jobs()`' cap, so the per-file passes of #2324 are limited by it as well on a PC with little free memory. That cost is not measured, and I can move it to its own PR if you prefer. ## Measured i9-13900K (24 cores, 32 threads), 64 GB, NVMe, Windows 11 Home 10.0.26300. The standalone lift (`nx2/lift.py --standalone --write`) in its default configuration (polls on, `A64_LOCAL_REGS` unset, no hot list), `main` (ba005ca4) and this branch alternated, three runs each: | | `emit and write` | whole standalone lift | |---|---|---| | `main`, 1 process | 66.5 s, 64.9 s, 66.0 s | 119 s, 117 s, 120 s | | this branch, 16 processes | 20.9 s, 21.2 s, 21.2 s | 73 s, 73 s, 74 s | - **Same output:** the 85 files of the gen folder are SHA-256 identical between the two. - **Fewer processes** (an earlier revision of this branch): 38.1 s with 4, 26.1 s with 8. - **Memory:** about 170 MB per worker. Peak working set summed over the lift's processes: 5.1–5.9 GB with 1, 7.9 GB with 16. ## In the game A full build with the Builder's steps (`nx2/make_game.py --nup`, French, shaders not prepared) from this branch plus one unrelated commit (the damaged-texture check opening files several at a time, its own PR): the log says `lift: emit and write (16 processes) in 23.2s`. Build `v2.1.0-rc.3-256-gf79aa054`, RTX 4090, NVIDIA 621.01, Vulkan 1.4, Windows 11, 5120x1440, upscaler off, frame generation off, 60 fps cap. Kokiri Forest from a save: 52–60 fps, no device lost, no stutter seen, nothing out of place. ## Testing - **`tests/test_spawn_emit.py` (new, game-free):** a real `a64_lift.Lifter` over a made-up image of 4,500 functions, written by `chunks.write` with 1 process and with 3 spawned ones: the same files and counters. It covers: - the plain configuration; - the lift's default (`poll.install()`, `A64_LOCAL_REGS=4`, comments on); - a translator that differs in the workers (the lift falls back and says so); - a `Lifter` subclass (kept in one process). A fallback cannot pass for the spawn: the "worker process stopped" line fails the test. - **Teeth:** each entry of `EMIT_TABLES`, dropped or sent empty, makes it fail (checked by hand). - **Where it runs:** it builds the translator in a temporary folder, so it needs a C++ compiler, as `tests/test_fpx_sticky.py` does. On the game-free CI (windows-2022 and ubuntu-24.04, Python 3.12) of a private copy of this branch, it passed on both (15.7 s on Windows; 18.2 s and 8.3 s in two Linux runs). Those two Linux runs each had one other failure, a different one each time and outside this change: `tests/test_bake_skeleton.py` stopped at 600 s on a runner where every check took twice its usual time, then `tests/test_linux_saves_move.py` (host/hos_main.c) failed once after passing in the first run. - **`python -I -B tests/run_all.py` on Windows:** 448 passed, 5 failed. `nx2/test_nup_paths.py` needs the symlink privilege. `tests/test_bug_report.py` and `tests/test_input_background.py` fail the same way here without this change. `nx2/test_nup_verify.py` and `tests/test_prepare_at_build.py` failed once with 32 checks at once and pass alone. ## Not measured - `--no-polls`, `A64_LOCAL_REGS` or a hot list on real data (the test covers the first two on the made-up image). - A PC with 16 GB or less, where the cap leaves fewer processes. - The per-file passes under the new Windows cap. See #2636 See #2324 Assisted-by: Claude Opus 5.5 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- github-sync:item:2748 -->
Where there is no fork the emit ran in one process: 65 s of a 101 s lift to
the end of the emit on a 32-thread PC with ample memory (nx2/chunks.py left
it serial when the per-file passes of #2324 went parallel). Workers are now
new processes; each gets the lifter's small tables once (EMIT_TABLES) and,
with each run of functions, their bodies, and translates that run's words
itself.

Measured here (32 threads, available memory well past 2 GB a worker, the
default configuration of nx2/lift.py: polls on, A64_LOCAL_REGS unset, no hot
list), main and this change alternated, three runs each: emit 64.9-66.5 s
-> 20.9-21.2 s with 16 processes, the whole standalone lift 117-120 s ->
73-74 s, and the 85 files of the gen folder are byte-identical (SHA-256) to
main's. Not measured on real data: --no-polls, A64_LOCAL_REGS, a hot list, a
PC with 16 GB or less; tests/test_spawn_emit.py covers the first two on a
made-up image.

The new processes only match the lift when their emit is the lift's, so
the spawn is used only for a plain a64_lift.Lifter (no subclass, no
emit_function of its own) with nx2/poll.py installed exactly when its
counter is passed; each worker compares where its translator and the
lifter's emit (nx2/insn.py's and nx2/poll.py's patches reach it by running
nx2/lift.py's module level again) come from with the lift's and stops if
they differ. The lift then emits the rest in this process, and the message
says a worker stopped or could not start. write() now prints the processes
that ran, not the ones asked for.

One more change: the available-memory cap of emit_jobs() (one worker per 2 GB,
a Steam Deck ran out with 8) now also applies on Windows, and it is
file_jobs()', so the per-file passes of #2324 are capped by it there too on
a PC with little free memory (they were not, on Windows).

Assisted-by: Claude Code
Co-Authored-By: Claude Opus 5.5 <[email protected]>
A real a64_lift.Lifter over a made-up image of 4,500 small functions (no
game data), written by chunks.write with 1 process and with 3 spawned ones:
the same files byte for byte and the same counters. The image has loops,
a word the translator does not handle, a PLT stub, BLRs with a usual
target, BRs with and without a jump table, code after an instruction,
names, functions without local registers and the callers of a function
named setjmp; the lift's default configuration is run too (poll.install()
with counters=[poll.stats], A64_LOCAL_REGS=4, comments on), as are a
translator that differs in the worker (the lift falls back and says so)
and a Lifter subclass (the gate keeps it in one process). The parent's
"worker process stopped" message fails the test, so a fallback cannot pass
for the spawn.

Checked by hand: each entry of chunks.EMIT_TABLES, dropped or sent empty,
makes the test fail (the docstring has the table). Not covered: --no-polls
with A64_LOCAL_REGS on real data, hot lists, a fork-less Linux. The
translator is built in a temporary folder, not under third_party/, so the
test is skipped without a C++ compiler (RUN_ALL_ARGS). tests/test_parallel_emit.py
only runs where there is fork.

Assisted-by: Claude Code
Co-Authored-By: Claude Opus 5.5 <[email protected]>
github-sync closed this pull request 2026-11-12 11:36:39 +00:00
github-sync commented 2026-11-12 11:36:41 +00:00
Author
Owner

@hckrman101 approved on GitHub · 2026-11-12 10:47 UTC · original

Nice change, and the measurements are what I wanted: 66 s down to 21 s for the emit on the i9-13900K (three alternating runs each, so the spread is clear), the 85 gen files SHA-256 identical, and a full Builder-style build plus a play check in the description.

I read the spawn path against Windows semantics and it holds up:

  • The workers are new processes that import chunks by the path the parent passes, and nx2/lift.py keeps its work under __main__, so its module-level patches (insn.install()) are all a child re-runs. -I is passed on to the children by multiprocessing (checked), so the isolated interpreter the build uses carries over.
  • No state is assumed in a child: it gets the tables once and the bodies per run, installs the polls only when the parent had, and compares the identity of the translator and emit passes with the parent's before emitting. A mismatch fails the initializer, so the pool breaks and the parent emits the rest and says so. A real exception in a run propagates out of pool.map and fails the write, so there is no hang and no half-written output (whole_write moves files over only at the end).
  • Order and chunk boundaries come from the parent: runs are consumed in sorted(L.funcs) order and write() still decides the boundaries from the texts it gets, so the lift cache and objcache see the same bytes. The .h files and the counters are compared in the test too, not only the .c files.
  • The worker count goes through file_jobs(), so HOOKSHOT_LIFT_JOBS and the memory gate apply, and with one job, a subclassed or patched Lifter, an unexpected poll setup, or fewer than 4000 functions it stays in one process. On Linux and macOS the forked path is untouched (only the log line changes).

I ran python3 -I -B tests/run_all.py on Linux (the spawn path is exercised there by patching _no_fork) for test_spawn_emit, test_parallel_emit, test_lift_cache, test_chunk_split, test_chunks_save_folder, test_stale_hot_pieces, test_failure_reason, test_mark_groups and test_import_span: all pass.

Remarks, none blocking:

  • Nit: the gate reads poll._installed, a private name of poll.py. A small public accessor there would be cleaner, but it is fine as is.
  • The Windows memory cap on file_jobs() is a behaviour change for the per-file passes, which you flag as unmeasured. I'm happy to keep it in this PR: it is the same rule Linux already runs under, and it only bites on a PC with under 32 GB free.
  • Linux timing is not in the table. Since that path is unchanged I don't need it, but one line saying the Linux emit time is the same would close the loop.
  • The checks on the head commit show only the title check so far (the game-free tests haven't run on it). Your private-copy run covers it, but it would be good to see them green here before merging.

Approving on the strength of the numbers and the test coverage.

> [@hckrman101](https://github.com/hckrman101) approved on GitHub · 2026-11-12 10:47 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2748#pullrequestreview-5873067516) Nice change, and the measurements are what I wanted: 66 s down to 21 s for the emit on the i9-13900K (three alternating runs each, so the spread is clear), the 85 gen files SHA-256 identical, and a full Builder-style build plus a play check in the description. I read the spawn path against Windows semantics and it holds up: - The workers are new processes that import `chunks` by the path the parent passes, and `nx2/lift.py` keeps its work under `__main__`, so its module-level patches (`insn.install()`) are all a child re-runs. `-I` is passed on to the children by `multiprocessing` (checked), so the isolated interpreter the build uses carries over. - No state is assumed in a child: it gets the tables once and the bodies per run, installs the polls only when the parent had, and compares the identity of the translator and emit passes with the parent's before emitting. A mismatch fails the initializer, so the pool breaks and the parent emits the rest and says so. A real exception in a run propagates out of `pool.map` and fails the write, so there is no hang and no half-written output (`whole_write` moves files over only at the end). - Order and chunk boundaries come from the parent: runs are consumed in `sorted(L.funcs)` order and `write()` still decides the boundaries from the texts it gets, so the lift cache and objcache see the same bytes. The `.h` files and the counters are compared in the test too, not only the `.c` files. - The worker count goes through `file_jobs()`, so `HOOKSHOT_LIFT_JOBS` and the memory gate apply, and with one job, a subclassed or patched `Lifter`, an unexpected poll setup, or fewer than 4000 functions it stays in one process. On Linux and macOS the forked path is untouched (only the log line changes). I ran `python3 -I -B tests/run_all.py` on Linux (the spawn path is exercised there by patching `_no_fork`) for `test_spawn_emit`, `test_parallel_emit`, `test_lift_cache`, `test_chunk_split`, `test_chunks_save_folder`, `test_stale_hot_pieces`, `test_failure_reason`, `test_mark_groups` and `test_import_span`: all pass. Remarks, none blocking: - Nit: the gate reads `poll._installed`, a private name of `poll.py`. A small public accessor there would be cleaner, but it is fine as is. - The Windows memory cap on `file_jobs()` is a behaviour change for the per-file passes, which you flag as unmeasured. I'm happy to keep it in this PR: it is the same rule Linux already runs under, and it only bites on a PC with under 32 GB free. - Linux timing is not in the table. Since that path is unchanged I don't need it, but one line saying the Linux emit time is the same would close the loop. - The checks on the head commit show only the title check so far (the game-free tests haven't run on it). Your private-copy run covers it, but it would be good to see them green here before merging. Approving on the strength of the numbers and the test coverage. <!-- github-sync:r:5873067516 -->
Commenting is not possible because the repository is archived.
Reviewers
No reviewers
Labels
Clear labels   
accessibility
Barrier affecting people with disabilities

  
autorelease: pending

   
autorelease: tagged

   
bug
Something isn't working

  
dependencies
Pull requests that update a dependency file

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
github_actions
Pull requests that update GitHub Actions code

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
testing wanted

   
wontfix
This will not be worked on

No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Hookshot!2748
No description provided.