feat(bin): enable the lib's rayon feature#33
Conversation
|
Hi, thank you for the PR!
The
This is not true, we have a different approach to threading/parallelization in the source for the binary. Have you tested the |
|
Thanks for the pointers — I read #21 and #35, and I agree with the reasoning for video mode: frame-level threading composes better than intra-frame rayon, and stacking both oversubscribes. My "always single-threaded" wording was too broad. The case I care about is Would you take a version scoped so video behavior is strictly preserved? Concretely: keep the feature enabled for the binary, and in video mode run each frame worker's scoring inside a 1-thread |
The binary depends on the ssimulacra2 lib with default-features = false, which turns the lib's rayon feature (on by default, parallel horizontal blur pass) off for every installation of the CLI, however it is built. Enabling it is measurable on still-image scoring: on a 1448x1080 pair, hyperfine reports 1.231 s -> 1.013 s (Core Ultra 7 155H). No output or score change.
3542d94 to
0fe1c90
Compare
|
Hmm right, sorry I was focused mainly on the video feature/subcommand. I don't think our current implementation of the underlying ssimulacra2 crate can easily include both the rayon and non-rayon code paths... we should probably find a proper solution for this. Just enabling rayon is going to regress video performance, but parallelizing image mode would be great. |
Frame-level threading already saturates the CPU in video mode; the lib's rayon feature must not multiply it (see ssimulacra2_bin#21). Each frame worker installs its scoring loop in a local 1-thread rayon pool, so per-frame scoring stays sequential and CPU usage matches the previous non-rayon build, while image mode uses the default parallel pool.
|
I've pushed the scoped version. To check the video-regression concern, I benchmarked a harness mirroring the video worker structure (N threads, each calling
The naive config reproduces the #21 findings; the pooled config is within run-to-run noise of current behavior. Image mode still goes 1.231 s ± 0.014 → 1.013 s ± 0.044. |
|
This looks like a good compromise. We might want to extend the parallelization for image mode in the future, e.g. by supplying a CLI parameter similar to video mode's |
|
Hm.. on my laptop CPU, the performance improvements are so small as to be negligible (<1%) for a 1920x1080 image pair. I will bench this again on my desktop system and report back. By the way:
If this is the usecase, have you tried running the entire program multiple times in parallel? This rayon approach is pretty inefficient by comparison. If you can do multiple images at the same time, it should be easily 2x as fast, which is why we do it in |
|
That would be consistent with my numbers: upstream's rayon feature only parallelizes the horizontal blur pass, so the gain scales roughly with core count (I measured 18% on a 22-thread machine; on a 4-8 core laptop it shrinks toward noise, and PNG decode time dilutes it further). Your desktop bench should tell. On running multiple instances instead: that works for throughput, but the use case is a quality-target search (bisection over encoder quality), where iteration N+1 depends on the score of iteration N — it's inherently sequential, so what matters is the latency of one comparison, not aggregate fps. Multiple processes don't help there. No hard feelings either way — if the numbers don't justify it on typical hardware, feel free to close. |
I understand this, although I would've expected a target quality search to produce more than one image for N before stepping to N+1, at least in a video encoding context. Either way, it's understandable to want a faster one-shot image comparison. On my 32-thread desktop machine, I gain a reproducible advantage of roughly 11%. That's a good increase. I tend towards merging it, but I'd like to hear from @shssoichiro first since it's a change with tradeoffs. |
The binary depends on the lib with
default-features = false, which silently disables the lib'srayonfeature (on by default, used for the parallel horizontal blur pass) for every install of the CLI, however it is built.cargo install ssimulacra2_rstherefore always produces a fully single-threaded scorer.This PR turns the feature back on for the binary:
default-features = false, features = ["rayon"].Measured with hyperfine (10 runs, Core Ultra 7 155H, Linux) on a 1448×1080 pair with
ssimulacra2_rs image:No score or output change (the parallel rows keep their per-row summation order).
There is more parallelism available in the rest of the pipeline (vertical pass, planes, downscale, maps); happy to follow up with that work if there's interest — this one-liner is the zero-risk part.