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

Single threaded option - rayon as the default feature #24

Open
apps4uco opened this issue Feb 8, 2022 · 1 comment
Open

Single threaded option - rayon as the default feature #24

apps4uco opened this issue Feb 8, 2022 · 1 comment
Labels
enhancement New feature or request

Comments

@apps4uco
Copy link

apps4uco commented Feb 8, 2022

I know the crate is specifically for multi-threaded encoding/decoding.
I have managed to get sub millisecond encoding per image for my use case of encoding hundreds of small png files concurrently, and I would like to use mtpng to have low level control over Indexed pngs with transparency.

However, server side I do not want to use many threads on each request. Throughput of the server is more important, not time per request, so I think using the current thread would be the best way to do this.

I have looked at the code to see how easy it would be to have rayon as a default (optional) dependency, and be able to add default-features=false. However, I dont understand the code enough to remove the multithreading part in encoder.rs.

Also Im not even sure there would be a significant performance gain over

let pool = rayon::ThreadPoolBuilder::new().num_threads(1).build().unwrap();
(except that creating a thread pool per request seems like a bad idea)

I'd like to get feedback on this, also it could be useful for the WASM issue #13

@bvibber bvibber added the enhancement New feature or request label Apr 24, 2022
@bvibber
Copy link
Owner

bvibber commented Apr 24, 2022

Hmmmmmm, well honestly it should be possible to gate stuff on a platform/feature, and it may indeed be useful to have a common API between threaded and non-threaded versions of apps. I'll keep this in mind for my upcoming refactor. :)

I'm planning to move the deflate compression portion out to a separate library, so that's a great chance to do some cleanup on the guts while I'm moving it around. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants