Stop autosave from stalling the game at high speed - #260
Conversation
At 8x-40x the game froze for a fraction of a second every second or two. Every 256 ticks GameGUI::syncStep wrote the whole game, including every cached pathfinding gradient, to Auto_save.game. Late-game saves are 65-120 MB, and at 8x 256 ticks is about 1.3 seconds. - Write gradient fields as one run of bytes through OutputStream::writeUint16Sections. The bytes and SHA1 are the same as per-value writes, without a virtual call, name string and SHA1 update per value. - Give the atomic autosave writer a 1 MiB buffer instead of 16 KiB. - Skip the whole-file SHA1 for autosaves; the header keeps zeros. Engine::haveMap, the hash's only reader, now fetches the host's copy of any file without a hash instead of trusting a local file. - Scale the autosave interval with the game-speed preset so saves stay about 10 seconds of real time apart. Normal speed keeps ticks 79, 335, and so on. Counting from the last save also stops a soft-paused game on a save tick from autosaving every frame. Simulation checksums and replays are unchanged. At normal speed the autosave differs from master's only in its 20 SHA1 bytes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPPkz7Vd7QLMsbQfqy3kHU
Autosave serialized the game and wrote it to disk on the game thread. At late-game sizes (65-160 MB) the write alone took 30-70 ms, with spikes past 180 ms, on every autosave. - Serialize between ticks into memory, then hand the bytes to a new GAGCore::BackgroundFileWriter, which replaces the file through FileManager::writeAtomically on a worker thread. A snapshot that has not started writing is replaced by a newer one. The worker exists only while there is something to write, so waiting leaves no thread behind and a forked process never inherits one. - MemoryStreamBackend appends past its end instead of resizing, which zero-filled every byte before writing it, and gains reserve() and takeContents() so the snapshot moves to the writer without a copy. - Wait for a pending autosave before a session ends, before an in-game save, and when GameGUI is destroyed. - Add Settings > Gameplay > Autosave (autosaveGames, on by default), translated for every language. - Autosave now hands writeAtomically one complete buffer, which bypasses the stream buffer, so the 1 MiB atomic-write buffer from the previous commit no longer has any effect; revert it. Saved bytes, simulation checksums and replays are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPPkz7Vd7QLMsbQfqy3kHU
Two append/append conflicts: independent constant blocks in EngineTiming.h (autosave cadence here, gradient rebuild interval on master) and a duplicate Version.h include in SavegameSafetyHarness.cpp. Kept both constant blocks; kept the single existing Version.h include. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A5GEG5t9ithd47SFm6PTDZ
|
Merged current master ( Two files conflicted, both append/append:
No production logic was touched by the resolution. Verified locally on the merged head (macOS / Apple M3) before pushing:
CI is running on |
|
If savegames are up to 160MB, maybe we should save them differently? In a perfect world we could store replays with a timestamp but of course, every logic change breaks such savegames. We could also decide that gradients don't get saved and get computed first thing loading a savegame. That's probably besides the point of this PR but it would be the simpler change.
Why not compute the hash instead of downloading the map? It's 200ms to compute a 160MB hash but uploads from the host to 3 players ... takes much longer.
If that happens, something stops writing the savegame within 10s, so it might happen continuously. What happens if the prior has started writing but not ended? Will the next one skip and we drop to every 20s? I guess that would be a fair logic and probably never trigger.
Can't the thread saving the game store that hash? It's literally dirt cheap. If you threw away data needed to hash, it's ok to store a different hash as obviously the data is not needed to play the game. Just to verify it a map was distributed already, so the recipient should be able to hash and verify anyway ...
NACK |
At 8x, 13x and 40x the game runs smoothly for a second or two, then freezes for a fraction of a second, over and over. The freeze is the autosave. Every 256 ticks
GameGUI::syncStepserialized the whole game, including every cached pathfinding gradient, and wrote it toAuto_save.gameon the game thread. Late-game saves reach 65–160 MB, and at 8x 256 ticks is only about 1.3 seconds.Changes
OutputStream::writeUint16Sections. Each value used to cost three virtual calls, a name string the binary stream ignores, and its own 2-byte SHA1 update. The output bytes and SHA1 are identical;TextOutputStreamkeeps the per-value loop.Engine::haveMap, when a client joins a YOG game. It now treats a file without a hash as unverifiable and downloads the host's copy, instead of trusting a local file with the same name. Manual saves, maps and the replay header still hash.GAGCore::BackgroundFileWriterthen replaces the file throughFileManager::writeAtomicallyon a worker thread.GameGUIis destroyed.MemoryStreamBackendappends past its end instead of resizing, which zero-filled every byte before writing it. It also gainsreserve()andtakeContents(), so the snapshot moves to the writer without a copy.autosaveGames, on by default), translated for every language.For review: behavior changes
Compatibility
gd-bigarena-longfor 20,000 ticks ends on master's checksum,5abcb33f. A 3,000-tick run ends on2a2689edwith autosave on and off.Auto_save.gameis byte-identical to the one this PR's previous commit wrote synchronously.Known issue, not fixed here
MapHeader::operator==has compared the SHA1 inverted since 8cf81c9 (2008):std::equal(...)==0is true only when the hashes differ. SohaveMapre-downloads identical files and trusts a different file with the same name, team count and map offset. I'll fix that in a separate PR with a two-client join test.Verification
All results are from this branch's final tree.
BufferedFileStreamHarnesspasses. It now also checks thatMemoryStreamBackendoverwrites, appends, zero-fills seek gaps and hands over its contents.SavegameSafetyHarnesspasses, with new checks:test/run-settings-tests.pyandtest/run-game-speed-tests.py --settings-onlypass.SettingsScreenTestchecks that the Autosave toggle persists off and back on.data/check_translations.py --strictgives the same report as before this change, andtest/test_translations.pypasses.scons -C testbuilds, andTestsRunner(187 tests),WinningConditionsHarnessandReplayStepCounterTestpass.Test games
games/gd-bigarena-long.game: Oazis, 256×256, 11 Castor/Warrush teams.GLOB2_TEST_SEED=7 glob2 -test-games-nox 1 --map Oazis --matchup nicowar,econo,nicowar,econo,nicowar,econo,nicowar,econo,nicowar,econo,castor --save-game-as <file>How the runs were done: release build,
glob2 --nox <game> <ticks> 1, with a disposable HOME whosepreferences.txtsetsgameSpeedand, for the on/off runs,autosaveGames.Game-thread stall per autosave
Method:
Limits
Auto_save.gamesurvives, because the file is only replaced by rename.test/run-settings-tests.py, which covers the new toggle. It passed locally.🤖 Generated with Claude Code
https://claude.ai/code/session_01LPPkz7Vd7QLMsbQfqy3kHU