WIP: Build: a lift that lost functions is not reused; one build at a time on a work folder (#890) #918

Closed
amaralthalesalves wants to merge 3 commits from fix/lift-incomplete-not-reused into main
amaralthalesalves commented 2026-11-09 01:26:56 +00:00 (Migrated from github.com)

Refs #890 (and the v2.0.3 report in #733).

What the report points to. The v2.0.4 and v2.0.5 AppImages ship byte-identical work/ files (hot_funcs.txt, chunks.txt, the profile). Between those tags the lift code changed only in ways that don't affect its output: the lifted_c regex refactor and how made is held in chunks.write. So the difference comes from the state of the work folder, not the version.

With the shipped chunks.txt:

  • The lift makes 57 chunks, 8 hot pieces and 15 code_NNN_k.c parts, which is 80 files, v2.0.4's count. The reporter's 65 is 57 + 8 with no parts at all.
  • None of the reported symbols is a hot function. They cluster in the tails of the big chunks (014, 019, 022, 024 for #890; 014, 022, 031, 040 for #733), which is exactly what those parts hold.

A mechanism that gives exactly that (reproduced). Only split_big deletes parts. Suppose two lifts write into one sa/gen at once. Lift A cuts its big chunks into parts. Lift B's split_big then reads those already-cut chunks, finds them small, and deletes every part as stale. The result:

  • the functions in those parts are listed in functions.txt and called from other chunks and a64_table.c, but defined nowhere;
  • the remaining chunks are byte-identical to a good lift's, so their objects come from the cache;
  • the link fails on f_ symbols, and "20" is just lld's default --error-limit.

Nothing stops two builds on one work folder on Linux, because stop_stray_compilers acts on Windows only. A second build can come from another builder window, the game's update offer (--update) or the command line. I couldn't confirm that two builds actually overlapped on the reporter's PC; the lift section of their first v2.0.5 log would tell (no chunks: code_0NN.c (… MB) in N parts lines).

Why it never recovers (shown). write_lift_key vouched for the broken C, and every later build has the same key, so lifted_before skipped the lift. Clearing both object caches couldn't help, because the broken C is never regenerated.

The change

  • make_game.undefined_functions(gen) lists the functions functions.txt names that no code_*.c defines. It uses lifted_c.DEF_BYTES, which matches from the preceding newline and is about 10× faster than HEAD_BYTES (0.07 s vs 0.83 s on 181 MB).
  • After the lift, if anything is missing, the build stops with a clear message and writes no key, so the next build translates again.
  • lifted_before also requires that nothing is missing, so a folder already broken this way, like the reporter's, recovers by itself on the next build. This adds a scan of the lifted C (about 1.4 GB) to a build that skips the lift, roughly a second or two.
  • On POSIX, lock_work takes a non-blocking flock on <work>/.build_lock for the life of make_game, and a second build on the same work folder stops with a message. Python's fds are non-inheritable, so compilers don't hold the lock.

Tests. tests/test_lift_complete.py is game-free: it uses the real chunks.write, Lifter.write and split_hot, with no native translator needed. It:

  • reproduces the overlap (235 functions lost in that run);
  • checks that the loss is found and that lifted_before refuses the folder;
  • checks that a lift on its own restores it;
  • checks that the lock refuses a second build.

Against main's make_game.py it fails (no undefined_functions); on this branch it prints ok. tests/run_all.py lift make_game chunk gives 4 passed. An earlier run of related checks (test_parallel_emit, test_mem_gate, test_build_log, test_finish_output, test_build_times …) gave 11 passed.

Not verified. I haven't run a real game build. The reporter's trigger (two builds at once) is inferred. The detection and self-healing don't depend on it: any lift that leaves a listed function undefined is no longer reused.

Workaround for the reporter now: delete <work>/sa/gen/.lift_key (or sa/gen), then build once with no other build running.

Refs #890 (and the v2.0.3 report in #733). **What the report points to.** The v2.0.4 and v2.0.5 AppImages ship byte-identical `work/` files (`hot_funcs.txt`, `chunks.txt`, the profile). Between those tags the lift code changed only in ways that don't affect its output: the `lifted_c` regex refactor and how `made` is held in `chunks.write`. So the difference comes from the state of the work folder, not the version. With the shipped `chunks.txt`: - The lift makes 57 chunks, 8 hot pieces and 15 `code_NNN_k.c` parts, which is 80 files, v2.0.4's count. The reporter's 65 is 57 + 8 with **no parts at all**. - None of the reported symbols is a hot function. They cluster in the tails of the big chunks (014, 019, 022, 024 for #890; 014, 022, 031, 040 for #733), which is exactly what those parts hold. **A mechanism that gives exactly that (reproduced).** Only `split_big` deletes parts. Suppose two lifts write into one `sa/gen` at once. Lift A cuts its big chunks into parts. Lift B's `split_big` then reads those already-cut chunks, finds them small, and deletes every part as stale. The result: - the functions in those parts are listed in `functions.txt` and called from other chunks and `a64_table.c`, but defined nowhere; - the remaining chunks are byte-identical to a good lift's, so their objects come from the cache; - the link fails on `f_` symbols, and "20" is just lld's default `--error-limit`. Nothing stops two builds on one work folder on Linux, because `stop_stray_compilers` acts on Windows only. A second build can come from another builder window, the game's update offer (`--update`) or the command line. I couldn't confirm that two builds actually overlapped on the reporter's PC; the lift section of their first v2.0.5 log would tell (no `chunks: code_0NN.c (… MB) in N parts` lines). **Why it never recovers (shown).** `write_lift_key` vouched for the broken C, and every later build has the same key, so `lifted_before` skipped the lift. Clearing both object caches couldn't help, because the broken C is never regenerated. **The change** - `make_game.undefined_functions(gen)` lists the functions `functions.txt` names that no `code_*.c` defines. It uses `lifted_c.DEF_BYTES`, which matches from the preceding newline and is about 10× faster than `HEAD_BYTES` (0.07 s vs 0.83 s on 181 MB). - After the lift, if anything is missing, the build stops with a clear message and writes no key, so the next build translates again. - `lifted_before` also requires that nothing is missing, so a folder already broken this way, like the reporter's, recovers by itself on the next build. This adds a scan of the lifted C (about 1.4 GB) to a build that skips the lift, roughly a second or two. - On POSIX, `lock_work` takes a non-blocking `flock` on `<work>/.build_lock` for the life of `make_game`, and a second build on the same work folder stops with a message. Python's fds are non-inheritable, so compilers don't hold the lock. **Tests.** `tests/test_lift_complete.py` is game-free: it uses the real `chunks.write`, `Lifter.write` and `split_hot`, with no native translator needed. It: - reproduces the overlap (235 functions lost in that run); - checks that the loss is found and that `lifted_before` refuses the folder; - checks that a lift on its own restores it; - checks that the lock refuses a second build. Against main's `make_game.py` it fails (no `undefined_functions`); on this branch it prints `ok`. `tests/run_all.py lift make_game chunk` gives 4 passed. An earlier run of related checks (`test_parallel_emit`, `test_mem_gate`, `test_build_log`, `test_finish_output`, `test_build_times` …) gave 11 passed. **Not verified.** I haven't run a real game build. The reporter's trigger (two builds at once) is inferred. The detection and self-healing don't depend on it: any lift that leaves a listed function undefined is no longer reused. **Workaround for the reporter now:** delete `<work>/sa/gen/.lift_key` (or `sa/gen`), then build once with no other build running.
amaralthalesalves commented 2026-11-09 02:10:49 +00:00 (Migrated from github.com)

Moving this to draft: the cause is inferred from the reporters' logs, and I haven't reproduced it on their setup or tested it with a real game build. The change and its game-free test hold as described. It needs confirmation from someone who can build the game (ideally a reporter of the linked issue) before it's ready.

Moving this to draft: the cause is inferred from the reporters' logs, and I haven't reproduced it on their setup or tested it with a real game build. The change and its game-free test hold as described. It needs confirmation from someone who can build the game (ideally a reporter of the linked issue) before it's ready.
amaralthalesalves commented 2026-11-09 10:08:37 +00:00 (Migrated from github.com)

Rebased on current main (8b2e1d3). There were no conflicts, and test_lift_complete plus the lift / make_game / chunk / cache tests pass. Since #55abe59 deletes sa/ after a build by default, the extra undefined_functions scan in lifted_before only runs when HOOKSHOT_KEEP_BUILD_FILES=1. The check right after the lift is unchanged. It's still a draft until someone confirms it with a real build.

Rebased on current main (8b2e1d3). There were no conflicts, and `test_lift_complete` plus the lift / make_game / chunk / cache tests pass. Since #55abe59 deletes `sa/` after a build by default, the extra `undefined_functions` scan in `lifted_before` only runs when `HOOKSHOT_KEEP_BUILD_FILES=1`. The check right after the lift is unchanged. It's still a draft until someone confirms it with a real build.
amaralthalesalves commented 2026-11-09 10:15:11 +00:00 (Migrated from github.com)

About the red "Python source checks" here: the only failure is tests/test_program_answer_domains.py (use of undeclared identifier 'answers_in' in its C++ fixture). It fails the same way on main at 8b2e1d3, run 42588468881, on both Linux and Windows. This PR only touches nx2/make_game.py, nx2/lifted_c.py and tests/test_lift_complete.py, which passes. The other 129 checks pass.

About the red "Python source checks" here: the only failure is `tests/test_program_answer_domains.py` (`use of undeclared identifier 'answers_in'` in its C++ fixture). It fails the same way on main at 8b2e1d3, run 42588468881, on both Linux and Windows. This PR only touches `nx2/make_game.py`, `nx2/lifted_c.py` and `tests/test_lift_complete.py`, which passes. The other 129 checks pass.
NotRexed commented 2026-11-10 02:08:16 +00:00 (Migrated from github.com)

Superseded since v2.0.10: a failed compile or link now forgets the lift, and a new lift clears the earlier lifted C first (issues 1176 and 1197), so a lift that lost functions is not reused. This draft also fails CI. Closing; thanks for tracking #890 down!

Superseded since v2.0.10: a failed compile or link now forgets the lift, and a new lift clears the earlier lifted C first (issues 1176 and 1197), so a lift that lost functions is not reused. This draft also fails CI. Closing; thanks for tracking #890 down!
Commenting is not possible because the repository is archived.
No description provided.