feature-gate parallel (de)compression

[dakom]

• feature-gates parallel (de)compression
• doesn't change current API (enables by default)
• fixes #200

[johannesvollmer]

thanks for taking the time! looks reasonable, although I'd refactor it later some time to reduce the number of conditionals, maybe by moving all the parallel stuff into one module and cfg-ing that module as a whole. not now, though. i think we can merge this, I'll review it soon

one thing i want to see is some kind of test in the CI that checks whether the library actually compiles when this flag is active, otherwise someone might break the flag later without noticing

[johannesvollmer]

https://github.com/johannesvollmer/exrs/blob/66a247789004d974043300daf7bfc57c7d52a459/.github/workflows/rust.yml#L10

here's the CI pipeline. it should be fairly easy to add a job for compilation and test execution with the flag. are you interested in adding that?

[dakom]

thanks for the quick response! agreed on all those points :)

Added the CI job

I had to make a small opinionated refactor to get the tests to work, in 8_read_raw_blocks, moving the callback into a closure, but thankfully you had already structured both the sequential and parallel versions to take the same callback, so it was pretty easy

it's late for me - happy to followup more tomorrow ✌️

[dakom]

looks like they're all green :)

[johannesvollmer]

I can probably merge on the weekend :)

[johannesvollmer]

after thinking about it, I would definitely go the route of automatic fallback to sequential decompressions. I'd still accept having the feature flag, but I would first code up the fallback mechanism, as this will probably remove the need for some of the cfg(...) conditions in this pr.

I appreciate your time and the work you put in this. and I would welcome if you still want to merge the changed to the continuous integration. I would like to postpone the feature flag until the fallback is in place though. what are your thoughts?

[dakom]

Yeah, all good, feel free to close the PR if you prefer :)

The main thing for me was just being able to get wasm to work, and I wrote the PR before I realized that the error could just be avoided by specifying non-parallel

[johannesvollmer]

I'd still appreciate if you added the WASM CI task if you have the time. Just to celebrate your will to contribute at all :) I hope you understand what I mean. And of course for value it would bring to this project, by boosting the confidence for wasm targets

[dakom]

heya, sorry I was afk last week... reminding myself to circle back to this :)

[johannesvollmer]

the project will use a different mechanism for controlling whether threading is used, but thanks for the wasm ci code :)