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.

Sign up for GitHub

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

Jump to bottom

[TESTERS NEEDED!] Savestates: Compressed files #14822

Merged
merged 5 commits into from
Nov 27, 2023
Merged

[TESTERS NEEDED!] Savestates: Compressed files #14822

merged 5 commits into from
Nov 27, 2023

Conversation

elad335
Copy link
Contributor

@elad335 elad335 commented Nov 15, 2023 •
edited
Loading

Along with #14784, this pull requests aims to reduce significantly the file size of savestates which can anywhere from few hundreds of MBs to few GBs or even more.
Initial measurements suggest size reduction of more than 50 percent! But it is probably more in most cases, need user testing!

TODO:

  • Make it compatible with TAR serialization.
  • Make TAR loader RAM efficient as well, it was found that the previous code loads entire files on RAM when reading TAR entries, so this will get fixed here.
  • Test it.
  • Savestates: Implement initial RAM ventilation system #14784 also allows for asynchronous savestate file writes when I think about it, this would would increase the speed of saving and is now targeted for this pr. Note: postponed to another pull request.

Additional bugfixes:

  • Fix missing TAR last entry in savestate in some cases.
  • Fix a bug in saved HLE state in sys_lwcond/sys_lwmutex/sys_mutex/sys_cond syscalls.

@elad335 elad335 added Optimization Optimizes existing code Savestates Anything that involves savestates labels Nov 15, 2023
@elad335 elad335 added the Loader Involving the load of PS3 file formats label Nov 15, 2023
@mirh
Copy link

mirh commented Nov 16, 2023

I don't want to start a codec war, but certainly in this day and age there are better alternatives than zlib?
(yes, including also for performance if that was the most paramount concern)

@elad335
Copy link
Contributor Author

elad335 commented Nov 16, 2023 •
edited
Loading

Okay. I only have one requirement for them: it needs to be able to compress data chunks with the ability to perform well even if not all data is provided at a single time.

@mirh
Copy link

mirh commented Nov 16, 2023

I'm going out on a limb, and guess that just like in every other case ever you can check xz/lzma2 for usually the best compression, and then zstd or brotli for some slight compromise with speed.

@elad335 elad335 force-pushed the save-breathe branch 5 times, most recently from f4f35b0 to 57cde7b Compare November 22, 2023 16:48
@elad335 elad335 force-pushed the save-breathe branch 3 times, most recently from 87640f6 to c85974c Compare November 23, 2023 10:46
@elad335
Copy link
Contributor Author

elad335 commented Nov 23, 2023

56% compression rate and the file is valid, still RPCS3 won't load it but it is work in progress.
image

@elad335 elad335 force-pushed the save-breathe branch 3 times, most recently from 7239e00 to d276ef4 Compare November 23, 2023 19:06
@elad335 elad335 marked this pull request as ready for review November 23, 2023 19:07
@elad335 elad335 changed the title [WIP] Savestates: Compressed files [WIP][TESTERS NEEDED!] Savestates: Compressed files Nov 23, 2023
@elad335
Copy link
Contributor Author

elad335 commented Nov 23, 2023

Applied some bugfixes, loading of files is also starting to work!

@elad335 elad335 force-pushed the save-breathe branch 5 times, most recently from 45f23e5 to 6fd0648 Compare November 24, 2023 13:55
@elad335 elad335 changed the title [WIP][TESTERS NEEDED!] Savestates: Compressed files [TESTERS NEEDED!] Savestates: Compressed files Nov 24, 2023
@elad335 elad335 force-pushed the save-breathe branch 3 times, most recently from 874c83d to 494bc72 Compare November 25, 2023 07:37
Megamouse
Megamouse reviewed Nov 25, 2023
View reviewed changes
rpcs3/Emu/System.cpp Outdated
auto on_select = [](u32, spu_thread& spu)
const bool autostart = !!g_cfg.misc.autostart;

static const auto cpu_op = [](atomic_bs_t<cpu_flag>& state, bs_t<cpu_flag> add, bs_t<cpu_flag> sub)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these functions really don't belong here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to cpu_thread::add_remove_flags.

Megamouse
Megamouse reviewed Nov 25, 2023
View reviewed changes
rpcs3/Emu/System.cpp
if (fs::file save{m_path, fs::isfile + fs::read}; save && save.size() >= 8 && save.read<u64>() == "RPCS3SAV"_u64)
fs::file save{m_path, fs::isfile + fs::read};

if (!m_path.ends_with(".gz") && save && save.size() >= 8 && save.read<u64>() == "RPCS3SAV"_u64)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh, I think this is getting out of hand. Every section of code that has m_ar-> in it belongs to another file.
Half of System.cpp is just writing or reading random stuff to that object.
I'd even be happy if it's just a second cpp with Emulator internal functions that only deal with the savestate.

@elad335 elad335 force-pushed the save-breathe branch 3 times, most recently from 3e16546 to 30f2ddf Compare November 26, 2023 11:13
@Ordinary205
Copy link
Contributor

This may sound like a silly question, but do I have to test this PR just to see if the files has significantly reduced? Or do I need to test this PR just to see if there's any savestate errors on other games? Because Demon's Souls seems to behave fine after loading.
Demon's Souls
I've tested the performance on offical and PR build from each comparison, and haven't noticed a slight regression.
RPCS3.log.gz

@elad335
Copy link
Contributor Author

elad335 commented Nov 27, 2023

The performance measurement needed here is for saving time itself, and also test file size by comparing file size to the one after extraction using 7zip or WinRAR etc. Although I know there is a a bug here preventing some games to reload properly such as Skate 3 or TLoU.

@elad335
Copy link
Contributor Author

elad335 commented Nov 27, 2023

Scratch that, I found the bug!! Will merge and you are welcome to test on master!

@elad335 elad335 merged commit fdb2fec into RPCS3:master Nov 27, 2023
4 of 6 checks passed
@elad335 elad335 deleted the save-breathe branch November 27, 2023 10:36
@elad335 elad335 mentioned this pull request Nov 28, 2023
Merged
@AniLeo AniLeo mentioned this pull request Jan 1, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Megamouse Megamouse Megamouse left review comments

Assignees
No one assigned
Labels
Loader Involving the load of PS3 file formats Optimization Optimizes existing code Savestates Anything that involves savestates
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@elad335 @mirh @Ordinary205 @Megamouse