-
Notifications
You must be signed in to change notification settings - Fork 18
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
Huge size of the InflateStream::next_state method #34
Comments
Yeah, this was intentional, although there might be ways to reduce it. I didn't know about Which uses the dynamic implementation for the fixed huffman codes (there could be a further optimization by using |
FWIW nowadays I'd do an on-line (aka "non-blocking") design much differently, by using generators directly (sadly they don't work on stable), which would result in easier to write and more contributor-friendly code, compared to manual state-machines. |
I've tried to uncomment this code, but nothing is changed. Nor size, not speed. I've also tried to refract this code by myself, but there are too many macros to understand it. |
There is also miniz_oxide (a rust port of miniz), which is now used as an alternate rust back-end in flate2. I have a tool here that I originally wrote for Due to the lack of a switch fallthrough in rust, state machines will easily end up large. |
@RazrFalcon oh I guess you'd also need to remove the |
@oyvindln currently, I'm only interested in the crate size. Performance is irrelevant. Also, I only need decompression. |
@eddyb Wow! All tests are passed and the method size is down to 45.6KiB (from 200KiB)! Performance is mostly the same, maybe a bit faster. Bench results:
Original code has around 1.9s. But why this 'method' is commented out? |
@RazrFalcon It wasn't faster when I tried it! I left it commented out in case we'd eventually optimize the dynamic path further for this usecase. Feel free to open a PR 🎉 |
Will do. |
Closed by #35 |
According to the
cargo-bloat
theInflateStream::next_state
method is a whopping 200KiB. I presume it's because of heavy macro usage because according tocargo-expand
this method is being expanded from 405 LOC up to 10000 LOC.libflate
on other hand is just 30-60KiB total.Also,
libflate
is faster, so it may be a good idea to use it instead ofinflate
.I have no idea how this code work so I can't refract it by myself.
The text was updated successfully, but these errors were encountered: