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

Support writing compressed save data #83

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

Conversation

JoeyBF
Copy link
Collaborator

@JoeyBF JoeyBF commented Mar 9, 2022

It works but is much slower for low stems (7x). I would expect it to be asymptotically negligible. Already going from stem 100 to stem 101 takes only 61% longer.

One idea would be to have a static ext::save::COMPRESSION_LEVEL: AtomicI32 that would control the compression level dynamically, and we could adjust it as the stems get larger.

Resolves #82

@dalcde
Copy link
Contributor

dalcde commented Mar 9, 2022

We would need to update the delete-empty-file code to deal with this as well.

(also, can you put the #[allow(unused_mut)] on the let mut p line instead of
the whole function? Not sure if this is a thing)

@JoeyBF
Copy link
Collaborator Author

JoeyBF commented Mar 9, 2022

Did you have something like this in mind? It looks clean to me but I might have overengineered it

@dalcde
Copy link
Contributor

dalcde commented Mar 9, 2022 via email

@JoeyBF
Copy link
Collaborator Author

JoeyBF commented Mar 9, 2022

Yeah you're right. Even if the program crashes right at the end of create_file it still creates a 25-byte compressed file, containing the 16-byte header. Compressing an empty file still gives 13 bytes, which I suppose is the zstd header.

@JoeyBF
Copy link
Collaborator Author

JoeyBF commented Mar 9, 2022

Actually, why is this not a problem for uncompressed files? Shouldn't they always at least contain the header?

@dalcde
Copy link
Contributor

dalcde commented Mar 9, 2022 via email

@JoeyBF
Copy link
Collaborator Author

JoeyBF commented Mar 10, 2022

I think then we should immediately flush at the end of create_file to make sure the file is never empty, and then we can panic when open_file encounters an empty file, because it would never happen unless something went very wrong. We can move the emptiness check to the SaveFile::open_file with the header verification

@dalcde
Copy link
Contributor

dalcde commented Oct 11, 2022 via email

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

Successfully merging this pull request may close these issues.

Enable writing compressed files
2 participants