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

Replace miniz.c with a pure-Rust implementation #41770

Closed
bstrie opened this issue May 5, 2017 · 7 comments
Closed

Replace miniz.c with a pure-Rust implementation #41770

bstrie opened this issue May 5, 2017 · 7 comments
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bstrie
Copy link
Contributor

bstrie commented May 5, 2017

As of today, it looks like the only two in-tree C dependencies are libbacktrace and miniz. Unlike libbacktrace, the existence of pure-Rust (de)compression libs sounds broadly useful and feasible to implement. Determine which functions in miniz we actually need and explore whether or not any crates in our library ecosystem can serve as a suitable replacement.

@bstrie bstrie added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 5, 2017
@kennytm
Copy link
Member

kennytm commented May 5, 2017

miniz is only used in the flate crate, which probably could be replaced by an external crate (flate2? deflate?) as a whole. Compression speed seems to be a primary concern.

/// Compress a buffer without writing any sort of header on the output. Fast
/// compression is used because it is almost twice as fast as default
/// compression and the compression ratio is only marginally worse.
pub fn deflate_bytes(bytes: &[u8]) -> Bytes;

/// Decompress a buffer without parsing any sort of header on the input.
pub fn inflate_bytes(bytes: &[u8]) -> Result<Bytes, Error>;

@est31
Copy link
Member

est31 commented May 5, 2017

There is also compiler-rt, but its being replaced by a pure rust implementation.

@BurntSushi
Copy link
Member

Do you need flate specifically? The snap crate is a pure Rust snappy implementation.

@bstrie
Copy link
Contributor Author

bstrie commented May 6, 2017

@BurntSushi If I had to guess, I'd say the purpose of this crate's inclusion is solely to operate on crate metadata. I wonder how hard it would be to go through and drop in several different implementations to benchmark different algorithms and settings.

@kennytm
Copy link
Member

kennytm commented May 6, 2017

@est31 compiler-rt is imported as a submodule, so it is out-of-tree and off-topic for this issue I guess? Otherwise we would need to consider hoedown, jemalloc, graphviz and LLVM as well 😉.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented May 6, 2017

For how compression is used in the compiler, see #37086 for prior discussion and some experiments.

@kennytm @est31 Submodule or not, all these things are still dependencies and potentially merit discussion — just not in this issue since it's specifically about miniz.c. Separate issues might be worthwhile for some of them, and also for native libraries that get pulled in via crates.io dependencies (if any). But of those @kennytm named, hoedown is temporary, LLVM is 200% impractical to get rid of, graphviz AFAIK isn't a dependency (we have "libgraphviz" but AFAIK that's a pure-Rust implementation of writing out dot files). jemalloc might be interesting, once rust-lang/rfcs#1974 merges and alloc_jemalloc moves to crates.io, it will only be used internally by the compiler.

@Mark-Simulacrum
Copy link
Member

miniz.c is no longer in the compiler. We've removed the in-compiler libflate, replacing with https://crates.io/crates/flate2, which itself does bind to it, but IMO that's outside the scope of tracking here, see rust-lang/flate2-rs#67.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants