Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core: Fix UI slowdown for savestate timestamp reads #12256

Merged

Conversation

malleoz
Copy link
Contributor

@malleoz malleoz commented Oct 26, 2023

Special thanks to @TryTwo for identifying this regression.

On core state changes (Play/Pause for example), Dolphin reads the headers of all savestates associated with the currently running game. Normally this just involves reading the state header, which is uncompressed, thus reads are quick and simple. Changes introduced in #12217 change the behavior when calling ReadHeader() in State.cpp, whose aim is to just get each state's timestamp. Instead of reading just the legacy header, ReadStateHeaderFromFile() reads what we consider to be the NEW header, which includes version information. To support legacy savestate version checks, we decompress the state using LZ4 to retrieve the version info to copy over to our new header. However, this decompression does not need to occur if we simply wish to read timestamps. If you have several legacy LZ4 states in your Savestate folder, this would cause several decompressions to happen. Toggling play/pause several times in quick succession will lead to the UI hanging.

To fix this I can just introduce a boolean for ReadStateHeaderFromFile() to exit out if we only need the legacy portion of the header to be read. This fixes the issue for me and seems safe as the callers to ReadHeader() solely reference the timestamp field.

@TryTwo
Copy link
Contributor

TryTwo commented Oct 26, 2023

Tested and fixes the issues I was having. Good job on finding a simple fix.

@malleoz
Copy link
Contributor Author

malleoz commented Oct 27, 2023

Updated to reflect the earlier discussion. Marking as resolved for now, unless anyone has additional qualms with the current changes.

@malleoz
Copy link
Contributor Author

malleoz commented Oct 27, 2023

Oops linter!

@AdmiralCurtiss AdmiralCurtiss merged commit 045868b into dolphin-emu:master Oct 30, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants