Separate codecs as a separate crate, allow direct configuration

[jkushmaul]

• git mv all codecs into a new compression-codecs crate to retain git history
• Add missing constructors to all codecs to allow custom configuration
• Exposes that by adding a common with_codec option to all impl as an alternative

Fixes #324, #359

[jkushmaul]

I used my fork - so main a commit behind, I rebased, next push will be up to date.
I am looking at the other failures. cargo test --all-features works fine locally - because I am on windows, I see the tests failing are due to that cfg requiring not(windows) so I can get to that real quicki.

What I really need help on though - is docs... I've never contributed to anything on crates.io so I don't know the level to which I need to add docs on each exposed struct or function.

[jkushmaul]

I just removed the restriction, the tests work fine on windows (after fixing the import I missed due to the cfg not windows) - maybe there was a good reason to not, but seems OK.

[NobodyXu]

CI failed in test_lzma_decoder_from_params, seems like a new test function

https://github.com/jkushmaul/async-compression/blob/0653a4dddf40595eea3c7ec578402d1000323042/crates/compression-codecs/src/xz2/decoder.rs#L121

Edit:

I think using

Xz2Decoder::try_from(params).unwrap();

would give us more detail in error message

[NobodyXu]

Thanks, just few feedback

[jkushmaul]

Thanks for the feedback - yeah I hit two issues
The first one was the cfg(not(windows)) and me at this time being on windows, never ran those tests.. So I fixed that one

I found what all these other ones are running in the github actions and was able to reproduce locally - going through those now

After that, I'll look at the re-export comments and the other ones you left, thanks for those. That's the kind of stuff I expected to have problems with having never published a crate, this is good for me.

[jkushmaul]

Interesting..

Locally - windows -

cargo --locked nextest run --workspace --all-features
...
Summary [  13.258s] 782 tests run: 782 passed, 0 skipped

And for the wasm32 - not sure there but maybe related - this might be why the cfg(not(windows)) was there... I'm not sure how I'm able to run these though...

[NobodyXu]

You can try WSL2, which uses a lite linux VM, to run tests on linux

For wasm, AFAIK right now no way of running tests for it

[jkushmaul]

Let me know if you want me to resolve those, I usually leave the resolve to the reviewer, is why I have not myself already.

[NobodyXu]

Thank you!

Just minor nit on the doc, plus clamping of the compression level (we probably want to clamp all of them)

[NobodyXu]

Thank you, LGTM!

cc @robjtede @Nemo157 if there's no objection, I will merge and then publish these new crates, plus invite you.

I will probably create an empty 0.1 crate, configure trusted release, invite you and then use release-plz to create release

[jkushmaul]

Thanks for tolerating me through all of that also, I'm sure you don't have a lot of time but it was helpful to me.

[NobodyXu]

https://crates.io/crates/compression-core

https://crates.io/crates/compression-codecs

cc @Nemo157 @yoshuawuyts @robjtede I've invited you to the new crates

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (ebae65f) to head (6ccf73f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@     Coverage Diff     @@
##   main   #363   +/-   ##
===========================
===========================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[NobodyXu]

0.4.28 released for all crates cc @jkushmaul

[jkushmaul]

You made that super easy for me to get through, Thanks!

I've updated my own crates (which required the custom lzma options).

My tests were already using the git+branch but now with the official crates versions all my tests still passing there as well. I did not have to update any signatures - other than the new signatures available to me which was nice too.

[NobodyXu]

Good to hear that everything works well without any issue!

Hopefully this will also reduce compilation time since codegen of codecs can be done in parallel to async-compression

[NobodyXu]

I did a cargo build --release --all-features --all-targets of async-compression 0.4.28 on my M2, and it gives me some interesting results:

compression-codecs takes about 51% of time on codegen, while async-compression takes only 2%.

Separating the codecs out does reduce compilation time a bit, and async-compression is so generic-heavy, I wonder if there's too much generic and some can be put in non-generic function to reduce codegen on instantiation?

[NobodyXu]

Seems like we do have a duplicate buf_write.rs within tokio/futures-io

[jkushmaul]

I understand the general concept, a compile performance issue but as far as what to look for I'm not sure I follow about the duplicate buf_write bit -

I tested with the same command you used above, with --timings - for all four of

• 0.4.27
• 5a04534a9eda5a0859d6dd782bfb13a75a8fb29c (The merged PR commit)
• 0.4.28
• 0.4.29
• 0.4.30

The total time was about the same - I know this was about codegen but the limits of what I know was to just look at the totals.

I'm not sure I can fix this quickly unless someone else has already quickly fixed it perhaps - I see a couple new releases with larger refactor - maybe that was to address this issue? If you need me to fix this, I'm all for it but my speed probably won't be what you would want, if someone else can (or has) fixed it that'd probably be better for you

[NobodyXu]

Hmmm actually any refactoring that fan simplify the codebase is good.

I've put together #377 just not sure if it's the right approach and I'd want robjtede to have a look at it