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

Package pillow-simd? #103

Open
h-vetinari opened this issue Dec 21, 2021 · 11 comments
Open

Package pillow-simd? #103

h-vetinari opened this issue Dec 21, 2021 · 11 comments

Comments

@h-vetinari
Copy link
Member

Hey @conda-forge/pillow

I'm co-maintaining the torchvision feedstock, and recently noticed that they are actually depending** on pillow-simd, a SIMD-optimized fork of pillow. Here is a performance comparison that - AFAICT - is even hosted on the main pillow website, where SSE4 already achieves substantial speedups, and AVX2 even more so, on the order of a factor of 5x.

Given that the sources are very very similar, I'd be wary of having this in a separate feedstock (requiring a mutex etc.), and also pillow-simd formulates their policy:

Pillow-SIMD is "following" Pillow. Pillow-SIMD versions are 100% compatible drop-in replacements for Pillow of the same version. For example, Pillow-SIMD 3.2.0.post3 is a drop-in replacement for Pillow 3.2.0, and Pillow-SIMD 3.3.3.post0 — for Pillow 3.3.3.

The downside of building both outputs in this feedstock would be that new pillow releases would have to also wait on the corresponding pillow-simd release.

Any thoughts?

CC @conda-forge/torchvision @conda-forge/core

** conditional on availability, with fallback to pillow

@ocefpaf
Copy link
Member

ocefpaf commented Dec 21, 2021

Is pillow-simd at PyPI? I could not find it. (Not important here, just curious.)

I think that the approach here should be a different feedstock with a run_constrained line to avoid installing both in the same environment. What do you think @isuruf?

@xhochy
Copy link
Member

xhochy commented Dec 21, 2021

Different feedstock sounds like a good idea. Both of them could have a metapackage that provides, e.g. pillow-meta with a {{ pin_subpackage(…, exact=True) }} for the corresponding implementation.

@h-vetinari
Copy link
Member Author

Both of them could have a metapackage that provides, e.g. pillow-meta with a {{ pin_subpackage(…, exact=True) }} for the corresponding implementation.

Would we need repopatches for the old pillow releases? Or just add a version constraint? 9.0.0 was recently released, so that'd be a decent opportunity.

Do I understand you correctly that this would roughly be the structure?

# pillow-feedstock/recipe/meta.yaml
...
outputs:
  - name: pillow
    build:
      [...]
      track_features:
        - ?
      run_exports:
        - pillow-meta =*=*default

  - name: pillow-meta
    build:
      string: "{{ build_num }}_default"
      track_features:
        - ?
    requirements:
      host:
        - {{ pin_subpackage("pillow", exact=True) }}
      run_constrained:
        - ?
# pillow-simd-feedstock/recipe/meta.yaml
...
outputs:
  - name: pillow-simd
    build:
      [...]
      track_features:
        - ?
      run_exports:
        - pillow-meta =*=*simd

  - name: pillow-meta
    build:
      string: "{{ build_num }}_simd"
      track_features:
        - ?
    requirements:
      host:
        - {{ pin_subpackage("pillow-simd", exact=True) }}
      run_constrained:
        - ?

@h-vetinari
Copy link
Member Author

9.0.0 was recently released, so that'd be a decent opportunity.

Sidenote: pillow apparently has an 18 month cadence between major releases. The source code already contains the release date for Pillow 10.

@isuruf
Copy link
Member

isuruf commented Jan 4, 2022

there's no need for a new meta package. pillow itself can be the mutex package just like python being used for both cpython and pypy.

@h-vetinari
Copy link
Member Author

there's no need for a new meta package. pillow itself can be the mutex package just like python being used for both cpython and pypy.

OK cool; though that still means at least a modification of the build string, correct?

@h-vetinari
Copy link
Member Author

Actually, on second thought, I'm not sure it's a great idea to use pillow as the mutex, or at least we should then add an extra output pillow-simd (since those that are aware of the package variant are using pip install pillow-simd currently)?

@isuruf
Copy link
Member

isuruf commented Jan 4, 2022

No. pillow-simd can be the package with the files and pillow can be a meta package. This is what we do for python which is a meta-package for pypy variant and it depends on pypy3.8 package which has the files.

@hugovk
Copy link
Contributor

hugovk commented Jan 4, 2022

Sidenote: pillow apparently has an 18 month cadence between major releases. The source code already contains the release date for Pillow 10.

Pillow releases quarterly (Jan, Apr, Jul, Oct), and we're doing major releases to coincide with dropping Python versions.

So the next major will coincide with dropping 3.7 in July 2023, and after that Python's annual release cadence will have filtered through to EOLs: 3.8 in October 2024, 3.9 in October 2025, 3.10 in October 2026...

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Jan 4, 2022

Is there is any functionality in torchvision that depends on pillow-simd? If not, I would be worried about pinning pillow-simd at the torchvision package level.

@h-vetinari
Copy link
Member Author

@isuruf: No. pillow-simd can be the package with the files and pillow can be a meta package.

OK, got it. I'll try to open a PR for a new feedstock

@hmaarrfk: Is there is any functionality in torchvision that depends on pillow-simd? If not, I would be worried about pinning pillow-simd at the torchvision package level.

No, it just prefers pillow-simd if it's available (presumably for speed).

Thanks for the context about the release cadence @hugovk!

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

No branches or pull requests

6 participants