Repository navigation
release: engine 4.2.0-rc.3 with WASM build performance work - #927
Merged
Merged
Conversation
…tions (#921) (#926) * test(save): pin serializer output with a golden save fixture (#921) Captures the exact bytes `serializeGameState` writes today for a fixture world covering registry components (scalars, enums, optionals, nested structs, EnumSet), pad and field-less markers, transient and unregistered components, entity refs, multi-component ref arrays and every built-in channel (Position, Parent, PrefabInstance, PrefabChild, Tilemap with layer bindings, Camera with viewport/tag, scene name) with JSON escaping. Also loads that golden save back as an existing save, sweeps every allocation failure for clean error propagation without leaks, and pins the empty-world output. Committed before the #921 serializer split so the golden is provably the pre-change output. Claude-Session: https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS * perf(save): split the save serializer into bounded per-component functions (#921) `serializeGameState` walked the ComponentRegistry three times with `inline for` bodies, so every registered component's entity collection, serde write and ref-array write was expanded into one function. In Flying Platform that function reached ~523 KB, and LLVM's super-linear passes over it dominated the ReleaseSafe compile. Move the three per-component bodies into `noinline` helpers (`collectComponentEntities`, `writeRegisteredComponent`, `writeComponentRefArrays`), instantiated once per component type, and fold the four copies of the entity dedup into `rememberEntity`. The helpers execute the same statements in the same order, so the save bytes, error propagation (`try` through every helper), cleanup (`defer` of the per-component temporary list stays in its helper) and safety checks are unchanged; the golden test added in the previous commit passes unmodified. Claude-Session: https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS * tools(save): add the #921 serializer compile-cost harness and results `measure.py` compiles a game's generated WASM `game` library with a given save.zig dropped into its vendored engine copy (restoring it afterwards), samples the compiler process tree for wall time and peak private memory, and lists every save-mixin function size from `llvm-nm -S`. `run_ab.sh` drives two interleaved baseline/outlined runs under a shared-machine lock. results/ holds the Flying Platform runs behind the PR table (ReleaseSafe, -j2, Windows, Zig 0.16.0): the game compile drops from ~222 s / 10.2 GiB peak private to ~142 s / 5.6 GiB, and serializeGameState from 523,186 to 58,476 bytes. Claude-Session: https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS * tools(save): fail the A/B driver on a failed run, release its lock on interrupt Review of #926: - run_ab.sh printed a failed measure.py status and then returned the status of the cleanup that followed, so a failed measurement still exited 0. It now counts failures, still attempts all four runs, and exits 1 if any failed. - An INT/TERM/HUP after taking the heavy lock left the lock directory behind. The driver now records that it owns the lock, and an EXIT trap stops the running measurement and releases only that lock. The measurement runs in the background and is waited on, so a signal is handled immediately rather than after the compile. - Stopping the measurement uses the new `measure.py --abort PID`, which kills the PID's process tree (the zig compilers) and restores the game files the run edited. Those originals are now also kept on disk as `<file>.ab-backup` until restored, because under Git Bash a plain `kill` hard-terminates the native python, so its `finally` never runs and the compilers are orphaned. A later run also restores stale backups. Checked with stub compilers (no game build): a failing compile gives `4 of 4 measurements failed` and exit 1, with files restored and the lock free. A TERM mid-compile gives exit 143, both stub processes gone, files restored, no backups left and the lock free. A lock owned by someone else is never removed. shellcheck and pyflakes are clean. Claude-Session: https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS (cherry picked from commit 7e804d4)
…save loader (#922) (#925) * test(save): pin serializer output with a golden save fixture (#921) Captures the exact bytes `serializeGameState` writes today for a fixture world covering registry components (scalars, enums, optionals, nested structs, EnumSet), pad and field-less markers, transient and unregistered components, entity refs, multi-component ref arrays and every built-in channel (Position, Parent, PrefabInstance, PrefabChild, Tilemap with layer bindings, Camera with viewport/tag, scene name) with JSON escaping. Also loads that golden save back as an existing save, sweeps every allocation failure for clean error propagation without leaks, and pins the empty-world output. Committed before the #921 serializer split so the golden is provably the pre-change output. Claude-Session: https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS * perf(save): split the save serializer into bounded per-component functions (#921) `serializeGameState` walked the ComponentRegistry three times with `inline for` bodies, so every registered component's entity collection, serde write and ref-array write was expanded into one function. In Flying Platform that function reached ~523 KB, and LLVM's super-linear passes over it dominated the ReleaseSafe compile. Move the three per-component bodies into `noinline` helpers (`collectComponentEntities`, `writeRegisteredComponent`, `writeComponentRefArrays`), instantiated once per component type, and fold the four copies of the entity dedup into `rememberEntity`. The helpers execute the same statements in the same order, so the save bytes, error propagation (`try` through every helper), cleanup (`defer` of the per-component temporary list stays in its helper) and safety checks are unchanged; the golden test added in the previous commit passes unmodified. Claude-Session: https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS * test(save): pin what an existing save loads into (#922) Before splitting the loader, add a compatibility fixture: a hand-written v2 save that exercises every channel `deserializeGameState` reads (sparse, out-of-order ids; entity refs; ref arrays on two components, including an id with no saved entity; postLoad; saveable and transient post_load_add markers; post_load_create; the built-in Parent, PrefabInstance, PrefabChild, Tilemap with bindings and Camera without zoom) plus malformed entries the loader must skip without failing. The world it loads into is pinned by its re-serialization, captured with the current loader (7ea1d13), and the postLoad / marker / create effects and save -> load -> save stability are asserted directly. The serializer golden test from #921 already covers loading a save written by the pre-#921 serializer. An allocation-failure sweep like the serializer's is not possible here: the load resets the ECS, and the mock ECS panics on OOM by design. Claude-Session: https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS * perf(save): split the save loader into bounded per-component functions (#922) `deserializeGameState` walked the ComponentRegistry with `inline for` bodies in Phase 2 (JSON read + entity-ref remap + addComponent), Step 4 (ref arrays) and Step 6a/6b/6c (postLoad, post_load_add markers, post_load_create), so every registered component's code was expanded into one function. Together with the built-in restores it reached ~133 KB in Flying Platform's ReleaseSafe WASM build (3rd largest). Move each per-component body into a `noinline` helper instantiated once per type (`restoreRegisteredComponent`, `restoreComponentRefArrays`, `runComponentPostLoad`, `addPostLoadMarkers`, `createPostLoadEntity`) and each built-in restore (Parent, PrefabInstance, PrefabChild, Tilemap, Camera) into its own bounded function, called in the same order. The helpers run the same statements: `try` still propagates every error, the per-component temporary lists are still freed by `defer`, and the "skip malformed input" contract is unchanged (`break :blk` became `return`). The compatibility test from the previous commit and #921's golden test pass unmodified. Claude-Session: https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS * perf(scene): keep per-scene registration out of the generated main (#922) `registerScene` takes the scene name and loader as comptime parameters, so it is instantiated once per scene, and the generated `main` calls each instantiation exactly once. LLVM therefore inlined every one of them - hash-map insert, growth path and the #697 error log - into `main`. Flying Platform registers ~136 scenes, which made its startup `main.main` the largest function in the ReleaseSafe WASM build: ~335 KB, of which ~281 KB was inlined `registerScene` bodies and ~25 KB inlined scene-map lookups from `setSceneSource` / `setSceneAssets` (attributed with llvm-dwarfdump inlined-subroutine ranges). Keep the comptime shim (it still builds the per-scene loader wrapper) but do the insert in one shared `noinline putSceneEntry`, and route the per-scene setters' lookup through one `noinline sceneEntryPtr`. The calls the generated main makes, their order, the stored entries and the error log are unchanged, so no assembler template change is needed. `test/scene_registration_test.zig` replays the generated startup sequence: every scene dispatches to its own loader, the setters attach to the right entry, unknown names still fail with SceneNotFound, and re-registration replaces the loader. Claude-Session: https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS * tools: reproducible profile of oversized WASM functions + #922 results `tools/wasm_function_profile/` is the script set used for #922 on Flying Platform's ReleaseSafe threaded WASM build: - profile.sh: one variant, under the shared heavy-build lock: the game Zcu alone with `--time-report` (LLVM pass timing, sema/codegen), an edit-rebuild compile without it (wall time, peak private memory), and the full build (emcc link, final .wasm size). - profile_run.py / read_timing.py: process-tree memory sampling and the build web UI's time report. - symbols.py: per-function code size (llvm-nm -S), top N and clusters. - dwarf_inline.py: attributes a function's bytes to the calls LLVM inlined into it (DW_TAG_inlined_subroutine ranges) - this is how the startup main's 281 KB were traced to `registerScene`. - sequence.sh / summarize.py: alternating base/after samples and the before/after table (results/summary.md). Paths default to the Windows measurement machine and can be overridden (PY, ZIG, EMSDK, ZIG_LOCAL_CACHE_DIR). Claude-Session: https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS (cherry picked from commit be0991e)
Carries the save serializer split (#926) and the save loader / startup scene registration outlining (#925) onto the v4.2.0-rc.2 endpoint integration baseline used by Flying Platform (labelle-cli#528). Claude-Session: https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
Author
|
CI passed on 1a6c9e9. That commit is tagged as I left this PR open and unmerged. rc.2 was squash-merged into |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prepares labelle-engine v4.2.0-rc.3 on the v4.2.0-rc.2 endpoint-integration line (
feat/watched-navigation-inputs) so Flying Platform, which pins 4.2.0-rc.2, can adopt the WASM build performance work. Epic: labelle-toolkit/labelle-cli#528.Commits on top of
v4.2.0-rc.2(1d0eaca):-x) of perf(save): split the save serializer into bounded per-component functions (#921) #926 (7e804d4): split the save serializer into bounded per-component functions (Split the save serializer into bounded functions to cut WASM compile time/memory (labelle-cli#528 WP2a) #921)-x) of perf: shrink oversized WASM functions — startup scene registration + save loader (#922) #925 (be0991e): outline startup scene registration and the save loader, plus tests and profiling tools (Profile and shrink remaining oversized WASM functions: startup, inspector, deserialization (labelle-cli#528 WP2b) #922)build.zig.zonversion4.2.0-rc.2->4.2.0-rc.3(same shape as the rc.1/rc.2 release commits)Both cherry-picks applied cleanly, with no conflicts. This PR is opened so CI runs on the exact commit that will be tagged
v4.2.0-rc.3.Measured on Flying Platform WASM:
Refs #921 #922 #926 #925, labelle-toolkit/labelle-cli#528
https://claude.ai/code/session_01Hpt1haxcboZPznpG1KqDRS