28× faster reset + complete snapshot capture (fixes goto_depth+snapshot SIGSEGV) - #21
Merged
Conversation
RawEngine.start() previously mkdtemp'd + copytree'd the entire ~3.7MB dat tree on every reset (~29ms, ~85% of the 35ms reset cost) just to give the engine a writable hackdir. The dat files are read-only game data the engine only ever opens O_RDONLY (confirmed via strace), so copying them per reset was pure waste. Now the binding passes the shared source dat as settings.datadir (read directly, never copied) and creates only a tiny per-env writable hackdir holding the four writable templates (record/logfile/xlogfile/perm); the engine writes level/save/ bones/lock files there. The hackdir is reused across resets and scrubbed back to those templates on each start. Result: reset/reroll 35ms -> ~1.3ms. Behavior is byte-identical (the C change is a no-op when datadir is empty; verified identical game traces across 30 seeds x 200 steps old-vs-new, plus 20/20 reuse-vs-fresh trace identity). The remaining ~0.9ms is nle_start's actual game generation. Bumps the NetHack submodule to 121ddf5 (the datadir prefix split). Adds test_reset_speedup.py (behavior identity + tiny-hackdir + speed guard) and updates test_snapshot_multilevel for the templates-now-in-datadir layout.
…nce dump Binds nle_dbg_memmap as RawEngine.memmap(path): dumps the per-env arena layout (named buffers by offset, fmon/fobj chains, monster grid with fmon-membership and data-validity) to a file. A reusable diagnostic for arena-reuse / dangling- pointer / snapshot-corruption debugging. Includes docs/arena_memory_map.txt, a reference dump from a normal game showing the fixed buffer layout (tcap..muse_m, 0..~255KB), the level struct region (~194-255KB), and the dynamic monster/object/string allocations above it. Bumps the NetHack submodule to c074f62 (nle_dbg_memmap).
Renames docs/arena_memory_map.txt -> docs/game_memory_map.txt and regenerates it from the extended nle_dbg_memmap: now leads with the whole-game region table (nle_ctx_t struct, arena, coroutine stack, rl mirror) before the arena buffer/chain/grid detail. Bumps the NetHack submodule to dcaaff4.
…ession test Bumps the NetHack submodule to e7197b4, which makes nle_fr_snapshot capture all per-env state: arena-allocates nle_pline_state (fixes the dangling-you_buf SIGSEGV during snapshot/restore/reseed combat) and the ~20 other lazy per-env *_state structs, and serializes the rl mirror's inventory_ + WIN_MESSAGE last_msg (fixes inv_*/message observation divergence after a restore). Adds tests/test_snapshot_completeness.py: a differential invariant test (snapshot -> divergent reseeded branches -> restore -> replay fixed line == the first replay). Validated far wider out of band: 40 seeds x 30 rounds, 0 crashes and 0 divergence; forward-game traces byte-identical pre/post (behavior unchanged).
liujonathan24
force-pushed
the
engine-reset-speedup
branch
from
June 25, 2026 08:07
9196321 to
d8bf259
Compare
Owner
Author
|
Depends on the engine-side PR liujonathan24/NetHack#5 (this PR's submodule pointer |
The vision_radius tune knob set u.nv_range unclamped; a large value (e.g. 1e9, or inf) indexed circle_data[] out of bounds via circle_ptr() -> SIGSEGV in vision_recalc at game start (and vr*vr overflowed int in the sight-limit path). Now clamped to [1, MAX_RADIUS]. Found by fuzzing every tune knob with extreme values; vision_radius was the only one that crashed.
liujonathan24
added a commit
that referenced
this pull request
Jun 26, 2026
…#22) Re-bumps the engine submodule from 1ff9e00 to fork main 71f0be3, which now includes the include/nleobs.h header sync (NetHack#6). That fix landed on the fork after PR #21 was merged at 1ff9e00, so harness main was left pointing at the pre-fix engine where the top-level include/nleobs.h still lacked the datadir + tune fields (an ABI mismatch for external consumers building against include/nleobs.h).
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
1 participant
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
Two engine wins, both verified behavior-preserving for the forward game:
1. Fixes the snapshot/restore monster-corruption SIGSEGV
nle_pline_state(pline.c) wascalloc'd on the libc heap — outside the per-env arena, sonle_fr_snapshotnever captured it — yet it holds_you_buf, an arena pointer. After arestorerewinds the arena, that stale pointer aliases a reused arena offset, and the nextYou_hear/plinewrites its message string over whatever now lives there (observed: a live monster'sdatafield) → SIGSEGV inpassivemmduring pet combat. Reproduced deterministically via snapshot/restore/reseed branching aftermodify(goto_depth).Fixed the whole class: arena-allocate
nle_pline_stateplus ~20 other lazy per-env*_statestructs, and serialize the rl display mirror's cachedinventory_+WIN_MESSAGElast_msginNetHackRL::save_mirror/load_mirror. These were the remaining per-env state that escaped the snapshot and leaked the previous branch's state through a restore (inv_*/message/glyphs/blstats observation divergence).2. ~28× faster reset/reroll
RawEngine.start()copytree'd the entire ~3.7MB read-onlydattree into a fresh temp dir on every reset (~29ms, ~85% of reset cost). The engine only ever reads that dataO_RDONLY. Added adatadirtonle_settingsso read-only prefixes resolve to a shared source dir and only a tiny per-env writablehackdiris created. 35ms → ~1.3ms.Plus a reusable arena memory-map debug tool (
RawEngine.memmap()+docs/game_memory_map.txt) that was instrumental in root-causing the corruption.Verification (behavior unchanged)
test_snapshot_completeness.py.Submodule
Bumps
third_party/NetHackto the fork branchengine-snapshot-completeness(4 commits: datadir split, memmap tool, whole-game map, snapshot completeness).Known separate follow-up (pre-existing, not a regression)
A deeper crash surfaces only at ≥60 rounds of the aggressive differential pattern (hundreds of snapshot/restore cycles): SIGSEGV in
jump_fcontext(coroutine resume to a corrupt context), independent of depth/goto_depth/reset. Confirmed pre-existing (crashes with the mirror change reverted) — was masked before by the round-4 crash. Tracked for separate investigation.