fix: run correctly on modern Emscripten + release-grade performance flags (~15x less CPU)#14
Open
zackarychapple wants to merge 4 commits into
Open
fix: run correctly on modern Emscripten + release-grade performance flags (~15x less CPU)#14zackarychapple wants to merge 4 commits into
zackarychapple wants to merge 4 commits into
Conversation
doomtype.h defined boolean as a 4-byte enum in TUs that had not yet included <stdbool.h> and as 1-byte bool in TUs that had (via SDL headers). The same globals (netgame, etc.) were therefore written with different store sizes depending on the writer's TU; 4-byte stores clobbered the 3 adjacent bytes, corrupting playeringame[], the deathmatch flag, and renderer view state (sheared/garbage 3D view, phantom 'Player 4 left the game' at spawn). Caught with an Emscripten -fsanitize=address build as a global-buffer-overflow WRITE on netgame. spriteframe_t.rotate becomes int: R_InstallSpriteLump memsets it to -1 as an uninitialized marker and compares against it, which a 1-byte bool cannot represent (R_InitSprites aborted with 'frame I has rotations and a rot=0 lump' once boolean shrank). Note: struct layout changes make old savegames/demos incompatible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lity Remove debug-only flags that shipped in the release build and carried a large runtime cost: SAFE_HEAP=1 (checks every memory access), STACK_OVERFLOW_CHECK=1, -g/-gsource-map/--source-map-base. Add --closure 1 to minify the JS glue. Toolchain compatibility (brew Emscripten 6.0.2 / clang 21): - -std=gnu99: modern clang defaults to C23 where true/false are keywords, colliding with the boolean enum in doomtype.h. - -fno-strict-aliasing: the id-era code type-puns byte buffers through int16/int32 pointers everywhere; do not let -O3 exploit that UB. - STACK_SIZE=5MB: restores the pre-3.1.25 default; the modern 64KB default overflows during init with no checks enabled. - EXPORTED_RUNTIME_METHODS / INITIAL_MEMORY: renamed settings. - Do NOT add -flto: at -O3 it compiles UB in this codebase into an unconditional trap in main() (verified: crashes right after Z_Init). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The rAF-driven main loop (60-120Hz) outpaces the 35Hz tic rate, so the TryRunTics wait loop and the screen-wipe loop hit I_Sleep(1) -> emscripten_sleep() on almost every frame. Under Asyncify each such sleep unwinds and rewinds the entire wasm call stack. Returning to the browser event loop instead serves the same purpose (the main loop re-enters next frame) without the per-frame unwind cost. Asyncify itself stays: startup and netgame paths still block in emscripten_sleep(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- FS.createPreloadedFile was renamed; use Module.FS_preloadFile. - callMain is no longer exported; let INVOKE_RUN start main() once preloaded files are ready via Module.arguments. - Surface wasm crash stacks via unhandledrejection/error handlers; async-runtime errors were otherwise swallowed without a trace. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Summary
This port no longer builds or runs correctly on current Emscripten (tested with 6.0.2): compilation fails under the new clang's C23 default, and once that is worked around, a latent ABI bug corrupts memory at runtime (garbled 3D view, phantom "Player 4 left the game" messages, wrong HUD). This PR fixes both, migrates
index.htmlto the current runtime APIs, and replaces the debug-grade build flags that were shipping in release builds.Correctness
booleanwas a different size in different translation units (src/doomtype.h): TUs that had already included<stdbool.h>(via SDL headers) got a 1-bytebool; the rest got a 4-byte enum. Four-byte stores to globals likenetgameclobbered the three adjacent bytes — corruptingplayeringame[], the deathmatch flag, and renderer view state. Found with an Emscripten-fsanitize=addressbuild (global-buffer-overflow WRITE onnetgame).booleanis now unconditionallybool, andspriteframe_t.rotatebecomesint(it usesmemset(-1)as an uninitialized marker, which a 1-byte bool can't hold). Note: this changes struct layouts, so old savegames/demo recordings are incompatible.-std=gnu99: modern clang defaults to C23, wheretrue/falseare keywords and collide with the boolean enum.-fno-strict-aliasing: the id-era code type-puns byte buffers throughint16/int32pointers; don't let-O3exploit that UB.STACK_SIZE=5MB: Emscripten's default shrank from 5 MB to 64 KB in 3.1.25; Doom's init path overflows the new default.index.html:FS.createPreloadedFile→Module.FS_preloadFile, andModule.argumentsinstead of the no-longer-exportedcallMain; added handlers so wasm crash stacks reach the console instead of vanishing as unhandled rejections.Performance
SAFE_HEAP=1(instruments every memory access),STACK_OVERFLOW_CHECK=1,-g/-gsource-map. Added--closure 1for the JS glue.src/d_loop.c,src/doom/d_main.c): the rAF main loop (60–120 Hz) outpaces the 35 Hz tic rate, soTryRunTicsand the screen-wipe loop hitemscripten_sleep(1)on almost every frame — a full Asyncify stack unwind/rewind each time. They now return to the browser event loop instead; Asyncify stays for startup/netgame paths.-flto: at-O3it compiles UB in this codebase into an unconditional trap inmain()(verified; documented inconfigure.ac).Before / after
Both builds use the same source (including the correctness fixes — the original code can't run at all on this toolchain) and the same Emscripten 6.0.2; "before" uses the original flag set, "after" this PR's. Measured on an Apple-Silicon Mac, headless Chrome, shareware
doom1.wad.-timedemo demo1(5198 gametics, display-capped)The CPU numbers are the meaningful ones: both builds are display-cap-bound on a fast desktop, but the before-build burns ~15x the CPU for the same frames — the gap that shows up as dropped frames and battery drain on phones and slower machines.
Manual Testing
./scripts/clean.sh && ./scripts/build.sh(brewemscripten6.0.2 +automake).doom1.wadintosrc/, thencd src && python3 -m http.server 8000.-fsanitize=addressbuild running clean.🤖 Generated with Claude Code
Deployed and playable at https://t-web-latest-zackary-chapple-doom-wasm-doom-wasm-zack-ef80f1-ze.zephyrcloud.app/