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

Allow users to pass max_threads to the avif encoder via Image.save #54

Merged
merged 4 commits into from
Jul 3, 2024

Conversation

fdintino
Copy link
Owner

@fdintino fdintino commented Jul 3, 2024

Copy of #49 from @yit-b, which was accidentally closed when I unintentionally pushed this repository's main branch to the PR's origin (and now that the pull request is closed I am unable to push the correct ref). Original PR body:

It's not always desirable to set max_threads equal to the number of cpus on the host machine - especially in shared or containerized environments. Performance is often significantly worse if you have too many CPUs (see benchmarks below).

This PR gives users control over how many threads will be used for encoding by adding a max_threads argument accepted by Image.save(). Example:

save_test.py

import time
import pillow_avif
from PIL import Image

if __name__ == "__main__":
    img = Image.open("tests/images/flower.jpg")

    n_warmups = 1
    n_iters = 10

    for n_threads in range(0, 17):
        for _ in range(n_warmups):
            img.save("flower.avif", quality=80, max_threads=n_threads)
        start = time.time()
        for _ in range(n_iters):
            img.save("flower.avif", quality=80, max_threads=n_threads)
        print(
            f"N threads: {n_threads}. Avg time: {((time.time() - start) / n_iters * 1000):.2f} ms/img"
        )

Output:

N threads: 0. Avg time: 71.45 ms/img
N threads: 1. Avg time: 128.72 ms/img
N threads: 2. Avg time: 76.32 ms/img
N threads: 3. Avg time: 61.18 ms/img
N threads: 4. Avg time: 62.81 ms/img
N threads: 5. Avg time: 61.81 ms/img
N threads: 6. Avg time: 61.33 ms/img
N threads: 7. Avg time: 65.15 ms/img
N threads: 8. Avg time: 62.29 ms/img
N threads: 9. Avg time: 63.38 ms/img
N threads: 10. Avg time: 62.56 ms/img
N threads: 11. Avg time: 61.89 ms/img
N threads: 12. Avg time: 64.02 ms/img
N threads: 13. Avg time: 64.10 ms/img
N threads: 14. Avg time: 66.18 ms/img
N threads: 15. Avg time: 64.52 ms/img
N threads: 16. Avg time: 64.76 ms/img

The default behavior remains unchanged. It is 0 if not specified which is set to the cpu count.

Maybe this should be changed to a more reasonable default since performance seems the same with max_threads=0 (all CPUs) as it does with just 2 - probably because of contention? Maybe changing the default is outside the scope of this PR but I'd like to be able to tailor the parallelism to my compute environment.

Reproduce my tests:

conda activate foobar # This is some new empty conda environment
conda install -c conda-forge pillow 'libavif>=1.0.2' aom 'python=3.10.*' pillow
cd pillow-avif-plugin
pip install --no-deps .
python save_test.py

@fdintino fdintino merged commit a6d6dd1 into main Jul 3, 2024
120 of 121 checks passed
github-merge-queue bot referenced this pull request in BSStudio/bss-web-file-api Jul 9, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [pillow-avif-plugin](https://togithub.com/fdintino/pillow-avif-plugin)
| `1.4.3` -> `1.4.4` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/pillow-avif-plugin/1.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/pillow-avif-plugin/1.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/pillow-avif-plugin/1.4.3/1.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/pillow-avif-plugin/1.4.3/1.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>fdintino/pillow-avif-plugin (pillow-avif-plugin)</summary>

###
[`v1.4.4`](https://togithub.com/fdintino/pillow-avif-plugin/releases/tag/v1.4.4)

[Compare
Source](https://togithub.com/fdintino/pillow-avif-plugin/compare/v1.4.3...v1.4.4)

#### What's Changed

- chore(ci): bump libavif to
[`e10e6d9`](https://togithub.com/fdintino/pillow-avif-plugin/commit/e10e6d9)-2024-07-01;
fix CI build issues by [@&#8203;fdintino](https://togithub.com/fdintino)
in
[https://github.com/fdintino/pillow-avif-plugin/pull/53](https://togithub.com/fdintino/pillow-avif-plugin/pull/53).
See table below for new versions (all versions are upgrades from the
1.4.3 release).

    |             |            |
    |-------------|------------|
| **libavif** | <b>1.0.3
([e10e6d9](https://togithub.com/AOMediaCodec/libavif/commit/e10e6d98e6d1dbcdd409859a924d1b607a1e06dc))</b>
|
    | **libaom**  | **3.9.1**  |
    | **dav1d**   | **1.4.3**  |
    | **SVT-AV1** | **2.1.1**  |
    | **rav1e**   | **0.7.1**  |

- feat: Allow users to pass `max_threads` to the avif encoder via
`Image.save` by [@&#8203;yit-b](https://togithub.com/yit-b) in
[https://github.com/fdintino/pillow-avif-plugin/pull/54](https://togithub.com/fdintino/pillow-avif-plugin/pull/54),
originally in
[https://github.com/fdintino/pillow-avif-plugin/pull/49](https://togithub.com/fdintino/pillow-avif-plugin/pull/49)

- feat: Let users pass `max_threads` as an argument to
`_avif.AvifDecoder` by [@&#8203;yit-b](https://togithub.com/yit-b) in
[https://github.com/fdintino/pillow-avif-plugin/pull/50](https://togithub.com/fdintino/pillow-avif-plugin/pull/50)

- chore(ci): build SVT-AV1 for aarch64 or arm64 by
[@&#8203;RaphaelVRossi](https://togithub.com/RaphaelVRossi) in
[https://github.com/fdintino/pillow-avif-plugin/pull/38](https://togithub.com/fdintino/pillow-avif-plugin/pull/38)

- fix: keep alpha channel for images with mode P and custom transparency
by [@&#8203;fdintino](https://togithub.com/fdintino) in
[https://github.com/fdintino/pillow-avif-plugin/pull/56](https://togithub.com/fdintino/pillow-avif-plugin/pull/56);
fixes
[https://github.com/fdintino/pillow-avif-plugin/issues/48](https://togithub.com/fdintino/pillow-avif-plugin/issues/48)

- fix: disable decoder strictness for `clap` and `pixi` properties by
[@&#8203;fdintino](https://togithub.com/fdintino) in
[https://github.com/fdintino/pillow-avif-plugin/pull/57](https://togithub.com/fdintino/pillow-avif-plugin/pull/57).
fixes
[https://github.com/fdintino/pillow-avif-plugin/issues/13](https://togithub.com/fdintino/pillow-avif-plugin/issues/13),
fixes
[https://github.com/fdintino/pillow-avif-plugin/issues/28](https://togithub.com/fdintino/pillow-avif-plugin/issues/28)

- fix(ci): lint secrets permission error and macOS GHA runner homebrew
`PATH` bug by [@&#8203;fdintino](https://togithub.com/fdintino) in
[https://github.com/fdintino/pillow-avif-plugin/pull/55](https://togithub.com/fdintino/pillow-avif-plugin/pull/55)

#### New Contributors

- [@&#8203;RaphaelVRossi](https://togithub.com/RaphaelVRossi) made their
first contribution in
[https://github.com/fdintino/pillow-avif-plugin/pull/38](https://togithub.com/fdintino/pillow-avif-plugin/pull/38)

**Full Changelog**:
fdintino/pillow-avif-plugin@v1.4.3...v1.4.4

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/BSStudio/bss-web-file-api).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MjUuMSIsInVwZGF0ZWRJblZlciI6IjM3LjQyNS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants