Skip to content

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