inflate
inflate copied to clipboard
image-rs
Big functions makes it difficult to parallelize compilation
Rust is adding support to compile and optimize at link time in parallel. Code units seems to be functions? or at least it can't parallelize in finer grain than functions. Because this crate has a big single function compared with the rest of the code, compile times suffer. Ideally (and I would argue may make code easier to maintain) that function can be broken up into smaller ones . Please see this awesome technical explanation.
it's one huge state machine so optimizing separate functions seems counter-intuitive IMO. I guess you could try splitting it off into even more methods, for nested state machines - but we do this already to some extent. Codegen units aren't function-level, LLVM can't optimize functions in parallel, it was never designed with that in mind and there are too many hazards. Also, aren't compile time mostly in macro expansion for this crate?
Related to macro expansion: there may be code-size vs performance tradeoffs to look at. The way I wrote the static Huffman decoding was with a lot of macro-generated code, for many combinations.
There's some commented out code for building a dynamic Huffman decoder with the appropriate data for the static one, getting that to the same level of performance would probably be a huge win.
Just ran cargo-bloat on a project using this crate (through image) and this was at the top:
5.9% 10.3% 178.4KiB inflate::InflateStream::next_state
So, yeah. That's way larger than I expected it to be.
@killercup See #34 - it'd be nice to close this issue (do we need to publish a new inflate version?).
Ah yes, 0.3.3 is the latest version but it doesn't contain #35.
@eddyb since this issue is about using codegen units, I don't think this affected at all? (But I also think crates should be optimized for this…)
@killercup Well, I'm not sure the issue is relevant anymore now that the huge macro-expanded part is gone - I expect compile times to be much more reasonable now.
cc @bvssvni for releasing a new patch version
@bvssvni can you make me the owner of inflate as well? Just tried to publish myself, didn’t work 😄
