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
Uh oh!
There was an error while loading. Please reload this page.
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: