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

Better parallelism controls #2357

Open
Shnatsel opened this issue Oct 21, 2024 · 7 comments
Open

Better parallelism controls #2357

Shnatsel opened this issue Oct 21, 2024 · 7 comments
Labels
kind: API missing or awkward public APIs

Comments

@Shnatsel
Copy link
Contributor

image only provides the rayon compile-time feature for controlling parallelism. There are no runtime controls exposed, which means there isn't an obvious way to control things like:

  1. Whether format encoders/decoders will use multi-threading (OpenEXR, AVIF). Multi-threading in batch workloads that saturate all cores anyway would only cause slowdowns.
  2. Whether certain image operations will use parallelism internally. For example, resize Resizing images is slow #2223 and blur blur function is too slow #986 could benefit, but unconditionally enabling parallelism would degrade performance for small images so we aren't using parallelism right now.
@Shnatsel Shnatsel added the kind: API missing or awkward public APIs label Oct 21, 2024
@fintelia
Copy link
Contributor

fintelia commented Oct 21, 2024

  1. This is particularly tricky because methods like image::open or DynamicImage::save don't provide knobs to precisely control behavior, but I'm not thrilled about just defaulting to single-threaded implementations unless users go out of their way to request multithreading. For codecs, maybe the right thing to do is to add overrides on ImageEncoder / ImageDecoder to force single-threading for folks who need it, and otherwise base on whether the rayon feature is enabled?
  2. I think the decision of whether an image is too small to benefit from parallelism should be within the library. Afterall, we're probably the best suited to measure what the fixed overheads are.

@awxkee
Copy link
Contributor

awxkee commented Oct 21, 2024

I'd also prefer to delegate decision to library how many threads to use, and if its even worth to the library, with simple flag if its enabled or no, with multi-threading enabled by default.

@kornelski
Copy link
Contributor

I'd like to be able to limit max number of cores. I've got servers with 128 cores, and each library creating a threadpool for the entire core count makes everything an overkill.

@Shnatsel
Copy link
Contributor Author

For deployment-time rather than coding-time thread count customization, I believe the RAYON_NUM_THREADS environment variable documented here would be a good option.

@fintelia
Copy link
Contributor

Also, I'd expect we'd use the rayon global thread pool rather than creating our own. That seems like it would mitigate the issue by sharing the same threads with any other library that used rayon

@kornelski
Copy link
Contributor

But I'd like to use max 4 cores for each image being decoded (there are diminishing results from splitting decoding further, and I don't want one job to monopolize the thread pool), but not have the whole 128-core server limited to 4 cores.

@fintelia
Copy link
Contributor

Wait, doesn't rayon default to using the current thread pool? So if you call image::open from within a 4 thread pool, any parallel operations would be limited to those thread unless we went out of our way to use a different pool?

You'd have to keep track of a bunch of different thread pools, but that seems unavoidable if you're trying to subdivide 128 cores

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: API missing or awkward public APIs
Projects
None yet
Development

No branches or pull requests

4 participants