Save/Load are slow

[MadShadow-]

In my opinion, saving & loading takes too long. And I compared it with the original game, where save and load times are significantly shorter.

What I found so far

I started with looking into the save game routine and discovered the main bottle neck to be in the serialization class, inside the SetEntry method which calls mz_zip_reader_locate_file_v2. If I got it correct, the issue here is that each entry is a new file in the zip directory. Everytime SetEntry is called, the algorithm will search in the directory for a file with that entry name - if not present, create one, if present, add content. This takes extremely long, as a lot of string comparisons have to be done. I unzipped one of the save files, that contained up to 14k files.

Possible solutions

• A) find a way to remove the calls to mz_zip_reader_locate_file_v2 by adding some sort of tracking in the serialized classes. keep the rest as is.
• B) rewrite serialization to use a single file and let each class handle its own serialization. this is more work, lowkey error prone and not very flexible. but the result would probably be extremly fast in both save/load.. however solution A might be sufficent.

A general good writeup is isocpp.org/wiki/faq/serialization.

Further thoughts?

[thokkat]

As addition there was an unfinished attempt in https://github.com/Try/OpenGothic/pull/445 to accelerate loading times.

[MadShadow-]

In his last comment he says:

I'm preparing a better structured version of this PR.

Did this ever happen?

[MadShadow-]

Created related PR https://github.com/Try/OpenGothic/pull/617

[thokkat]

Did this ever happen?

No

[Try]

rewrite serialization to use a single file and let each class handle its own serialization

This is how it was long ago, and it's terrible to support and debug. One of key features of zip-based structure is ability to do partial skips, file-surgery and easier version management overall. I don't think it's acceptable to revert this back to big-binary blob.

As addition there was an unfinished attempt in https://github.com/Try/OpenGothic/pull/445 to accelerate loading times.

Not all, but some element of it were incorporated separately over time: game no longer uses stringstream.

find a way to remove the calls to mz_zip_reader_locate_file_v2

After looking into miniz details: it has 2 paths for mz_zip_reader_locate_file_v2. One is binary-search (on read) and linear on write. I'm assuming that write path here is offended, not the read. It's possible to use external check instead, via std::unordered_set<> or similar solution. With this check goal is to allocate folder in zip, but never allocate it twice.

[Try]

From discord chat:

[MadShadow-]

I'm assuming that write path here is offended, not the read. It's possible to use external check instead, via std::unordered_set<> or similar solution.

Thats right, using an external cache reduces save time from 6 to 2.8s - together with the best speed option. (3 saved games each, left is best compression, middle ist best speed, right is best speed with cached entries)

[Try]

Looks very promising! May I ask you to also check MZ_DEFAULT_LEVEL for completeness?

[MadShadow-]

These are times I got with the default level:

The necessary changes are pretty small - will you implement them yourself?

[Try]

Merged new mesh-parsing. While not quite what we discussed, but also load-time performance related.

The necessary changes are pretty small - will you implement them yourself?

Atm working on sound-related code, will pick it only afterwards. If you want - you can create a PR, just let me know.