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

Save Database.json with gzip #164

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

yfdyh000
Copy link
Contributor

@yfdyh000 yfdyh000 commented Sep 14, 2019

This closes #162.

This is a dirty implementation for #162, including hard-coded, non-reusable, no tests, etc. I originally planned it as new functions or a new user option.

I see that the compression ratio is about 4%, which is huge. Database writes are infrequent, but it seems to occur at startup, and perhaps multiple loops when a particular operation.

Thanks to https://stackoverflow.com/questions/32943899/can-i-decompress-and-deserialize-a-file-using-streams.

Copy link
Member

@mvegter mvegter left a comment

Choose a reason for hiding this comment

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

Great idea, please adhere to the code style. I have added a few points that require some further thinking.

src/Depressurizer.Core/Helpers/Locations.cs Outdated Show resolved Hide resolved
src/Depressurizer/AutomaticModeForm.cs Outdated Show resolved Hide resolved
return custPath;
}

if (File.Exists(Locations.File.DatabaseGzip))
Copy link
Member

Choose a reason for hiding this comment

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

Are we going to prefer Gzip version or should this be a user option as default saving strategy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't thought of any necessity for user options, although I have imagined it to be switchable.

@@ -576,7 +617,9 @@ public bool IsType(int appId, AppType appType)

public void Load()
Copy link
Member

Choose a reason for hiding this comment

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

See previous comment regarding user/profile option regarding saving strategy.

@mvegter mvegter added this to the v4.13.X milestone Sep 14, 2019
@mvegter mvegter self-assigned this Sep 14, 2019
@mvegter mvegter self-requested a review September 14, 2019 21:13
@mvegter
Copy link
Member

mvegter commented Sep 15, 2019

@yfdyh000 I have done some changes on the master branch, be sure to update asap to prevent any conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

[REQUEST] Save database.json to a zip file
2 participants