Deflate settings changed when replacing in-tree flate with flate2 #42879
Labels
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Comments
|
Seems like going back to a lower compression level here is a good idea at least. I don't know if worrying about the zlib wrapper is too important though. |
Mark-Simulacrum
added
the
T-compiler
|
The zlib wrapper could cause problems if trying to compress data with the zlib wrapper using the functions that don't expect it, and vice versa. I don't know if rustc will read the rlib files from a different version though, so this may or may not be an issue. Changing this back to the old behaviour is just changing to use the reader/writers that operate on raw deflate streams so it would be very easy to change if it's needed. |
|
Ah yes no functional changes were intended! Should be easy to switch back to the deflate scheme and switch the compression level! |
bors
added a commit
that referenced
this issue
Jul 11, 2017
Use similar compression settings as before updating to use flate2 Fixes #42879 (My first PR to rust-lang yay) This changes the compression settings back to how they were before the change to use the flate2 crate rather than the in-tree flate library. The specific changes are to use the `Fast` compression level (which should be equivialent to what was used before), and use a raw deflate stream rather than wrapping the stream in a zlib wrapper. The [zlib](https://tools.ietf.org/html/rfc1950) wrapper adds an extra 2 bytes of header data, and 4 bytes for a checksum at the end. The change to use a faster compression level did give some compile speedups in the past (see #37298). Having to calculate a checksum also added a small overhead, which didn't exist before the change to flate2. r? @alexcrichton
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In the pr that replaced the in-tree flate library, with flate2, there were some changes to how the compression was used, that may not have been intentional. I was suggested to make an issue after commenting there as the PR was already merged.
Is using the
Compression::Defaultcompression level intended? The flate functions were changed to use a faster compression level some time ago (see #37298), going back to a higher compression setting could cause some performance regressions. I think the previous settings would be equal to using theCompression::Fastsetting in flate2.Also, the old functions didn't use a zlib wrapper from what I can see (there is a flag for that), but the new code does. Don't know if this could cause some issues when trying to use libs from different versions, though at least it adds 6 bytes of overhead due to the header and checksum.
The text was updated successfully, but these errors were encountered: