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

Significantly Slash Savefile Sizes #5075

Merged
merged 1 commit into from
Dec 19, 2020

Conversation

Web-eWorks
Copy link
Member

@Web-eWorks Web-eWorks commented Dec 15, 2020

Our savefiles are truly too large. Not only are we storing completely redundant data about MatrixTransform nodes that will immediately be overwritten by the animation system, but we're also storing every last detail about every market in the system regardless of whether we've been there or not.

On top of removing redundant save data from the C++ side, we now only create a station's market when we actually land there, which removes one of the biggest hogs of save data. How big, you ask? Uncompressed, our savefiles dropped from 2MB to ~500KB, and compressed they dropped from 350KB to 75KB. I've tested on a Mars start after taking off and landing at Mars High to generate a representative sample of the average in-system activity a player might have.

This has a nice side effect of making autosaves almost entirely imperceptible to gameplay; it's not quite as smooth as an asynchronous save might be, but we practically fly through the save process now.

@impaktor for your reference, this is following on to your(?) comment in the code about only creating adverts when docked... I just extended that to the entire station market instead of just the BBS.

I'd appreciate some thorough testing - I've put it through a minimal test case, but I'd appreciate it if this could be tested against someone's in-progress campaign just to be sure there aren't any edge cases or catastrophic oversights I've missed!

EDIT: I know I said we'd be freezing for an RC build soon, but while we're bumping the save version I wanted to fix as many "bugs" as possible. 😛

This change drops the average savefile from 2MB to 500KB.
We're smarter about what data we actually need to serialize -
MatrixTransform is only ever positioned by animations, and we don't
(yet) need to know about every space station market in the system at once.
@Gliese852
Copy link
Contributor

That's wonderful!

@Gliese852
Copy link
Contributor

that is, until you land on the station, prices for goods do not exist now?

@Web-eWorks
Copy link
Member Author

Web-eWorks commented Dec 15, 2020

@Gliese852 yes, this defers creating a station's market, stocking it with goods, and making ships and advertisements for the BBS until a player physically lands at a station.

Right now a station's prices for goods does not differ from the overall system, so this poses no problem for existing code. In the future, I'd recommend treading lightly with what you serialize for "remote" stations that the player hasn't landed at - a table of string keys and string/number values is the most optimal and lightweight serialization mechanism when we're talking about filesizes and work needed to save/load. Storing subtables (e.g. equipment items) should ideally be avoided if you can, as they represent additional work (and a significantly larger portion of filesize) compared to basic "scalar" values like strings, numbers, and booleans.

Edit: a subtable, at the very lightest, involves an extra 20 bytes of overhead for every single reference to it stored in the save, as well as a decently expensive de-duplication check. Strings and numbers can be simply pushed directly to lua and don't have to go through the same mechanism.

@impaktor
Copy link
Member

this defers creating a station's market, stocking it with goods, and making ships and advertisements for the BBS until a player physically lands at a station.

Is there a way to trigger spawning fully populated stations from lua? I've relied on being able to query all stations for stock or adverts or ship market, to gather statistics on how many ships for sale, how many adverts of some flavour, or as you just saw, how many of some stock. It helps to balance things out.

For testers: make sure to check if BBS adverts are duplicated or lost between saves, as this has been recurrent theme.

@Web-eWorks
Copy link
Member Author

@impaktor I definitely had that in mind when writing this; it's not currently supported to generate everything in the system, but adding that when testing is as simple as looping through all stations in an onEnterSystem event and initializing a station market.

I'll look into adding that in a commented out function for your convenience when I get a chance!

@impaktor
Copy link
Member

well, if it's as easy as looping through the stations, then it sounds trivial, as only 2-3 lines of code, so no action needed on your part, thanks!

@fluffyfreak fluffyfreak self-requested a review December 15, 2020 09:36
@fluffyfreak
Copy link
Contributor

This makes me wonder what's in that remaining 500KB

@Web-eWorks
Copy link
Member Author

This makes me wonder what's in that remaining 500KB

@fluffyfreak mostly it's space Frames, bodies serialized from C++, modules' copies of advertisements that are also stored on space stations... I've done the 10% to get the 90% benefit, but there's certainly a few more tens of KB we could shave off. Although at this point I think that compression renders it negligible. We're almost fitting our savefile into a single LZ4 frame, which means that we're getting near-optimal compression of repeated data.

Copy link
Contributor

@fluffyfreak fluffyfreak left a comment

Choose a reason for hiding this comment

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

Code looks good as always!
I haven't had chance to test it yet so maybe let it bake whilst others give that a go for a few days?

@impaktor
Copy link
Member

I haven't had chance to test it yet so maybe let it bake whilst others give that a go for a few days?

I wonder if the best thing in these cases is to do a windows build and put on our discord channel and ask for players to throw themselves on the blade of the bleeding edge release

@Web-eWorks
Copy link
Member Author

@impaktor tada... https://github.com/pioneerspacesim/pioneer/suites/1669330574/artifacts/31118667 - there's your windows build, now all that's required is to put it on discord! 😄

@Web-eWorks
Copy link
Member Author

Because we're coming very close to cutting a release candidate build for large-scale testing, I'm going to go ahead and merge this in; I haven't seen any issues be reported on IRC and we now have people playing nightly master builds as primary bugtesting.

@Web-eWorks Web-eWorks merged commit 25688d0 into pioneerspacesim:master Dec 19, 2020
@Web-eWorks Web-eWorks deleted the slash-savefiles branch April 9, 2021 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants